* [PATCH 0/3] Protect against concurrent calls into UV BIOS
@ 2019-01-09 10:45 Hedi Berriche
2019-01-09 10:45 ` [PATCH 1/3] efi/x86: turn EFI runtime semaphore into a global lock Hedi Berriche
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Hedi Berriche @ 2019-01-09 10:45 UTC (permalink / raw)
To: Ard Biesheuvel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin
Cc: x86, linux-efi, linux-kernel, Hedi Berriche
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 gets clobbered.
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.
Patch #2 removes uv_bios_call_reentrant() because it's dead code.
Patch #3 makes uv_bios_call() variants use efi_runtime_sem to protect
against concurrency.
Hedi Berriche (3):
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_runtime_sem to serialise BIOS calls
arch/x86/include/asm/uv/bios.h | 4 +-
arch/x86/platform/uv/bios_uv.c | 37 +++++++++++--------
drivers/firmware/efi/runtime-wrappers.c | 60 ++++++++++++++++----------------
include/linux/efi.h | 3 +
4 files changed, 57 insertions(+), 47 deletions(-)
--
2.20.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] efi/x86: turn EFI runtime semaphore into a global lock 2019-01-09 10:45 [PATCH 0/3] Protect against concurrent calls into UV BIOS Hedi Berriche @ 2019-01-09 10:45 ` Hedi Berriche 2019-01-15 18:55 ` Thomas Gleixner 2019-01-26 14:03 ` Ard Biesheuvel 2019-01-09 10:45 ` [PATCH 2/3] x86/platform/UV: kill uv_bios_call_reentrant() as it has no callers Hedi Berriche 2019-01-09 10:45 ` [PATCH 3/3] x86/platform/UV: use efi_runtime_sem to serialise BIOS calls Hedi Berriche 2 siblings, 2 replies; 11+ messages in thread From: Hedi Berriche @ 2019-01-09 10:45 UTC (permalink / raw) To: Ard Biesheuvel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin Cc: x86, linux-efi, linux-kernel, Hedi Berriche, Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl 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. The immediate motivation is to piggy-back it to serialise UV platform BIOS calls. No functional changes. Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com> Reviewed-by: Russ Anderson <rja@hpe.com> Reviewed-by: Mike Travis <mike.travis@hpe.com> Reviewed-by: Dimitri Sivanich <sivanich@hpe.com> Reviewed-by: Steve Wahl <steve.wahl@hpe.com> --- 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(&efi_runtime_lock)) + if (down_interruptible(&efi_runtime_sem)) return EFI_ABORTED; status = efi_queue_work(GET_TIME, tm, tc, NULL, NULL, NULL); - up(&efi_runtime_lock); + up(&efi_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(&efi_runtime_lock)) + if (down_interruptible(&efi_runtime_sem)) return EFI_ABORTED; status = efi_queue_work(SET_TIME, tm, NULL, NULL, NULL, NULL); - up(&efi_runtime_lock); + up(&efi_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(&efi_runtime_lock)) + if (down_interruptible(&efi_runtime_sem)) return EFI_ABORTED; status = efi_queue_work(GET_WAKEUP_TIME, enabled, pending, tm, NULL, NULL); - up(&efi_runtime_lock); + up(&efi_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(&efi_runtime_lock)) + if (down_interruptible(&efi_runtime_sem)) return EFI_ABORTED; status = efi_queue_work(SET_WAKEUP_TIME, &enabled, tm, NULL, NULL, NULL); - up(&efi_runtime_lock); + up(&efi_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(&efi_runtime_lock)) + if (down_interruptible(&efi_runtime_sem)) return EFI_ABORTED; status = efi_queue_work(GET_VARIABLE, name, vendor, attr, data_size, data); - up(&efi_runtime_lock); + up(&efi_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(&efi_runtime_lock)) + if (down_interruptible(&efi_runtime_sem)) return EFI_ABORTED; status = efi_queue_work(GET_NEXT_VARIABLE, name_size, name, vendor, NULL, NULL); - up(&efi_runtime_lock); + up(&efi_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(&efi_runtime_lock)) + if (down_interruptible(&efi_runtime_sem)) return EFI_ABORTED; status = efi_queue_work(SET_VARIABLE, name, vendor, &attr, &data_size, data); - up(&efi_runtime_lock); + up(&efi_runtime_sem); return status; } @@ -330,12 +330,12 @@ virt_efi_set_variable_nonblocking(efi_char16_t *name, efi_guid_t *vendor, { efi_status_t status; - if (down_trylock(&efi_runtime_lock)) + if (down_trylock(&efi_runtime_sem)) return EFI_NOT_READY; status = efi_call_virt(set_variable, name, vendor, attr, data_size, data); - up(&efi_runtime_lock); + up(&efi_runtime_sem); return status; } @@ -350,11 +350,11 @@ static efi_status_t virt_efi_query_variable_info(u32 attr, if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; - if (down_interruptible(&efi_runtime_lock)) + if (down_interruptible(&efi_runtime_sem)) return EFI_ABORTED; status = efi_queue_work(QUERY_VARIABLE_INFO, &attr, storage_space, remaining_space, max_variable_size, NULL); - up(&efi_runtime_lock); + up(&efi_runtime_sem); return status; } @@ -369,12 +369,12 @@ virt_efi_query_variable_info_nonblocking(u32 attr, if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; - if (down_trylock(&efi_runtime_lock)) + if (down_trylock(&efi_runtime_sem)) return EFI_NOT_READY; status = efi_call_virt(query_variable_info, attr, storage_space, remaining_space, max_variable_size); - up(&efi_runtime_lock); + up(&efi_runtime_sem); return status; } @@ -382,11 +382,11 @@ static efi_status_t virt_efi_get_next_high_mono_count(u32 *count) { efi_status_t status; - if (down_interruptible(&efi_runtime_lock)) + if (down_interruptible(&efi_runtime_sem)) return EFI_ABORTED; status = efi_queue_work(GET_NEXT_HIGH_MONO_COUNT, count, NULL, NULL, NULL, NULL); - up(&efi_runtime_lock); + up(&efi_runtime_sem); return status; } @@ -395,14 +395,14 @@ static void virt_efi_reset_system(int reset_type, unsigned long data_size, efi_char16_t *data) { - if (down_interruptible(&efi_runtime_lock)) { + if (down_interruptible(&efi_runtime_sem)) { pr_warn("failed to invoke the reset_system() runtime service:\n" "could not get exclusive access to the firmware\n"); return; } efi_rts_work.efi_rts_id = RESET_SYSTEM; __efi_call_virt(reset_system, reset_type, status, data_size, data); - up(&efi_runtime_lock); + up(&efi_runtime_sem); } static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules, @@ -414,11 +414,11 @@ static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules, if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; - if (down_interruptible(&efi_runtime_lock)) + if (down_interruptible(&efi_runtime_sem)) return EFI_ABORTED; status = efi_queue_work(UPDATE_CAPSULE, capsules, &count, &sg_list, NULL, NULL); - up(&efi_runtime_lock); + up(&efi_runtime_sem); return status; } @@ -432,11 +432,11 @@ static efi_status_t virt_efi_query_capsule_caps(efi_capsule_header_t **capsules, if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; - if (down_interruptible(&efi_runtime_lock)) + if (down_interruptible(&efi_runtime_sem)) return EFI_ABORTED; status = efi_queue_work(QUERY_CAPSULE_CAPS, capsules, &count, max_size, reset_type, NULL); - up(&efi_runtime_lock); + up(&efi_runtime_sem); return status; } diff --git a/include/linux/efi.h b/include/linux/efi.h index 45ff763fba76..930cd20842b8 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -1745,6 +1745,9 @@ extern struct efi_runtime_work efi_rts_work; /* Workqueue to queue EFI Runtime Services */ extern struct workqueue_struct *efi_rts_wq; +/* EFI runtime semaphore */ +extern struct semaphore efi_runtime_sem; + struct linux_efi_memreserve { int size; // allocated size of the array atomic_t count; // number of entries used ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] efi/x86: turn EFI runtime semaphore into a global lock 2019-01-09 10:45 ` [PATCH 1/3] efi/x86: turn EFI runtime semaphore into a global lock Hedi Berriche @ 2019-01-15 18:55 ` Thomas Gleixner 2019-01-15 19:35 ` Hedi Berriche 2019-01-26 14:03 ` Ard Biesheuvel 1 sibling, 1 reply; 11+ messages in thread From: Thomas Gleixner @ 2019-01-15 18:55 UTC (permalink / raw) To: Hedi Berriche Cc: Ard Biesheuvel, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-efi, linux-kernel, Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl On Wed, 9 Jan 2019, 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. The changelog should mention why the lock is renamed. I have no strong opinion, but to apply that I need an Ack from Ard. Thanks, tglx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] efi/x86: turn EFI runtime semaphore into a global lock 2019-01-15 18:55 ` Thomas Gleixner @ 2019-01-15 19:35 ` Hedi Berriche 0 siblings, 0 replies; 11+ messages in thread From: Hedi Berriche @ 2019-01-15 19:35 UTC (permalink / raw) To: Thomas Gleixner Cc: Ard Biesheuvel, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-efi, linux-kernel, Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl On Tue, Jan 15, 2019 at 18:55 Thomas Gleixner wrote: >On Wed, 9 Jan 2019, 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. > >The changelog should mention why the lock is renamed. OK; will add to V2 that it was renamed so that it doesn't clash with the efi_runtime_lock spinlock defined by CONFIG_EFI_MIXED. >I have no strong opinion, but to apply that I need an Ack from Ard. OK; thanks for the feedback. Cheers, Hedi. -- Be careful of reading health books, you might die of a misprint. -- Mark Twain ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] efi/x86: turn EFI runtime semaphore into a global lock 2019-01-09 10:45 ` [PATCH 1/3] efi/x86: turn EFI runtime semaphore into a global lock Hedi Berriche 2019-01-15 18:55 ` Thomas Gleixner @ 2019-01-26 14:03 ` Ard Biesheuvel 1 sibling, 0 replies; 11+ messages in thread From: Ard Biesheuvel @ 2019-01-26 14:03 UTC (permalink / raw) To: Hedi Berriche Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers, linux-efi, Linux Kernel Mailing List, Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl Hello Hedi, On Wed, 9 Jan 2019 at 11:46, Hedi Berriche <hedi.berriche@hpe.com> 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. > > The immediate motivation is to piggy-back it to serialise UV platform BIOS > calls. > > No functional changes. > > Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com> > Reviewed-by: Russ Anderson <rja@hpe.com> > Reviewed-by: Mike Travis <mike.travis@hpe.com> > Reviewed-by: Dimitri Sivanich <sivanich@hpe.com> > Reviewed-by: Steve Wahl <steve.wahl@hpe.com> Please drop these reviewed-bys. They may be given on the mailing list, but having a whole list of R-bs from your co-workers on a first posting of a series doesn't make a lot of sense. > --- > 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); Please don't rename the lock unless there is a good reason for doing so. Your commit log does not even mention that the lock is renamed, so I am going to assume there is no good reason for it. > > /* > * 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(&efi_runtime_lock)) > + if (down_interruptible(&efi_runtime_sem)) > return EFI_ABORTED; > status = efi_queue_work(GET_TIME, tm, tc, NULL, NULL, NULL); > - up(&efi_runtime_lock); > + up(&efi_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(&efi_runtime_lock)) > + if (down_interruptible(&efi_runtime_sem)) > return EFI_ABORTED; > status = efi_queue_work(SET_TIME, tm, NULL, NULL, NULL, NULL); > - up(&efi_runtime_lock); > + up(&efi_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(&efi_runtime_lock)) > + if (down_interruptible(&efi_runtime_sem)) > return EFI_ABORTED; > status = efi_queue_work(GET_WAKEUP_TIME, enabled, pending, tm, NULL, > NULL); > - up(&efi_runtime_lock); > + up(&efi_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(&efi_runtime_lock)) > + if (down_interruptible(&efi_runtime_sem)) > return EFI_ABORTED; > status = efi_queue_work(SET_WAKEUP_TIME, &enabled, tm, NULL, NULL, > NULL); > - up(&efi_runtime_lock); > + up(&efi_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(&efi_runtime_lock)) > + if (down_interruptible(&efi_runtime_sem)) > return EFI_ABORTED; > status = efi_queue_work(GET_VARIABLE, name, vendor, attr, data_size, > data); > - up(&efi_runtime_lock); > + up(&efi_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(&efi_runtime_lock)) > + if (down_interruptible(&efi_runtime_sem)) > return EFI_ABORTED; > status = efi_queue_work(GET_NEXT_VARIABLE, name_size, name, vendor, > NULL, NULL); > - up(&efi_runtime_lock); > + up(&efi_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(&efi_runtime_lock)) > + if (down_interruptible(&efi_runtime_sem)) > return EFI_ABORTED; > status = efi_queue_work(SET_VARIABLE, name, vendor, &attr, &data_size, > data); > - up(&efi_runtime_lock); > + up(&efi_runtime_sem); > return status; > } > > @@ -330,12 +330,12 @@ virt_efi_set_variable_nonblocking(efi_char16_t *name, efi_guid_t *vendor, > { > efi_status_t status; > > - if (down_trylock(&efi_runtime_lock)) > + if (down_trylock(&efi_runtime_sem)) > return EFI_NOT_READY; > > status = efi_call_virt(set_variable, name, vendor, attr, data_size, > data); > - up(&efi_runtime_lock); > + up(&efi_runtime_sem); > return status; > } > > @@ -350,11 +350,11 @@ static efi_status_t virt_efi_query_variable_info(u32 attr, > if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION) > return EFI_UNSUPPORTED; > > - if (down_interruptible(&efi_runtime_lock)) > + if (down_interruptible(&efi_runtime_sem)) > return EFI_ABORTED; > status = efi_queue_work(QUERY_VARIABLE_INFO, &attr, storage_space, > remaining_space, max_variable_size, NULL); > - up(&efi_runtime_lock); > + up(&efi_runtime_sem); > return status; > } > > @@ -369,12 +369,12 @@ virt_efi_query_variable_info_nonblocking(u32 attr, > if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION) > return EFI_UNSUPPORTED; > > - if (down_trylock(&efi_runtime_lock)) > + if (down_trylock(&efi_runtime_sem)) > return EFI_NOT_READY; > > status = efi_call_virt(query_variable_info, attr, storage_space, > remaining_space, max_variable_size); > - up(&efi_runtime_lock); > + up(&efi_runtime_sem); > return status; > } > > @@ -382,11 +382,11 @@ static efi_status_t virt_efi_get_next_high_mono_count(u32 *count) > { > efi_status_t status; > > - if (down_interruptible(&efi_runtime_lock)) > + if (down_interruptible(&efi_runtime_sem)) > return EFI_ABORTED; > status = efi_queue_work(GET_NEXT_HIGH_MONO_COUNT, count, NULL, NULL, > NULL, NULL); > - up(&efi_runtime_lock); > + up(&efi_runtime_sem); > return status; > } > > @@ -395,14 +395,14 @@ static void virt_efi_reset_system(int reset_type, > unsigned long data_size, > efi_char16_t *data) > { > - if (down_interruptible(&efi_runtime_lock)) { > + if (down_interruptible(&efi_runtime_sem)) { > pr_warn("failed to invoke the reset_system() runtime service:\n" > "could not get exclusive access to the firmware\n"); > return; > } > efi_rts_work.efi_rts_id = RESET_SYSTEM; > __efi_call_virt(reset_system, reset_type, status, data_size, data); > - up(&efi_runtime_lock); > + up(&efi_runtime_sem); > } > > static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules, > @@ -414,11 +414,11 @@ static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules, > if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION) > return EFI_UNSUPPORTED; > > - if (down_interruptible(&efi_runtime_lock)) > + if (down_interruptible(&efi_runtime_sem)) > return EFI_ABORTED; > status = efi_queue_work(UPDATE_CAPSULE, capsules, &count, &sg_list, > NULL, NULL); > - up(&efi_runtime_lock); > + up(&efi_runtime_sem); > return status; > } > > @@ -432,11 +432,11 @@ static efi_status_t virt_efi_query_capsule_caps(efi_capsule_header_t **capsules, > if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION) > return EFI_UNSUPPORTED; > > - if (down_interruptible(&efi_runtime_lock)) > + if (down_interruptible(&efi_runtime_sem)) > return EFI_ABORTED; > status = efi_queue_work(QUERY_CAPSULE_CAPS, capsules, &count, > max_size, reset_type, NULL); > - up(&efi_runtime_lock); > + up(&efi_runtime_sem); > return status; > } > > diff --git a/include/linux/efi.h b/include/linux/efi.h > index 45ff763fba76..930cd20842b8 100644 > --- a/include/linux/efi.h > +++ b/include/linux/efi.h > @@ -1745,6 +1745,9 @@ extern struct efi_runtime_work efi_rts_work; > /* Workqueue to queue EFI Runtime Services */ > extern struct workqueue_struct *efi_rts_wq; > > +/* EFI runtime semaphore */ > +extern struct semaphore efi_runtime_sem; > + > struct linux_efi_memreserve { > int size; // allocated size of the array > atomic_t count; // number of entries used ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] x86/platform/UV: kill uv_bios_call_reentrant() as it has no callers 2019-01-09 10:45 [PATCH 0/3] Protect against concurrent calls into UV BIOS Hedi Berriche 2019-01-09 10:45 ` [PATCH 1/3] efi/x86: turn EFI runtime semaphore into a global lock Hedi Berriche @ 2019-01-09 10:45 ` Hedi Berriche 2019-01-26 14:04 ` Ard Biesheuvel 2019-01-09 10:45 ` [PATCH 3/3] x86/platform/UV: use efi_runtime_sem to serialise BIOS calls Hedi Berriche 2 siblings, 1 reply; 11+ messages in thread From: Hedi Berriche @ 2019-01-09 10:45 UTC (permalink / raw) To: Ard Biesheuvel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin Cc: x86, linux-efi, linux-kernel, Hedi Berriche, Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl uv_bios_call_reentrant() has no callers nor is it exported, kill it. Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com> Reviewed-by: Russ Anderson <rja@hpe.com> Reviewed-by: Mike Travis <mike.travis@hpe.com> Reviewed-by: Dimitri Sivanich <sivanich@hpe.com> Reviewed-by: Steve Wahl <steve.wahl@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 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.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] x86/platform/UV: kill uv_bios_call_reentrant() as it has no callers 2019-01-09 10:45 ` [PATCH 2/3] x86/platform/UV: kill uv_bios_call_reentrant() as it has no callers Hedi Berriche @ 2019-01-26 14:04 ` Ard Biesheuvel 0 siblings, 0 replies; 11+ messages in thread From: Ard Biesheuvel @ 2019-01-26 14:04 UTC (permalink / raw) To: Hedi Berriche Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers, linux-efi, Linux Kernel Mailing List, Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl On Wed, 9 Jan 2019 at 11:46, Hedi Berriche <hedi.berriche@hpe.com> wrote: > > uv_bios_call_reentrant() has no callers nor is it exported, kill it. > > Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com> > Reviewed-by: Russ Anderson <rja@hpe.com> > Reviewed-by: Mike Travis <mike.travis@hpe.com> > Reviewed-by: Dimitri Sivanich <sivanich@hpe.com> > Reviewed-by: Steve Wahl <steve.wahl@hpe.com> Please drop these reviewed-bys Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > 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.0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] x86/platform/UV: use efi_runtime_sem to serialise BIOS calls 2019-01-09 10:45 [PATCH 0/3] Protect against concurrent calls into UV BIOS Hedi Berriche 2019-01-09 10:45 ` [PATCH 1/3] efi/x86: turn EFI runtime semaphore into a global lock Hedi Berriche 2019-01-09 10:45 ` [PATCH 2/3] x86/platform/UV: kill uv_bios_call_reentrant() as it has no callers Hedi Berriche @ 2019-01-09 10:45 ` Hedi Berriche 2019-01-10 7:13 ` Bhupesh Sharma 2019-01-26 14:10 ` Ard Biesheuvel 2 siblings, 2 replies; 11+ messages in thread From: Hedi Berriche @ 2019-01-09 10:45 UTC (permalink / raw) To: Ard Biesheuvel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin Cc: x86, linux-efi, linux-kernel, Hedi Berriche, Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl Calls into UV firmware must be protected against concurrency, use the now visible efi_runtime_sem lock to serialise them. Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com> Reviewed-by: Russ Anderson <rja@hpe.com> Reviewed-by: Mike Travis <mike.travis@hpe.com> Reviewed-by: Dimitri Sivanich <sivanich@hpe.com> Reviewed-by: Steve Wahl <steve.wahl@hpe.com> --- arch/x86/include/asm/uv/bios.h | 3 ++- arch/x86/platform/uv/bios_uv.c | 25 ++++++++++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/arch/x86/include/asm/uv/bios.h b/arch/x86/include/asm/uv/bios.h index 4eee646544b2..33e94aa0b1ff 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 cd05af157763..92f960798e20 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) +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; @@ -44,13 +45,26 @@ 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); 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_runtime_sem)) + return BIOS_STATUS_ABORT; + + ret = __uv_bios_call(which, a1, a2, a3, a4, a5); + up(&efi_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(&efi_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(&efi_runtime_sem); + return ret; } -- 2.20.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] x86/platform/UV: use efi_runtime_sem to serialise BIOS calls 2019-01-09 10:45 ` [PATCH 3/3] x86/platform/UV: use efi_runtime_sem to serialise BIOS calls Hedi Berriche @ 2019-01-10 7:13 ` Bhupesh Sharma 2019-01-14 11:30 ` Hedi Berriche 2019-01-26 14:10 ` Ard Biesheuvel 1 sibling, 1 reply; 11+ messages in thread From: Bhupesh Sharma @ 2019-01-10 7:13 UTC (permalink / raw) To: Hedi Berriche, Ard Biesheuvel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin Cc: x86, linux-efi, linux-kernel, Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl, bhsharma, Bhupesh SHARMA Hi Hedi, Thanks for the patchset. I will give this a go on my sgi-uv300 machine and come back with more detailed inputs, but I wanted to ask about the hang/panic you mentioned in the cover letter when efi_scratch gets clobbered. Can you describe the same (for e.g. how to reproduce this). Nitpicks below: On 01/09/2019 04:15 PM, Hedi Berriche wrote: > Calls into UV firmware must be protected against concurrency, use the > now visible efi_runtime_sem lock to serialise them. > > Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com> > Reviewed-by: Russ Anderson <rja@hpe.com> > Reviewed-by: Mike Travis <mike.travis@hpe.com> > Reviewed-by: Dimitri Sivanich <sivanich@hpe.com> > Reviewed-by: Steve Wahl <steve.wahl@hpe.com> > --- > arch/x86/include/asm/uv/bios.h | 3 ++- > arch/x86/platform/uv/bios_uv.c | 25 ++++++++++++++++++++++--- > 2 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/include/asm/uv/bios.h b/arch/x86/include/asm/uv/bios.h > index 4eee646544b2..33e94aa0b1ff 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 cd05af157763..92f960798e20 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) > +s64 __uv_bios_call(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, > + u64 a4, u64 a5) Can we make this static? > { > struct uv_systab *tab = uv_systab; > s64 ret; > @@ -44,13 +45,26 @@ 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); > > 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_runtime_sem)) > + return BIOS_STATUS_ABORT; > + > + ret = __uv_bios_call(which, a1, a2, a3, a4, a5); > + up(&efi_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(&efi_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(&efi_runtime_sem); > + > return ret; > } > Thanks, Bhupesh ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] x86/platform/UV: use efi_runtime_sem to serialise BIOS calls 2019-01-10 7:13 ` Bhupesh Sharma @ 2019-01-14 11:30 ` Hedi Berriche 0 siblings, 0 replies; 11+ messages in thread From: Hedi Berriche @ 2019-01-14 11:30 UTC (permalink / raw) To: Bhupesh Sharma Cc: Ard Biesheuvel, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-efi, linux-kernel, Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl, Bhupesh SHARMA On Thu, Jan 10, 2019 at 07:14 Bhupesh Sharma wrote: >Hi Hedi, Hi Bhupesh, >Thanks for the patchset. Thanks for looking at it. >I will give this a go on my sgi-uv300 machine and come back with more >detailed inputs, and for testing it. >but I wanted to ask about the hang/panic you >mentioned in the cover letter when efi_scratch gets clobbered. Can you >describe the same (for e.g. how to reproduce this). When efi_switch_mm() gets called concurrently from two different CPUs --via arch_efi_call_virt_setup()-- due to lack of serialisation in uv_bios_call(), efi_scratch.prev_mm is overwritten and that's how all hell breaks loose, and that's when you see either a hang (the more frequent failure mode) or a panic. In order to reproduce the problem you'd need, for example, a kernel module that makes use of uv_bios_call(), in which case a test case would be a loop with: - 2 concurrent tasks both invoking uv_bios_call() or - 2 concurrent tasks - one invoking uv_bios_call() - one, for example, accessing an EFI vars via efivars >Nitpicks below: > > >On 01/09/2019 04:15 PM, Hedi Berriche wrote: >>Calls into UV firmware must be protected against concurrency, use the >>now visible efi_runtime_sem lock to serialise them. >> >>Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com> >>Reviewed-by: Russ Anderson <rja@hpe.com> >>Reviewed-by: Mike Travis <mike.travis@hpe.com> >>Reviewed-by: Dimitri Sivanich <sivanich@hpe.com> >>Reviewed-by: Steve Wahl <steve.wahl@hpe.com> >>--- >> arch/x86/include/asm/uv/bios.h | 3 ++- >> arch/x86/platform/uv/bios_uv.c | 25 ++++++++++++++++++++++--- >> 2 files changed, 24 insertions(+), 4 deletions(-) >> >>diff --git a/arch/x86/include/asm/uv/bios.h b/arch/x86/include/asm/uv/bios.h >>index 4eee646544b2..33e94aa0b1ff 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 cd05af157763..92f960798e20 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) >>+s64 __uv_bios_call(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3, >>+ u64 a4, u64 a5) > >Can we make this static? Will do. >> { >> struct uv_systab *tab = uv_systab; >> s64 ret; >>@@ -44,13 +45,26 @@ 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); >> 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_runtime_sem)) >>+ return BIOS_STATUS_ABORT; >>+ >>+ ret = __uv_bios_call(which, a1, a2, a3, a4, a5); >>+ up(&efi_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(&efi_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(&efi_runtime_sem); >>+ >> return ret; >> } > >Thanks, >Bhupesh Cheers, Hedi. -- Be careful of reading health books, you might die of a misprint. -- Mark Twain ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] x86/platform/UV: use efi_runtime_sem to serialise BIOS calls 2019-01-09 10:45 ` [PATCH 3/3] x86/platform/UV: use efi_runtime_sem to serialise BIOS calls Hedi Berriche 2019-01-10 7:13 ` Bhupesh Sharma @ 2019-01-26 14:10 ` Ard Biesheuvel 1 sibling, 0 replies; 11+ messages in thread From: Ard Biesheuvel @ 2019-01-26 14:10 UTC (permalink / raw) To: Hedi Berriche Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, the arch/x86 maintainers, linux-efi, Linux Kernel Mailing List, Russ Anderson, Mike Travis, Dimitri Sivanich, Steve Wahl On Wed, 9 Jan 2019 at 11:46, Hedi Berriche <hedi.berriche@hpe.com> wrote: > > Calls into UV firmware must be protected against concurrency, use the > now visible efi_runtime_sem lock to serialise them. > > Signed-off-by: Hedi Berriche <hedi.berriche@hpe.com> > Reviewed-by: Russ Anderson <rja@hpe.com> > Reviewed-by: Mike Travis <mike.travis@hpe.com> > Reviewed-by: Dimitri Sivanich <sivanich@hpe.com> > Reviewed-by: Steve Wahl <steve.wahl@hpe.com> > --- > arch/x86/include/asm/uv/bios.h | 3 ++- > arch/x86/platform/uv/bios_uv.c | 25 ++++++++++++++++++++++--- > 2 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/include/asm/uv/bios.h b/arch/x86/include/asm/uv/bios.h > index 4eee646544b2..33e94aa0b1ff 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 Nit: please add a trailing comma so the next patch looks cleaner. > }; > > /* Address map parameters */ > diff --git a/arch/x86/platform/uv/bios_uv.c b/arch/x86/platform/uv/bios_uv.c > index cd05af157763..92f960798e20 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) > +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; > @@ -44,13 +45,26 @@ 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))) This is an unrelated change. You should at least mention it in the commit log, or put it in a separate patch. > 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); > > 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_runtime_sem)) > + return BIOS_STATUS_ABORT; > + > + ret = __uv_bios_call(which, a1, a2, a3, a4, a5); > + up(&efi_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(&efi_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(&efi_runtime_sem); > + > return ret; > } > > -- > 2.20.0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-01-26 14:11 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-09 10:45 [PATCH 0/3] Protect against concurrent calls into UV BIOS Hedi Berriche 2019-01-09 10:45 ` [PATCH 1/3] efi/x86: turn EFI runtime semaphore into a global lock Hedi Berriche 2019-01-15 18:55 ` Thomas Gleixner 2019-01-15 19:35 ` Hedi Berriche 2019-01-26 14:03 ` Ard Biesheuvel 2019-01-09 10:45 ` [PATCH 2/3] x86/platform/UV: kill uv_bios_call_reentrant() as it has no callers Hedi Berriche 2019-01-26 14:04 ` Ard Biesheuvel 2019-01-09 10:45 ` [PATCH 3/3] x86/platform/UV: use efi_runtime_sem to serialise BIOS calls Hedi Berriche 2019-01-10 7:13 ` Bhupesh Sharma 2019-01-14 11:30 ` Hedi Berriche 2019-01-26 14:10 ` Ard Biesheuvel
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).