[Patch v3 0/4] Protect against concurrent calls into UV BIOS

[Patch v3 0/4] Protect against concurrent calls into UV BIOS

[Hedi Berriche]

- 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

[Patch v3 1/4] x86/platform/UV: remove unnecessary #ifdef CONFIG_EFI

[Hedi Berriche]

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

[tip:x86/uv] x86/platform/UV: Remove unnecessary #ifdef CONFIG_EFI

[tip-bot for Hedi Berriche]

Commit-ID:  30ad3e031d2feae075cd5fd2c443baa2d86c0195
Gitweb:     https://git.kernel.org/tip/30ad3e031d2feae075cd5fd2c443baa2d86c0195
Author:     Hedi Berriche <hedi.berriche@hpe.com>
AuthorDate: Wed, 13 Feb 2019 19:34:10 +0000
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Fri, 15 Feb 2019 15:05:15 +0100

x86/platform/UV: Remove unnecessary #ifdef CONFIG_EFI

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.

Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Russ Anderson <rja@hpe.com>
Reviewed-by: Dimitri Sivanich <sivanich@hpe.com>
Reviewed-by: Mike Travis <mike.travis@hpe.com>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: linux-efi <linux-efi@vger.kernel.org>
Cc: platform-driver-x86@vger.kernel.org
Cc: Steve Wahl <steve.wahl@hpe.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/20190213193413.25560-2-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

[Patch v3 2/4] x86/platform/UV: kill uv_bios_call_reentrant()

[Hedi Berriche]

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

[tip:x86/uv] x86/platform/UV: Remove uv_bios_call_reentrant()

[tip-bot for Hedi Berriche]

Commit-ID:  f816525d615fff0336d0c881e5d960bbec5ea016
Gitweb:     https://git.kernel.org/tip/f816525d615fff0336d0c881e5d960bbec5ea016
Author:     Hedi Berriche <hedi.berriche@hpe.com>
AuthorDate: Wed, 13 Feb 2019 19:34:11 +0000
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Fri, 15 Feb 2019 15:13:48 +0100

x86/platform/UV: Remove uv_bios_call_reentrant()

uv_bios_call_reentrant() has no callers nor is it exported, remove it.

Cleanup, no functional changes.

Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Russ Anderson <rja@hpe.com>
Reviewed-by: Dimitri Sivanich <sivanich@hpe.com>
Reviewed-by: Mike Travis <mike.travis@hpe.com>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: linux-efi <linux-efi@vger.kernel.org>
Cc: platform-driver-x86@vger.kernel.org
Cc: Steve Wahl <steve.wahl@hpe.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/20190213193413.25560-3-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);

[Patch v3 3/4] x86/platform/UV: use efi_enabled() instead of test_bit()

[Hedi Berriche]

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

[tip:x86/uv] x86/platform/UV: Use efi_enabled() instead of test_bit()

[tip-bot for Hedi Berriche]

Commit-ID:  8945d96f7b3ead56e053ac79b8f7b0de98a30bfe
Gitweb:     https://git.kernel.org/tip/8945d96f7b3ead56e053ac79b8f7b0de98a30bfe
Author:     Hedi Berriche <hedi.berriche@hpe.com>
AuthorDate: Wed, 13 Feb 2019 19:34:12 +0000
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Fri, 15 Feb 2019 15:15:18 +0100

x86/platform/UV: Use efi_enabled() instead of test_bit()

Use ad hoc efi_enabled() instead of fiddling with test_bit().

Cleanup, no functional changes.

Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Russ Anderson <rja@hpe.com>
Reviewed-by: Dimitri Sivanich <sivanich@hpe.com>
Reviewed-by: Mike Travis <mike.travis@hpe.com>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: linux-efi <linux-efi@vger.kernel.org>
Cc: platform-driver-x86@vger.kernel.org
Cc: Steve Wahl <steve.wahl@hpe.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/20190213193413.25560-4-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);

[Patch v3 4/4] x86/platform/UV: use efi_runtime_lock to serialise BIOS calls

[Hedi Berriche]

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

[tip:x86/urgent] x86/platform/UV: Use efi_runtime_lock to serialise BIOS calls

[tip-bot for Hedi Berriche]

Commit-ID:  f331e766c4be33f4338574f3c9f7f77e98ab4571
Gitweb:     https://git.kernel.org/tip/f331e766c4be33f4338574f3c9f7f77e98ab4571
Author:     Hedi Berriche <hedi.berriche@hpe.com>
AuthorDate: Wed, 13 Feb 2019 19:34:13 +0000
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Fri, 15 Feb 2019 15:19:56 +0100

x86/platform/UV: Use efi_runtime_lock to serialise BIOS calls

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.

Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Russ Anderson <rja@hpe.com>
Reviewed-by: Dimitri Sivanich <sivanich@hpe.com>
Reviewed-by: Mike Travis <mike.travis@hpe.com>
Cc: Andy Shevchenko <andy@infradead.org>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: linux-efi <linux-efi@vger.kernel.org>
Cc: platform-driver-x86@vger.kernel.org
Cc: stable@vger.kernel.org # v4.9+
Cc: Steve Wahl <steve.wahl@hpe.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/20190213193413.25560-5-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 e652a7cc6186..3f697a9e3f59 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 */
@@ -167,4 +168,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 4a6a5a26c582..eb33432f2f24 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.

Re: [Patch v3 0/4] Protect against concurrent calls into UV BIOS

[Ard Biesheuvel]

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
>

Re: [Patch v3 0/4] Protect against concurrent calls into UV BIOS

[Russ Anderson]

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

Re: [Patch v3 0/4] Protect against concurrent calls into UV BIOS

[Dimitri Sivanich]

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
> 

Re: [Patch v3 0/4] Protect against concurrent calls into UV BIOS

[Mike Travis]

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