stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Patch v3 0/4] Protect against concurrent calls into UV BIOS
@ 2019-02-13 19:34 Hedi Berriche
  2019-02-13 19:34 ` [Patch v3 1/4] x86/platform/UV: remove unnecessary #ifdef CONFIG_EFI Hedi Berriche
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Hedi Berriche @ 2019-02-13 19:34 UTC (permalink / raw)
  To: linux-kernel, Ard Biesheuvel, Thomas Gleixner, Bhupesh Sharma
  Cc: Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-efi, x86,
	Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl, stable

- 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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Patch v3 1/4] x86/platform/UV: remove unnecessary #ifdef CONFIG_EFI
  2019-02-13 19:34 [Patch v3 0/4] Protect against concurrent calls into UV BIOS Hedi Berriche
@ 2019-02-13 19:34 ` Hedi Berriche
  2019-02-13 19:34 ` [Patch v3 2/4] x86/platform/UV: kill uv_bios_call_reentrant() Hedi Berriche
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Hedi Berriche @ 2019-02-13 19:34 UTC (permalink / raw)
  To: linux-kernel, Ard Biesheuvel, Thomas Gleixner, Bhupesh Sharma
  Cc: Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-efi, x86,
	Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl, stable

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
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Patch v3 2/4] x86/platform/UV: kill uv_bios_call_reentrant()
  2019-02-13 19:34 [Patch v3 0/4] Protect against concurrent calls into UV BIOS Hedi Berriche
  2019-02-13 19:34 ` [Patch v3 1/4] x86/platform/UV: remove unnecessary #ifdef CONFIG_EFI Hedi Berriche
@ 2019-02-13 19:34 ` Hedi Berriche
  2019-02-13 19:34 ` [Patch v3 3/4] x86/platform/UV: use efi_enabled() instead of test_bit() Hedi Berriche
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Hedi Berriche @ 2019-02-13 19:34 UTC (permalink / raw)
  To: linux-kernel, Ard Biesheuvel, Thomas Gleixner, Bhupesh Sharma
  Cc: Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-efi, x86,
	Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl, stable

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);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Patch v3 3/4] x86/platform/UV: use efi_enabled() instead of test_bit()
  2019-02-13 19:34 [Patch v3 0/4] Protect against concurrent calls into UV BIOS Hedi Berriche
  2019-02-13 19:34 ` [Patch v3 1/4] x86/platform/UV: remove unnecessary #ifdef CONFIG_EFI Hedi Berriche
  2019-02-13 19:34 ` [Patch v3 2/4] x86/platform/UV: kill uv_bios_call_reentrant() Hedi Berriche
@ 2019-02-13 19:34 ` Hedi Berriche
  2019-02-13 19:34 ` [Patch v3 4/4] x86/platform/UV: use efi_runtime_lock to serialise BIOS calls Hedi Berriche
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Hedi Berriche @ 2019-02-13 19:34 UTC (permalink / raw)
  To: linux-kernel, Ard Biesheuvel, Thomas Gleixner, Bhupesh Sharma
  Cc: Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-efi, x86,
	Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl, stable

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);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Patch v3 4/4] x86/platform/UV: use efi_runtime_lock to serialise BIOS calls
  2019-02-13 19:34 [Patch v3 0/4] Protect against concurrent calls into UV BIOS Hedi Berriche
                   ` (2 preceding siblings ...)
  2019-02-13 19:34 ` [Patch v3 3/4] x86/platform/UV: use efi_enabled() instead of test_bit() Hedi Berriche
