• Ard Biesheuvel's avatar
    efi/x86: Fix mixed mode reboot loop by removing pointless call to PciIo->Attributes() · e2967018
    Ard Biesheuvel authored
    Hans de Goede reported that his mixed EFI mode Bay Trail tablet
    would not boot at all any more, but enter a reboot loop without
    any logs printed by the kernel.
    
    Unbreak 64-bit Linux/x86 on 32-bit UEFI:
    
    When it was first introduced, the EFI stub code that copies the
    contents of PCI option ROMs originally only intended to do so if
    the EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM attribute was *not* set.
    
    The reason was that the UEFI spec permits PCI option ROM images
    to be provided by the platform directly, rather than via the ROM
    BAR, and in this case, the OS can only access them at runtime if
    they are preserved at boot time by copying them from the areas
    described by PciIo->RomImage and PciIo->RomSize.
    
    However, it implemented this check erroneously, as can be seen in
    commit:
    
      dd5fc854 ("EFI: Stash ROMs if they're not in the PCI BAR")
    
    which introduced:
    
        if (!attributes & EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM)
                continue;
    
    and given that the numeric value of EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM
    is 0x4000, this condition never becomes true, and so the option ROMs
    were copied unconditionally.
    
    This was spotted and 'fixed' by commit:
    
      886d751a ("x86, efi: correct precedence of operators in setup_efi_pci")
    
    but inadvertently inverted the logic at the same time, defeating
    the purpose of the code, since it now only preserves option ROM
    images that can be read from the ROM BAR as well.
    
    Unsurprisingly, this broke some systems, and so the check was removed
    entirely in the following commit:
    
      73970188 ("x86, efi: remove attribute check from setup_efi_pci")
    
    It is debatable whether this check should have been included in the
    first place, since the option ROM image provided to the UEFI driver by
    the firmware may be different from the one that is actually present in
    the card's flash ROM, and so whatever PciIo->RomImage points at should
    be preferred regardless of whether the attribute is set.
    
    As this was the only use of the attributes field, we can remove
    the call to PciIo->Attributes() entirely, which is especially
    nice because its prototype involves uint64_t type by-value
    arguments which the EFI mixed mode has trouble dealing with.
    
    Any mixed mode system with PCI is likely to be affected.
    Tested-by: default avatarWilfried Klaebe <linux-kernel@lebenslange-mailadresse.de>
    Tested-by: default avatarHans de Goede <hdegoede@redhat.com>
    Signed-off-by: default avatarArd Biesheuvel <ard.biesheuvel@linaro.org>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Matt Fleming <matt@codeblueprint.co.uk>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Cc: linux-efi@vger.kernel.org
    Link: http://lkml.kernel.org/r/20180711090235.9327-2-ard.biesheuvel@linaro.orgSigned-off-by: default avatarIngo Molnar <mingo@kernel.org>
    e2967018
eboot.c 26.4 KB