Re: [RFC PATCH] efi/x86: limit EFI old memory map to SGI UV1 machines

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Thu, 2 Jan 2020 at 17:45, Russ Anderson <rja@hpe.com> wrote:
>
> On Thu, Jan 02, 2020 at 04:04:39PM +0100, Ard Biesheuvel wrote:
> > On Thu, 2 Jan 2020 at 15:38, Russ Anderson <rja@hpe.com> wrote:
> > >
> > > On Tue, Dec 31, 2019 at 05:05:47PM +0100, Borislav Petkov wrote:
> > > > On Tue, Dec 31, 2019 at 12:13:18PM +0100, Ard Biesheuvel wrote:
> > > > > (adding Boris and Dave for the historical perspective)
> > > > >
> > > > > On Thu, 26 Dec 2019 at 10:55, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > >
> > > > > > We carry a quirk in the x86 EFI code to switch back to an older
> > > > > > method of mapping the EFI runtime services memory regions, because
> > > > > > it was deemed risky at the time to implement a new method without
> > > > > > providing a fallback to the old method in case problems arose.
> > > > > >
> > > > > > Such problems did arise, but they appear to be limited to SGI UV1
> > > > > > machines, and so these are the only ones for which the fallback gets
> > > > > > enabled automatically (via a DMI quirk). The fallback can be enabled
> > > > > > manually as well, by passing efi=old_map, but there is very little
> > > > > > evidence that suggests that this is something that is being relied
> > > > > > upon in the field.
> > > > > >
> > > > > > Given that UV1 support is not enabled by default by the distros
> > > > > > (Ubuntu, Fedora), there is no point in carrying this fallback code
> > > > > > all the time if there are no other users. So let's refactor it a bit
> > > > > > to make it depend on CONFIG_X86_UV, and remove the ability to enable
> > > > > > it by hand.
> > > > > >
> > > > > > Cc: Dimitri Sivanich <dimitri.sivanich@hpe.com>
> > > > > > Cc: Mike Travis <mike.travis@hpe.com>
> > > > > > Cc: Hedi Berriche <hedi.berriche@hpe.com>
> > > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > >
> > > > > Boris, since you were the one that added efi=old_map: do you know of
> > > > > any cases beyond SGI UV1 where it was actually needed? There is some
> > > > > mention of using it to work around transient breakage on 32-bit caused
> > > > > by your original changes, but other than that, Google doesn't seem to
> > > > > know about any cases where efi=old_map is being used to deal with
> > > > > actual firmware quirks.
> > >
> > > We (SGI -> HPE) have used the efi=old_map quirk to work around issues,
> > > including on the currently shipping HPE Superdome Flex (aka UV4).
> > >
> > > An example was working around an EFI locking issues when calling
> > > into BIOS, fixed by this commit.
> > >
> > >   f331e766c4be ("x86/platform/UV: Use efi_runtime_lock to serialise BIOS calls")
> > >
> > > We do not currently use the quirk, and nopefully never will need to
> > > use it again, but it has been used recently and are very glad Boris
> > > added it.  I am hesitent to remove it because it has been used recently
> > > on currently shipping hardware.
> > >
> >
> > Thanks for the data point.
> >
> > So what about making it depend on CONFIG_X86_UV=y, would that still
> > work for you?
>
> I want to make sure my undestanding of what you are proposing
> is the same as what you are proposing.
>

What I am proposing is to document efi=old_map as only being available
on kernels built with CONFIG_X86_UV=y, and moving the code that
implements it into the UV support code.

> I will have some additional background information shortly.
>

Thanks, that is very helpful.