Re: [PATCH 1/1] x86/pci: Skip early E820 check for ECAM region

From: Bjorn Helgaas
Date: Thu Apr 18 2024 - 13:16:06 EST


On Thu, Apr 18, 2024 at 10:14:21AM +0200, Hans de Goede wrote:
> On 4/17/24 10:40 PM, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> >
> > Arul, Mateusz, Imcarneiro91, and Aman reported a regression caused by
> > 07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map"). On the
> > Lenovo Legion 9i laptop, that commit removes the area containing ECAM from
> > E820, which means the early E820 validation started failing, which meant we
> > didn't enable ECAM in the "early MCFG" path
> >
> > The lack of ECAM caused many ACPI methods to fail, resulting in the
> > embedded controller, PS/2, audio, trackpad, and battery devices not being
> > detected. The _OSC method also failed, so Linux could not take control of
> > the PCIe hotplug, PME, and AER features:
> >
> > # pci_mmcfg_early_init()
> >
> > PCI: ECAM [mem 0xc0000000-0xce0fffff] (base 0xc0000000) for domain 0000 [bus 00-e0]
> > PCI: not using ECAM ([mem 0xc0000000-0xce0fffff] not reserved)
> >
> > ACPI Error: AE_ERROR, Returned by Handler for [PCI_Config] (20230628/evregion-300)
> > ACPI: Interpreter enabled
> > ACPI: Ignoring error and continuing table load
> > ACPI BIOS Error (bug): Could not resolve symbol [\_SB.PC00.RP01._SB.PC00], AE_NOT_FOUND (20230628/dswload2-162)
> > ACPI Error: AE_NOT_FOUND, During name lookup/catalog (20230628/psobject-220)
> > ACPI: Skipping parse of AML opcode: OpcodeName unavailable (0x0010)
> > ACPI BIOS Error (bug): Could not resolve symbol [\_SB.PC00.RP01._SB.PC00], AE_NOT_FOUND (20230628/dswload2-162)
> > ACPI Error: AE_NOT_FOUND, During name lookup/catalog (20230628/psobject-220)
> > ...
> > ACPI Error: Aborting method \_SB.PC00._OSC due to previous error (AE_NOT_FOUND) (20230628/psparse-529)
> > acpi PNP0A08:00: _OSC: platform retains control of PCIe features (AE_NOT_FOUND)
> >
> > # pci_mmcfg_late_init()
> >
> > PCI: ECAM [mem 0xc0000000-0xce0fffff] (base 0xc0000000) for domain 0000 [bus 00-e0]
> > PCI: [Firmware Info]: ECAM [mem 0xc0000000-0xce0fffff] not reserved in ACPI motherboard resources
> > PCI: ECAM [mem 0xc0000000-0xce0fffff] is EfiMemoryMappedIO; assuming valid
> > PCI: ECAM [mem 0xc0000000-0xce0fffff] reserved to work around lack of ACPI motherboard _CRS
> >
> > Per PCI Firmware r3.3, sec 4.1.2, ECAM space must be reserved by a PNP0C02
> > resource, but it need not be mentioned in E820, so we shouldn't look at
> > E820 to validate the ECAM space described by MCFG.
> >
> > 946f2ee5c731 ("[PATCH] i386/x86-64: Check that MCFG points to an e820
> > reserved area") added a sanity check of E820 to work around buggy MCFG
> > tables, but that over-aggressive validation causes failures like this one.
> >
> > Keep the E820 validation check only for older BIOSes (pre-2016) so the
> > buggy 2006-era machines don't break. Skip the early E820 check for 2016
> > and newer BIOSes.
>
> I know a fix for this has been long in the making so I don't want to throw
> a spanner into the works, but I wonder why is the is_efi_mmio() check inside
> the if (!early && !acpi_disabled) {} block (before this patch) ?
>
> is_efi_mmio() only relies on EFI memdescriptors and those are setup pretty
> early. Assuming that the EFI memdescriptors are indeed setup before
> pci_mmcfg_reserved(..., ..., early=true) gets called we could simply move
> the is_efi_mmio(&cfg->res) outside (below) the if (!early && !acpi_disabled)
> {} so that it always runs before the is_mmconf_reserved(e820__mapped_all, ...)
> check.
>
> Looking at the dmesg above the is_efi_mmio() check does succeed, so this
> should fix the issue without needing a BIOS year check ?

As far as I know there is no spec requirement that an area described
by MCFG appear in either the E820 map or the EFI map.

I would like to get away from relying on these things that the spec
doesn't require because they are so prone to breakage.

I would love to just get rid of this early usage of
pci_mmcfg_reserved() completely; I'm just afraid of breaking some
ancient 2006-era machine that still happens to be running.

Did I understand your question correctly?

Bjorn