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@xxxxxxxxxx> 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@xxxxxxx>
> > > Cc: Mike Travis <mike.travis@xxxxxxx>
> > > Cc: Hedi Berriche <hedi.berriche@xxxxxxx>
> > > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> >
> > 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.
> > As a followup to this change, I'd like to move the old memmap handling
> > into the UV1 support code, so it is omitted unless UV support is
> > compiled it, and so it can be retired with the rest of it once that
> > time comes.
>
> Good idea.
>
> So I remember "some apple laptops" but reading this again:
>
> https://lore.kernel.org/patchwork/patch/704853/
>
> looks like mfleming meant the opposite: some apple laptops don't like
> the 1:1 runtime mapping. But there might be more and I believe mjg59 had
> some use case at the time but I could be remembering it wrong.
>
> Let me add them both to Cc for comment.
>
> So one other reason for adding the =old_map thing is having a fallback
> to the old method in case we break some boxes. It even says so in the
> commit message of:
>
> d2f7cbe7b26a ("x86/efi: Runtime services virtual mapping")
>
> "While at it, add a chicken bit called "efi=old_map" which can be used
> as a fallback to the old runtime services mapping method in case there's
> some b0rkage with a particular EFI implementation..."
>
> But I'm not aware of people still booting with "old_map" so I guess
> deprecating it should be probably ok...
>
> HTH.
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
--
Russ Anderson, SuperDome Flex Linux Kernel Group Manager
HPE - Hewlett Packard Enterprise (formerly SGI) rja@xxxxxxx
