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

As well as I:

Reviewed-by: Mike Travis <mike.travis@xxxxxxx>

> 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@xxxxxxxxxxxxxxx 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@xxxxxxx>
> > Cc: Mike Travis <mike.travis@xxxxxxx>
> > Cc: Dimitri Sivanich <sivanich@xxxxxxx>
> > Cc: Steve Wahl <steve.wahl@xxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx # 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