@ 2019-02-13 19:34 ` Hedi Berriche
  2019-02-14  8:17 ` [Patch v3 0/4] Protect against concurrent calls into UV BIOS Ard Biesheuvel
  2019-02-14 21:21 ` Dimitri Sivanich
  5 siblings, 0 replies; 9+ messages in thread
From: Hedi Berriche @ 2019-02-13 19:34 UTC (permalink / raw)
  To: linux-kernel, Ard Biesheuvel, Thomas Gleixner, Bhupesh Sharma
  Cc: Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-efi, x86,
	Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl, stable

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.
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Patch v3 0/4] Protect against concurrent calls into UV BIOS
  2019-02-13 19:34 [Patch v3 0/4] Protect against concurrent calls into UV BIOS Hedi Berriche
                   ` (3 preceding siblings ...)
  2019-02-13 19:34 ` [Patch v3 4/4] x86/platform/UV: use efi_runtime_lock to serialise BIOS calls Hedi Berriche
@ 2019-02-14  8:17 ` Ard Biesheuvel
  2019-02-14 21:08   ` Russ Anderson
  2019-02-14 21:21 ` Dimitri Sivanich
  5 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2019-02-14  8:17 UTC (permalink / raw)
  To: Hedi Berriche, Ingo Molnar, Thomas Gleixner, Borislav Petkov
  Cc: Linux Kernel Mailing List, Bhupesh Sharma, H. Peter Anvin,
	linux-efi, the arch/x86 maintainers, Russ Anderson, Mike Travis,
	Dimitri Sivanich, Steve Wahl, stable

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.

-- 
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
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch v3 0/4] Protect against concurrent calls into UV BIOS
  2019-02-14  8:17 ` [Patch v3 0/4] Protect against concurrent calls into UV BIOS Ard Biesheuvel
@ 2019-02-14 21:08   ` Russ Anderson
  0 siblings, 0 replies; 9+ messages in thread
From: Russ Anderson @ 2019-02-14 21:08 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Hedi Berriche, Ingo Molnar, Thomas Gleixner, Borislav Petkov,
	Linux Kernel Mailing List, Bhupesh Sharma, H. Peter Anvin,
	linux-efi, the arch/x86 maintainers, Mike Travis,
	Dimitri Sivanich, Steve Wahl, stable

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
> >

-- 
Russ Anderson,  SuperDome Flex Linux Kernel Group Manager
HPE - Hewlett Packard Enterprise (formerly SGI)  rja@hpe.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch v3 0/4] Protect against concurrent calls into UV BIOS
  2019-02-13 19:34 [Patch v3 0/4] Protect against concurrent calls into UV BIOS Hedi Berriche
                   ` (4 preceding siblings ...)
  2019-02-14  8:17 ` [Patch v3 0/4] Protect against concurrent calls into UV BIOS Ard Biesheuvel
@ 2019-02-14 21:21 ` Dimitri Sivanich
  2019-02-14 21:31   ` Mike Travis
  5 siblings, 1 reply; 9+ messages in thread
From: Dimitri Sivanich @ 2019-02-14 21:21 UTC (permalink / raw)
  To: Hedi Berriche
  Cc: linux-kernel, Ard Biesheuvel, Thomas Gleixner, Bhupesh Sharma,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-efi, x86,
	Russ Anderson, Mike Travis, Steve Wahl, stable

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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch v3 0/4] Protect against concurrent calls into UV BIOS
  2019-02-14 21:21 ` Dimitri Sivanich
@ 2019-02-14 21:31   ` Mike Travis
  0 siblings, 0 replies; 9+ messages in thread
From: Mike Travis @ 2019-02-14 21:31 UTC (permalink / raw)
  To: Dimitri Sivanich, Hedi Berriche
  Cc: linux-kernel, Ard Biesheuvel, Thomas Gleixner, Bhupesh Sharma,
	Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-efi, x86,
	Russ Anderson, Steve Wahl, stable



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
>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-02-14 21:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-13 19:34 [Patch v3 0/4] Protect against concurrent calls into UV BIOS Hedi Berriche
2019-02-13 19:34 ` [Patch v3 1/4] x86/platform/UV: remove unnecessary #ifdef CONFIG_EFI Hedi Berriche
2019-02-13 19:34 ` [Patch v3 2/4] x86/platform/UV: kill uv_bios_call_reentrant() Hedi Berriche
2019-02-13 19:34 ` [Patch v3 3/4] x86/platform/UV: use efi_enabled() instead of test_bit() Hedi Berriche
2019-02-13 19:34 ` [Patch v3 4/4] x86/platform/UV: use efi_runtime_lock to serialise BIOS calls Hedi Berriche
2019-02-14  8:17 ` [Patch v3 0/4] Protect against concurrent calls into UV BIOS Ard Biesheuvel
2019-02-14 21:08   ` Russ Anderson
2019-02-14 21:21 ` Dimitri Sivanich
2019-02-14 21:31   ` Mike Travis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).