Re: [PATCH] x86/pci: Stop requiring MMCONFIG to be declared in E820, ACPI or EFI for newer systems

From: Mario Limonciello
Date: Fri Dec 15 2023 - 10:49:30 EST


On 12/14/2023 17:30, Bjorn Helgaas wrote:

I'm fairly certain we're just getting lucky in Linux on a lot of
devices that the region is often overlapping with a region for EFI
runtime services.

Ugh. Yes, I'm sure it's not an isolated problem.

Given the severity of what I've seen it can do to a system I'm
proposing FWTS to move it to HIGH:

https://lists.ubuntu.com/archives/fwts-devel/2023-December/013772.html

Thanks. I don't know anything about FWTS, but I'm a little skeptical
that it actually catches this issue. It *looks* like FWTS builds its
idea of the memory map from a dmesg log or /sys/firmware/memmap, which
I think both come from the E820 map, which is x86-specific, of course.

I don't see anything that builds a memory map based on _CRS methods,
which I think is what we really want since the spec says:

The resources can optionally be returned in Int15 E820h or
EFIGetMemoryMap as reserved memory but must always be reported
through ACPI as a motherboard resource.

(PCI Firmware spec r3.3, sec 4.1.2)

You're right; it doesn't catch the "root" of this issue, it only catches specifically when the region doesn't overlap with an existing reservation (like EFI runtime services).

A more thorough check would need to build a memory map.


What is the actual *harm* in just using this MCFG table to make a
reservation when there isn't a PNP0C02 _CRS region declared?

At worst (a buggy BIOS) you would end up with hole in the memory map
that isn't usable for devices. At best you end up with more working
devices without changing the firmware.

We definitely need to work around this in Linux, and your patch might
well be the right thing.

I'm a *little* hesitant because all the code in mmconfig-shared.c that
attempts to validate MCFG entries suggests that relying on them
uncritically was a problem in some cases, so I want to try to convince
myself that we really won't break something.

Bjorn

As I mentioned in commit message this type of check was first introduced in 7752d5cfe3d1.

$ git describe --contains 7752d5cfe3d1
v2.6.26-rc1~369^2~18

That's roughly ~2008. This is a long time back; IIRC it's before MMIO over 4GB was really added to BIOS in many PC platforms.

How about we build an escape hatch for users to put on the kernel command line in case of problems to restore the behavior that enforces reservations?
Maybe "enforce_ecam_resv"?

We could keep that around for a a year or two and if nothing pops up tear it out later.