- Changes since v2 Addressed comments from Ard Biesheuvel: * expose efi_runtime_lock to UV platform only instead of globally * remove unnecessary #ifdef CONFIG_EFI from bios_uv.c
- Changes since v1: Addressed comments from Bhupesh Sharma, Thomas Gleixner, and Ard Biesheuvel: * made __uv_bios_call() static * moved the efi_enabled() cleanup to its own patchlet * explained the reason for renaming the efi_runtime_lock semaphore * dropped the reviewed-bys as they should be given on the mailing list * Cc'ng stable@vger.kernel.org given the nature of the problem addressed by the patches
---
Calls into UV BIOS were not being serialised which is wrong as it violates EFI runtime rules, and bad as it does result in all sorts of potentially hard to track down hangs and panics when efi_scratch.prev_mm gets clobbered whenever efi_switch_mm() gets called concurrently from two different CPUs.
Patch #1 removes an unnecessary #ifdef CONFIG_EFI guard from bios_uv.c.
Patch #2 removes uv_bios_call_reentrant() because it's dead code.
Patch #3 is a cleanup that drops test_bit() in favour of the ad hoc efi_enabled().
Patch #4 makes uv_bios_call() variants use the efi_runtime_lock semaphore to protect against concurrency.
Cc: Russ Anderson rja@hpe.com Cc: Mike Travis mike.travis@hpe.com Cc: Dimitri Sivanich sivanich@hpe.com Cc: Steve Wahl steve.wahl@hpe.com Cc: stable@vger.kernel.org # v4.9+
Hedi Berriche (4): x86/platform/UV: remove unnecessary #ifdef CONFIG_EFI x86/platform/UV: kill uv_bios_call_reentrant() as it has no callers x86/platform/UV: use efi_enabled() instead of test_bit() x86/platform/UV: use efi_runtime_lock to serialise BIOS calls
arch/x86/include/asm/uv/bios.h | 13 ++++----- arch/x86/platform/uv/bios_uv.c | 35 ++++++++++++++----------- drivers/firmware/efi/runtime-wrappers.c | 7 +++++ 3 files changed, 34 insertions(+), 21 deletions(-)
CONFIG_EFI is implied by CONFIG_X86_UV and x86/platform/uv/bios_uv.c requires the latter, get rid of the redundant #ifdef CONFIG_EFI directives.
Cleanup, no functional changes.
Cc: Russ Anderson rja@hpe.com Cc: Mike Travis mike.travis@hpe.com Cc: Dimitri Sivanich sivanich@hpe.com Cc: Steve Wahl steve.wahl@hpe.com Cc: stable@vger.kernel.org # v4.9+ Signed-off-by: Hedi Berriche hedi.berriche@hpe.com --- arch/x86/include/asm/uv/bios.h | 4 ---- arch/x86/platform/uv/bios_uv.c | 2 -- 2 files changed, 6 deletions(-)
diff --git a/arch/x86/include/asm/uv/bios.h b/arch/x86/include/asm/uv/bios.h index e652a7cc6186..00d862cfbcbe 100644 --- a/arch/x86/include/asm/uv/bios.h +++ b/arch/x86/include/asm/uv/bios.h @@ -151,11 +151,7 @@ extern s64 uv_bios_change_memprotect(u64, u64, enum uv_memprotect); extern s64 uv_bios_reserved_page_pa(u64, u64 *, u64 *, u64 *); extern int uv_bios_set_legacy_vga_target(bool decode, int domain, int bus);
-#ifdef CONFIG_EFI extern void uv_bios_init(void); -#else -void uv_bios_init(void) { } -#endif
extern unsigned long sn_rtc_cycles_per_second; extern int uv_type; diff --git a/arch/x86/platform/uv/bios_uv.c b/arch/x86/platform/uv/bios_uv.c index 4a6a5a26c582..4a61ed2a7bb8 100644 --- a/arch/x86/platform/uv/bios_uv.c +++ b/arch/x86/platform/uv/bios_uv.c @@ -188,7 +188,6 @@ int uv_bios_set_legacy_vga_target(bool decode, int domain, int bus) } EXPORT_SYMBOL_GPL(uv_bios_set_legacy_vga_target);
-#ifdef CONFIG_EFI void uv_bios_init(void) { uv_systab = NULL; @@ -218,4 +217,3 @@ void uv_bios_init(void) } pr_info("UV: UVsystab: Revision:%x\n", uv_systab->revision); } -#endif
uv_bios_call_reentrant() has no callers nor is it exported, kill it.
Cleanup, no functional changes.
Cc: Russ Anderson rja@hpe.com Cc: Mike Travis mike.travis@hpe.com Cc: Dimitri Sivanich sivanich@hpe.com Cc: Steve Wahl steve.wahl@hpe.com Cc: stable@vger.kernel.org # v4.9+ Signed-off-by: Hedi Berriche hedi.berriche@hpe.com --- arch/x86/include/asm/uv/bios.h | 1 - arch/x86/platform/uv/bios_uv.c | 12 ------------ 2 files changed, 13 deletions(-)
diff --git a/arch/x86/include/asm/uv/bios.h b/arch/x86/include/asm/uv/bios.h index 00d862cfbcbe..8c6ac271b5b3 100644 --- a/arch/x86/include/asm/uv/bios.h +++ b/arch/x86/include/asm/uv/bios.h @@ -140,7 +140,6 @@ enum uv_memprotect { */ extern s64 uv_bios_call(enum uv_bios_cmd, u64, u64, u64, u64, u64); extern s64 uv_bios_call_irqsave(enum uv_bios_cmd, u64, u64, u64, u64, u64); -extern s64 uv_bios_call_reentrant(enum uv_bios_cmd, u64, u64, u64, u64, u64);
extern s64 uv_bios_get_sn_info(int, int *, long *, long *, long *, long *); extern s64 uv_bios_freq_base(u64, u64 *); diff --git a/arch/x86/platform/uv/bios_uv.c b/arch/x86/platform/uv/bios_uv.c index 4a61ed2a7bb8..91e3d5285836 100644 --- a/arch/x86/platform/uv/bios_uv.c +++ b/arch/x86/platform/uv/bios_uv.c @@ -66,18 +66,6 @@ s64 uv_bios_call_irqsave(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, return ret; }
-s64 uv_bios_call_reentrant(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, - u64 a4, u64 a5) -{ - s64 ret; - - preempt_disable(); - ret = uv_bios_call(which, a1, a2, a3, a4, a5); - preempt_enable(); - - return ret; -} -
long sn_partition_id; EXPORT_SYMBOL_GPL(sn_partition_id);
Use ad hoc efi_enabled() instead of fiddling with test_bit().
Cleanup, no functional changes.
Cc: Russ Anderson rja@hpe.com Cc: Mike Travis mike.travis@hpe.com Cc: Dimitri Sivanich sivanich@hpe.com Cc: Steve Wahl steve.wahl@hpe.com Cc: stable@vger.kernel.org # v4.9+ Signed-off-by: Hedi Berriche hedi.berriche@hpe.com --- arch/x86/platform/uv/bios_uv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/platform/uv/bios_uv.c b/arch/x86/platform/uv/bios_uv.c index 91e3d5285836..38a2e3431fc6 100644 --- a/arch/x86/platform/uv/bios_uv.c +++ b/arch/x86/platform/uv/bios_uv.c @@ -44,7 +44,7 @@ s64 uv_bios_call(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, u64 a4, u64 a5) * If EFI_OLD_MEMMAP is set, we need to fall back to using our old EFI * callback method, which uses efi_call() directly, with the kernel page tables: */ - if (unlikely(test_bit(EFI_OLD_MEMMAP, &efi.flags))) + if (unlikely(efi_enabled(EFI_OLD_MEMMAP))) ret = efi_call((void *)__va(tab->function), (u64)which, a1, a2, a3, a4, a5); else ret = efi_call_virt_pointer(tab, function, (u64)which, a1, a2, a3, a4, a5);
Calls into UV firmware must be protected against concurrency, expose the efi_runtime_lock to the UV platform, and use it to serialise UV BIOS calls.
Cc: Russ Anderson rja@hpe.com Cc: Mike Travis mike.travis@hpe.com Cc: Dimitri Sivanich sivanich@hpe.com Cc: Steve Wahl steve.wahl@hpe.com Cc: stable@vger.kernel.org # v4.9+ Signed-off-by: Hedi Berriche hedi.berriche@hpe.com --- arch/x86/include/asm/uv/bios.h | 8 +++++++- arch/x86/platform/uv/bios_uv.c | 23 +++++++++++++++++++++-- drivers/firmware/efi/runtime-wrappers.c | 7 +++++++ 3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/uv/bios.h b/arch/x86/include/asm/uv/bios.h index 8c6ac271b5b3..8cfccc3cbbf4 100644 --- a/arch/x86/include/asm/uv/bios.h +++ b/arch/x86/include/asm/uv/bios.h @@ -48,7 +48,8 @@ enum { BIOS_STATUS_SUCCESS = 0, BIOS_STATUS_UNIMPLEMENTED = -ENOSYS, BIOS_STATUS_EINVAL = -EINVAL, - BIOS_STATUS_UNAVAIL = -EBUSY + BIOS_STATUS_UNAVAIL = -EBUSY, + BIOS_STATUS_ABORT = -EINTR, };
/* Address map parameters */ @@ -162,4 +163,9 @@ extern long system_serial_number;
extern struct kobject *sgi_uv_kobj; /* /sys/firmware/sgi_uv */
+/* + * EFI runtime lock; cf. firmware/efi/runtime-wrappers.c for details + */ +extern struct semaphore __efi_uv_runtime_lock; + #endif /* _ASM_X86_UV_BIOS_H */ diff --git a/arch/x86/platform/uv/bios_uv.c b/arch/x86/platform/uv/bios_uv.c index 38a2e3431fc6..ef60d789c76e 100644 --- a/arch/x86/platform/uv/bios_uv.c +++ b/arch/x86/platform/uv/bios_uv.c @@ -29,7 +29,8 @@
struct uv_systab *uv_systab;
-s64 uv_bios_call(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, u64 a4, u64 a5) +static s64 __uv_bios_call(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, + u64 a4, u64 a5) { struct uv_systab *tab = uv_systab; s64 ret; @@ -51,6 +52,19 @@ s64 uv_bios_call(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, u64 a4, u64 a5)
return ret; } + +s64 uv_bios_call(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, u64 a4, u64 a5) +{ + s64 ret; + + if (down_interruptible(&__efi_uv_runtime_lock)) + return BIOS_STATUS_ABORT; + + ret = __uv_bios_call(which, a1, a2, a3, a4, a5); + up(&__efi_uv_runtime_lock); + + return ret; +} EXPORT_SYMBOL_GPL(uv_bios_call);
s64 uv_bios_call_irqsave(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, @@ -59,10 +73,15 @@ s64 uv_bios_call_irqsave(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, unsigned long bios_flags; s64 ret;
+ if (down_interruptible(&__efi_uv_runtime_lock)) + return BIOS_STATUS_ABORT; + local_irq_save(bios_flags); - ret = uv_bios_call(which, a1, a2, a3, a4, a5); + ret = __uv_bios_call(which, a1, a2, a3, a4, a5); local_irq_restore(bios_flags);
+ up(&__efi_uv_runtime_lock); + return ret; }
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c index 8903b9ccfc2b..e2abfdb5cee6 100644 --- a/drivers/firmware/efi/runtime-wrappers.c +++ b/drivers/firmware/efi/runtime-wrappers.c @@ -146,6 +146,13 @@ void efi_call_virt_check_flags(unsigned long flags, const char *call) */ static DEFINE_SEMAPHORE(efi_runtime_lock);
+/* + * Expose the EFI runtime lock to the UV platform + */ +#ifdef CONFIG_X86_UV +extern struct semaphore __efi_uv_runtime_lock __alias(efi_runtime_lock); +#endif + /* * Calls the appropriate efi_runtime_service() with the appropriate * arguments.
On Wed, 13 Feb 2019 at 20:34, Hedi Berriche hedi.berriche@hpe.com wrote:
- Changes since v2 Addressed comments from Ard Biesheuvel:
- expose efi_runtime_lock to UV platform only instead of globally
- remove unnecessary #ifdef CONFIG_EFI from bios_uv.c
- Changes since v1: Addressed comments from Bhupesh Sharma, Thomas Gleixner, and Ard Biesheuvel:
- made __uv_bios_call() static
- moved the efi_enabled() cleanup to its own patchlet
- explained the reason for renaming the efi_runtime_lock semaphore
- dropped the reviewed-bys as they should be given on the mailing list
- Cc'ng stable@vger.kernel.org given the nature of the problem addressed by the patches
Hi Hedi,
The patches look good to me now.
For the series,
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
However, I don't think all the patches should go to -stable. Only 4/4 fixes an actual bug, and the remaining patches don't look like they are prerequisites for that change.
Also, if your colleagues reviewed your patches, now would be the time to ask them to give their Reviewed-by as well.
On Thu, Feb 14, 2019 at 09:17:37AM +0100, Ard Biesheuvel wrote:
On Wed, 13 Feb 2019 at 20:34, Hedi Berriche hedi.berriche@hpe.com wrote:
- Changes since v2 Addressed comments from Ard Biesheuvel:
- expose efi_runtime_lock to UV platform only instead of globally
- remove unnecessary #ifdef CONFIG_EFI from bios_uv.c
- Changes since v1: Addressed comments from Bhupesh Sharma, Thomas Gleixner, and Ard Biesheuvel:
- made __uv_bios_call() static
- moved the efi_enabled() cleanup to its own patchlet
- explained the reason for renaming the efi_runtime_lock semaphore
- dropped the reviewed-bys as they should be given on the mailing list
- Cc'ng stable@vger.kernel.org given the nature of the problem addressed by the patches
Hi Hedi,
The patches look good to me now.
For the series,
Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org
However, I don't think all the patches should go to -stable. Only 4/4 fixes an actual bug, and the remaining patches don't look like they are prerequisites for that change.
Also, if your colleagues reviewed your patches, now would be the time to ask them to give their Reviewed-by as well.
Reviewed-by: Russ Anderson rja@hpe.com
Thanks.
-- Ard.
Calls into UV BIOS were not being serialised which is wrong as it violates EFI runtime rules, and bad as it does result in all sorts of potentially hard to track down hangs and panics when efi_scratch.prev_mm gets clobbered whenever efi_switch_mm() gets called concurrently from two different CPUs.
Patch #1 removes an unnecessary #ifdef CONFIG_EFI guard from bios_uv.c.
Patch #2 removes uv_bios_call_reentrant() because it's dead code.
Patch #3 is a cleanup that drops test_bit() in favour of the ad hoc efi_enabled().
Patch #4 makes uv_bios_call() variants use the efi_runtime_lock semaphore to protect against concurrency.
Cc: Russ Anderson rja@hpe.com Cc: Mike Travis mike.travis@hpe.com Cc: Dimitri Sivanich sivanich@hpe.com Cc: Steve Wahl steve.wahl@hpe.com Cc: stable@vger.kernel.org # v4.9+
Hedi Berriche (4): x86/platform/UV: remove unnecessary #ifdef CONFIG_EFI x86/platform/UV: kill uv_bios_call_reentrant() as it has no callers x86/platform/UV: use efi_enabled() instead of test_bit() x86/platform/UV: use efi_runtime_lock to serialise BIOS calls
arch/x86/include/asm/uv/bios.h | 13 ++++----- arch/x86/platform/uv/bios_uv.c | 35 ++++++++++++++----------- drivers/firmware/efi/runtime-wrappers.c | 7 +++++ 3 files changed, 34 insertions(+), 21 deletions(-)
-- 2.20.1
For the series:
Reviewed-by: Dimitri Sivanich sivanich@hpe.com
On Wed, Feb 13, 2019 at 07:34:09PM +0000, Hedi Berriche wrote:
- Changes since v2 Addressed comments from Ard Biesheuvel:
- expose efi_runtime_lock to UV platform only instead of globally
- remove unnecessary #ifdef CONFIG_EFI from bios_uv.c
- Changes since v1: Addressed comments from Bhupesh Sharma, Thomas Gleixner, and Ard Biesheuvel:
- made __uv_bios_call() static
- moved the efi_enabled() cleanup to its own patchlet
- explained the reason for renaming the efi_runtime_lock semaphore
- dropped the reviewed-bys as they should be given on the mailing list
- Cc'ng stable@vger.kernel.org given the nature of the problem addressed by the patches
Calls into UV BIOS were not being serialised which is wrong as it violates EFI runtime rules, and bad as it does result in all sorts of potentially hard to track down hangs and panics when efi_scratch.prev_mm gets clobbered whenever efi_switch_mm() gets called concurrently from two different CPUs.
Patch #1 removes an unnecessary #ifdef CONFIG_EFI guard from bios_uv.c.
Patch #2 removes uv_bios_call_reentrant() because it's dead code.
Patch #3 is a cleanup that drops test_bit() in favour of the ad hoc efi_enabled().
Patch #4 makes uv_bios_call() variants use the efi_runtime_lock semaphore to protect against concurrency.
Cc: Russ Anderson rja@hpe.com Cc: Mike Travis mike.travis@hpe.com Cc: Dimitri Sivanich sivanich@hpe.com Cc: Steve Wahl steve.wahl@hpe.com Cc: stable@vger.kernel.org # v4.9+
Hedi Berriche (4): x86/platform/UV: remove unnecessary #ifdef CONFIG_EFI x86/platform/UV: kill uv_bios_call_reentrant() as it has no callers x86/platform/UV: use efi_enabled() instead of test_bit() x86/platform/UV: use efi_runtime_lock to serialise BIOS calls
arch/x86/include/asm/uv/bios.h | 13 ++++----- arch/x86/platform/uv/bios_uv.c | 35 ++++++++++++++----------- drivers/firmware/efi/runtime-wrappers.c | 7 +++++ 3 files changed, 34 insertions(+), 21 deletions(-)
-- 2.20.1
On 2/14/2019 1:21 PM, Dimitri Sivanich wrote:
For the series:
Reviewed-by: Dimitri Sivanich sivanich@hpe.com
As well as I:
Reviewed-by: Mike Travis mike.travis@hpe.com
On Wed, Feb 13, 2019 at 07:34:09PM +0000, Hedi Berriche wrote:
Changes since v2 Addressed comments from Ard Biesheuvel:
- expose efi_runtime_lock to UV platform only instead of globally
- remove unnecessary #ifdef CONFIG_EFI from bios_uv.c
Changes since v1: Addressed comments from Bhupesh Sharma, Thomas Gleixner, and Ard Biesheuvel:
- made __uv_bios_call() static
- moved the efi_enabled() cleanup to its own patchlet
- explained the reason for renaming the efi_runtime_lock semaphore
- dropped the reviewed-bys as they should be given on the mailing list
- Cc'ng stable@vger.kernel.org given the nature of the problem addressed by the patches
Calls into UV BIOS were not being serialised which is wrong as it violates EFI runtime rules, and bad as it does result in all sorts of potentially hard to track down hangs and panics when efi_scratch.prev_mm gets clobbered whenever efi_switch_mm() gets called concurrently from two different CPUs.
Patch #1 removes an unnecessary #ifdef CONFIG_EFI guard from bios_uv.c.
Patch #2 removes uv_bios_call_reentrant() because it's dead code.
Patch #3 is a cleanup that drops test_bit() in favour of the ad hoc efi_enabled().
Patch #4 makes uv_bios_call() variants use the efi_runtime_lock semaphore to protect against concurrency.
Cc: Russ Anderson rja@hpe.com Cc: Mike Travis mike.travis@hpe.com Cc: Dimitri Sivanich sivanich@hpe.com Cc: Steve Wahl steve.wahl@hpe.com Cc: stable@vger.kernel.org # v4.9+
Hedi Berriche (4): x86/platform/UV: remove unnecessary #ifdef CONFIG_EFI x86/platform/UV: kill uv_bios_call_reentrant() as it has no callers x86/platform/UV: use efi_enabled() instead of test_bit() x86/platform/UV: use efi_runtime_lock to serialise BIOS calls
arch/x86/include/asm/uv/bios.h | 13 ++++----- arch/x86/platform/uv/bios_uv.c | 35 ++++++++++++++----------- drivers/firmware/efi/runtime-wrappers.c | 7 +++++ 3 files changed, 34 insertions(+), 21 deletions(-)
-- 2.20.1
linux-stable-mirror@lists.linaro.org
-
Ard Biesheuvel -
Dimitri Sivanich -
Hedi Berriche -
Mike Travis -
Russ Anderson