[Patch v2 2/4] x86/platform/UV: kill uv_bios_call_reentrant() as it has no callers
uv_bios_call_reentrant() has no callers nor is it exported, kill it.
Cc: Russ Anderson
Cc: Mike Travis
Cc: Dimitri Sivanich
Cc: Steve Wahl
Cc: sta...@vger.kernel.org
Signed-off-by: Hedi Berriche
---
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 e652a7cc6186..4eee646544b2 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 4a6a5a26c582..cd05af157763 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
[Patch v2 4/4] x86/platform/UV: use efi_runtime_sem to serialise BIOS calls
Calls into UV firmware must be protected against concurrency, use the
now visible efi_runtime_sem lock to serialise them.
Cc: Russ Anderson
Cc: Mike Travis
Cc: Dimitri Sivanich
Cc: Steve Wahl
Cc: sta...@vger.kernel.org
Signed-off-by: Hedi Berriche
---
arch/x86/include/asm/uv/bios.h | 3 ++-
arch/x86/platform/uv/bios_uv.c | 23 +--
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/uv/bios.h b/arch/x86/include/asm/uv/bios.h
index 4eee646544b2..581e2978a16c 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 */
diff --git a/arch/x86/platform/uv/bios_uv.c b/arch/x86/platform/uv/bios_uv.c
index 8bace0ca9e57..e779b2a128ea 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(_runtime_sem))
+ return BIOS_STATUS_ABORT;
+
+ ret = __uv_bios_call(which, a1, a2, a3, a4, a5);
+ up(_runtime_sem);
+
+ 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(_runtime_sem))
+ 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(_runtime_sem);
+
return ret;
}
--
2.20.1
[Patch v2 0/4] Protect against concurrent calls into UV BIOS
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 sta...@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 makes the efi_runtime_lock semaphore visible to EFI runtime callers defined outside drivers/firmware/efi/runtime-wrappers.c in preparation for using it to serialise calls into UV BIOS; the lock is also renamed to efi_runtime_sem so that it can coexist with the efi_runtime_lock spinlock defined in arch/x86/platform/efi/efi_64.c when CONFIG_EFI_MIXED is enabled. 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 efi_runtime_sem to protect against concurrency. Cc: Russ Anderson Cc: Mike Travis Cc: Dimitri Sivanich Cc: Steve Wahl Cc: sta...@vger.kernel.org Hedi Berriche (4): efi/x86: turn EFI runtime semaphore into a global lock 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_sem to serialise BIOS calls arch/x86/include/asm/uv/bios.h | 4 +- arch/x86/platform/uv/bios_uv.c | 33 -- drivers/firmware/efi/runtime-wrappers.c | 60 - include/linux/efi.h | 3 ++ 4 files changed, 55 insertions(+), 45 deletions(-) -- 2.20.1
[Patch v2 1/4] efi/x86: turn EFI runtime semaphore into a global lock
Make efi_runtime_lock semaphore global so that it can be used by EFI
runtime callers that may be defined outside efi/runtime-wrappers.c.
Also now that efi_runtime_lock semaphore is no longer static, rename it
to efi_runtime_sem so it doesn't clash with the efi_runtime_lock spinlock
defined in arch/x86/platform/efi/efi_64.c when CONFIG_EFI_MIXED is
enabled.
The immediate motivation of this change is to serialise UV platform BIOS
calls using the efi_runtime_sem semaphore.
No functional changes.
Cc: Russ Anderson
Cc: Mike Travis
Cc: Dimitri Sivanich
Cc: Steve Wahl
Cc: sta...@vger.kernel.org
Signed-off-by: Hedi Berriche
---
drivers/firmware/efi/runtime-wrappers.c | 60 -
include/linux/efi.h | 3 ++
2 files changed, 33 insertions(+), 30 deletions(-)
diff --git a/drivers/firmware/efi/runtime-wrappers.c
b/drivers/firmware/efi/runtime-wrappers.c
index 8903b9ccfc2b..ec60d6227925 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -53,7 +53,7 @@ struct efi_runtime_work efi_rts_work;
* @rts_arg<1-5>: efi_runtime_service() function arguments
*
* Accesses to efi_runtime_services() are serialized by a binary
- * semaphore (efi_runtime_lock) and caller waits until the work is
+ * semaphore (efi_runtime_sem) and caller waits until the work is
* finished, hence _only_ one work is queued at a time and the caller
* thread waits for completion.
*/
@@ -144,7 +144,7 @@ void efi_call_virt_check_flags(unsigned long flags, const
char *call)
* none of the remaining functions are actually ever called at runtime.
* So let's just use a single lock to serialize all Runtime Services calls.
*/
-static DEFINE_SEMAPHORE(efi_runtime_lock);
+DEFINE_SEMAPHORE(efi_runtime_sem);
/*
* Calls the appropriate efi_runtime_service() with the appropriate
@@ -233,10 +233,10 @@ static efi_status_t virt_efi_get_time(efi_time_t *tm,
efi_time_cap_t *tc)
{
efi_status_t status;
- if (down_interruptible(_runtime_lock))
+ if (down_interruptible(_runtime_sem))
return EFI_ABORTED;
status = efi_queue_work(GET_TIME, tm, tc, NULL, NULL, NULL);
- up(_runtime_lock);
+ up(_runtime_sem);
return status;
}
@@ -244,10 +244,10 @@ static efi_status_t virt_efi_set_time(efi_time_t *tm)
{
efi_status_t status;
- if (down_interruptible(_runtime_lock))
+ if (down_interruptible(_runtime_sem))
return EFI_ABORTED;
status = efi_queue_work(SET_TIME, tm, NULL, NULL, NULL, NULL);
- up(_runtime_lock);
+ up(_runtime_sem);
return status;
}
@@ -257,11 +257,11 @@ static efi_status_t virt_efi_get_wakeup_time(efi_bool_t
*enabled,
{
efi_status_t status;
- if (down_interruptible(_runtime_lock))
+ if (down_interruptible(_runtime_sem))
return EFI_ABORTED;
status = efi_queue_work(GET_WAKEUP_TIME, enabled, pending, tm, NULL,
NULL);
- up(_runtime_lock);
+ up(_runtime_sem);
return status;
}
@@ -269,11 +269,11 @@ static efi_status_t virt_efi_set_wakeup_time(efi_bool_t
enabled, efi_time_t *tm)
{
efi_status_t status;
- if (down_interruptible(_runtime_lock))
+ if (down_interruptible(_runtime_sem))
return EFI_ABORTED;
status = efi_queue_work(SET_WAKEUP_TIME, , tm, NULL, NULL,
NULL);
- up(_runtime_lock);
+ up(_runtime_sem);
return status;
}
@@ -285,11 +285,11 @@ static efi_status_t virt_efi_get_variable(efi_char16_t
*name,
{
efi_status_t status;
- if (down_interruptible(_runtime_lock))
+ if (down_interruptible(_runtime_sem))
return EFI_ABORTED;
status = efi_queue_work(GET_VARIABLE, name, vendor, attr, data_size,
data);
- up(_runtime_lock);
+ up(_runtime_sem);
return status;
}
@@ -299,11 +299,11 @@ static efi_status_t virt_efi_get_next_variable(unsigned
long *name_size,
{
efi_status_t status;
- if (down_interruptible(_runtime_lock))
+ if (down_interruptible(_runtime_sem))
return EFI_ABORTED;
status = efi_queue_work(GET_NEXT_VARIABLE, name_size, name, vendor,
NULL, NULL);
- up(_runtime_lock);
+ up(_runtime_sem);
return status;
}
@@ -315,11 +315,11 @@ static efi_status_t virt_efi_set_variable(efi_char16_t
*name,
{
efi_status_t status;
- if (down_interruptible(_runtime_lock))
+ if (down_interruptible(_runtime_sem))
return EFI_ABORTED;
status = efi_queue_work(SET_VARIABLE, name, vendor, , _size,
data);
- up(_runtime_lock);
+ up(_runtime_sem);
return status;
}
@@ -330,12 +330,12 @@ virt_efi_set_variable_nonblocking(efi_char16_t *name,
