Re: [Patch v2 1/4] efi/x86: turn EFI runtime semaphore into a global lock
On Tue, Feb 12, 2019 at 17:25 Hedi Berriche wrote:
On Thu, Feb 07, 2019 at 17:38 Hedi Berriche wrote:
On Thu, Feb 07, 2019 at 16:05 Ard Biesheuvel wrote:
On Thu, 7 Feb 2019 at 05:23, Hedi Berriche wrote:
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
Hello Hedi,
Hi Ard,
Same feedback as on v1: please don't rename the lock.
With the efi_runtime_lock semaphore being no longer static, not renaming it
will cause a compile failure as it will clash with the declaration of the
efi_runtime_lock *spinlock* in arch/x86/platform/efi/efi_64.c in the
CONFIG_EFI_MIXED case.
Ard,
Any comments on whether resolving the conflict between
{efi_runtime_lock, drivers/firmware/efi/runtime-wrappers.c}
and
{efi_runtime_lock, arch/x86/platform/efi/efi_64.c}
provides the required justification for renaming the efi_runtime_lock semaphore?
Ard,
Apologies for sending this email which must have come across as absurd given
the *second* email you sent on 2019-02-07 19:38:42.
The trouble is that I *never* received that email (nor the one you sent today
2019-02-12 17:26:06 as reply to this one) because for some reason my email
address was dropped from the distribution list.
It's only about 30 minutes ago that a colleague brought it up to my attention
and forwarded both emails:
Thu, 7 Feb 2019 20:38:42 +0100
Tue, 12 Feb 2019 18:26:06 +0100
No idea how my email address got dropped but I wanted to make sure to explain
the seemingly absurd email I sent today as well as the lack of comment on your
earlier email.
Cheers,
Hedi.
---
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);
+
Re: [Patch v2 1/4] efi/x86: turn EFI runtime semaphore into a global lock
On Tue, 12 Feb 2019 at 18:25, Hedi Berriche wrote:
>
> On Thu, Feb 07, 2019 at 17:38 Hedi Berriche wrote:
> >On Thu, Feb 07, 2019 at 16:05 Ard Biesheuvel wrote:
> >>On Thu, 7 Feb 2019 at 05:23, Hedi Berriche wrote:
> >>>
> >>>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
> >>
> >>Hello Hedi,
> >
> >Hi Ard,
> >
> >>Same feedback as on v1: please don't rename the lock.
> >
> >With the efi_runtime_lock semaphore being no longer static, not renaming it
> >will cause a compile failure as it will clash with the declaration of the
> >efi_runtime_lock *spinlock* in arch/x86/platform/efi/efi_64.c in the
> >CONFIG_EFI_MIXED case.
>
> Ard,
>
> Any comments on whether resolving the conflict between
>
> {efi_runtime_lock, drivers/firmware/efi/runtime-wrappers.c}
>
> and
> {efi_runtime_lock, arch/x86/platform/efi/efi_64.c}
>
> provides the required justification for renaming the efi_runtime_lock
> semaphore?
>
Hello Hedi,
No it doesn't. I responded 5 days ago with a suggestion on how to
address this instead.
> Cheers,
> Hedi.
>
> >>>---
> >>> 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,
> >>>
Re: [Patch v2 1/4] efi/x86: turn EFI runtime semaphore into a global lock
On Thu, Feb 07, 2019 at 17:38 Hedi Berriche wrote:
On Thu, Feb 07, 2019 at 16:05 Ard Biesheuvel wrote:
On Thu, 7 Feb 2019 at 05:23, Hedi Berriche wrote:
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
Hello Hedi,
Hi Ard,
Same feedback as on v1: please don't rename the lock.
With the efi_runtime_lock semaphore being no longer static, not renaming it
will cause a compile failure as it will clash with the declaration of the
efi_runtime_lock *spinlock* in arch/x86/platform/efi/efi_64.c in the
CONFIG_EFI_MIXED case.
Ard,
Any comments on whether resolving the conflict between
{efi_runtime_lock, drivers/firmware/efi/runtime-wrappers.c}
and
{efi_runtime_lock, arch/x86/platform/efi/efi_64.c}
provides the required justification for renaming the efi_runtime_lock semaphore?
Cheers,
Hedi.
---
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;
Re: [Patch v2 1/4] efi/x86: turn EFI runtime semaphore into a global lock
On Thu, 7 Feb 2019 at 18:38, Hedi Berriche wrote:
>
> On Thu, Feb 07, 2019 at 16:05 Ard Biesheuvel wrote:
> >On Thu, 7 Feb 2019 at 05:23, Hedi Berriche wrote:
> >>
> >> 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
> >
> >Hello Hedi,
>
> Hi Ard,
>
> >Same feedback as on v1: please don't rename the lock.
>
> With the efi_runtime_lock semaphore being no longer static, not renaming it
> will cause a compile failure as it will clash with the declaration of the
> efi_runtime_lock *spinlock* in arch/x86/platform/efi/efi_64.c in the
> CONFIG_EFI_MIXED case.
>
Ah right, it's in the commit log, as requested. Apologies for not spotting that.
Given how UV is a bit of an outlier, I'd prefer it if we could make
this functionality a bit more light weight, by only exposing the lock
if needed, and not create a global symbol for an internal lock if we
can avoid it.
Could you please try something along the lines of
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -146,6 +146,10 @@
*/
static DEFINE_SEMAPHORE(efi_runtime_lock);
+#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.
and add a declaration
extern struct semaphore __efi_uv_runtime_lock;
to bios_uv.c itself rather than to a header file? You can combine this
with your changes in patch #4 (and drop the first patch entirely)
Also, I noticed that there is some CONFIG_EFI guarded code in
bios_uv.c while the whole file already depends on CONFIG_EFI via the
Kconfig symbol. Perhaps you could include a patch to clean that up as
well? Thanks.
> >> ---
> >> 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;
> >>
Re: [Patch v2 1/4] efi/x86: turn EFI runtime semaphore into a global lock
On Thu, Feb 07, 2019 at 16:05 Ard Biesheuvel wrote:
On Thu, 7 Feb 2019 at 05:23, Hedi Berriche wrote:
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
Hello Hedi,
Hi Ard,
Same feedback as on v1: please don't rename the lock.
With the efi_runtime_lock semaphore being no longer static, not renaming it
will cause a compile failure as it will clash with the declaration of the
efi_runtime_lock *spinlock* in arch/x86/platform/efi/efi_64.c in the
CONFIG_EFI_MIXED case.
---
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,
{
Re: [Patch v2 1/4] efi/x86: turn EFI runtime semaphore into a global lock
On Thu, 7 Feb 2019 at 05:23, Hedi Berriche wrote:
>
> 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
Hello Hedi,
Same feedback as on v1: please don't rename the lock.
> ---
> 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
[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,
