pci_cfgreg.c: Use io port config access for early boot time.

[kib]

Summary

Some early PCIe chipsets are explicitly listed in the white-list to enable use of the MMIO config space accesses, perhaps because ACPI tables were not reliable source of the base MCFG address at that time.

During very early stage of boot, see pci_early_quirks.c, we cannot map 255MB of registers because the method used overflows initial kernel page tables. Move fallback for some old Intel chipsets where the MCFG base is read from the known chipset register to the attachment method of the legacy device, and use io access until MCFG is parsed or legacy attach called.

There is a mention in the Intel documentation for corresponding chipsets that OS must use either io port or MMIO access method, but we already break this rule by reading MCFGbase register, so one more access seems to be innocent.

PR: 236838

[avg]

LGTM.

I wonder if the pci_cfgregopen problem reported for AMD Ryzen systems has a similar cause...

[kib]

In D19833#425441, @avg wrote:

LGTM.

I wonder if the pci_cfgregopen problem reported for AMD Ryzen systems has a similar cause...

What is the Ryzen problem ? This issue can only occur when config space is accessed before pmap is initialized, and I am only aware of pci_early_quirks which do that, which might falls to MCFG access only on some Intels.

[kib]

And, after thinking about this code more, why do we check for specific chipsets and read MCFGbase from the register ? amd64 requires ACPI, were that era machines so broken that ACPI did not reported MCFG base ? Reported PR with Pentium-IV Xeon and E7500 MCH has the line

PCIe: Memory Mapped configuration base @XXXX

So may be we should remove special handling of some Intel chipsets from amd64 pci_cfgregopen() instead of my hack.

[avg]

In D19833#425585, @kib wrote:

In D19833#425441, @avg wrote:

LGTM.

I wonder if the pci_cfgregopen problem reported for AMD Ryzen systems has a similar cause...

What is the Ryzen problem ? This issue can only occur when config space is accessed before pmap is initialized, and I am only aware of pci_early_quirks which do that, which might falls to MCFG access only on some Intels.

I have seen multiple reports when people had to disable hw.pci.mcfg to be able to boot a Ryzen system.
Here is one example: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=231760
But I have never looked into details...
Although, it seems that John has.

[kib]

In D19833#425621, @avg wrote:

I have seen multiple reports when people had to disable hw.pci.mcfg to be able to boot a Ryzen system.
Here is one example: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=231760

This seems to be completely unrelated.

[jhb]

Hmm, so scottl@ added this version of MMIO access before we looked at the MCFG table, and I'm not sure that the BIOS for these chipsets don't actually already provide the MCFG table. However, on systems where we rely on the MCFG table, we won't have parsed the table to setup the mapping until later anyway, so I think the change is ok. Alternatively, we could move the fallback for these chipsets into a separate function that we explicitly call at around the same time we parse MCFG and just do it later in boot once it's ok to map the table, but that's a lot more work (though maybe not if we just call it from the pmap code at the time that pmap_initialized is set?) Reading the commit message was a bit confusing to me at first, so I think it might be useful to say that this makes those chipsets act more like other chipsets where we use MCFG as with MCFG we don't enable mmio access until part-way through the boot.

Hmm, actually, isn't this disabling MMIO entirely on these chipsets? Since cfgmech is set to CFG_MECH_1 before the return, the calls to pci_cfgregopen() in the future will all return right away and these chipsets will never use mmio. To preserve use of mmio you would need to do the callback method where we invoke pcie_cfgregopen to switch from CFG_MECH_1 to CFG_MECH_PCIE after the direct-map is setup. We should probably move the MCFG handling out of acpi.c as well and find the MCFG table the way we find the MADT tables and just have a single callback after the direct-map is setup that looks for the MCFG table to setup the mmio window and if that doesn't exist, tries the fallbacks for old chipsets so we can ensure MCFG always has preference.

sys/amd64/pci/pci_cfgreg.c

288 ↗

(On Diff #55862)

This is an always-true condition since cfgmech was just set, so pci_cfgregopen() will always return 1.

[kib]

In D19833#425992, @jhb wrote:

Hmm, so scottl@ added this version of MMIO access before we looked at the MCFG table, and I'm not sure that the BIOS for these chipsets don't actually already provide the MCFG table.

At least on the machine of the person who reported the problem, there is MCFG table. Chipset MCH is E7500.

However, on systems where we rely on the MCFG table, we won't have parsed the table to setup the mapping until later anyway, so I think the change is ok. Alternatively, we could move the fallback for these chipsets into a separate function that we explicitly call at around the same time we parse MCFG and just do it later in boot once it's ok to map the table, but that's a lot more work (though maybe not if we just call it from the pmap code at the time that pmap_initialized is set?) Reading the commit message was a bit confusing to me at first, so I think it might be useful to say that this makes those chipsets act more like other chipsets where we use MCFG as with MCFG we don't enable mmio access until part-way through the boot.

In fact, in the later response I proposed to remove the fallback at all. We require ACPI on amd64, and it seems that at least one BIOS is not so much broken to not provide MCFG.

Hmm, actually, isn't this disabling MMIO entirely on these chipsets? Since cfgmech is set to CFG_MECH_1 before the return, the calls to pci_cfgregopen() in the future will all return right away and these chipsets will never use mmio.

I do not think so. acpi.c::acpi_enable_pcie() calls pciE_cfgregopen(), not pci_cfgregopen(). And pcie_cfgregopen() always set mode to CFGMECH_PCIE.

To preserve use of mmio you would need to do the callback method where we invoke pcie_cfgregopen to switch from CFG_MECH_1 to CFG_MECH_PCIE after the direct-map is setup. We should probably move the MCFG handling out of acpi.c as well and find the MCFG table the way we find the MADT tables and just have a single callback after the direct-map is setup that looks for the MCFG table to setup the mmio window and if that doesn't exist, tries the fallbacks for old chipsets so we can ensure MCFG always has preference.

I think this would be overkill.

[jhb]

In D19833#426028, @kib wrote:

In D19833#425992, @jhb wrote:

Hmm, so scottl@ added this version of MMIO access before we looked at the MCFG table, and I'm not sure that the BIOS for these chipsets don't actually already provide the MCFG table.

At least on the machine of the person who reported the problem, there is MCFG table. Chipset MCH is E7500.

However, on systems where we rely on the MCFG table, we won't have parsed the table to setup the mapping until later anyway, so I think the change is ok. Alternatively, we could move the fallback for these chipsets into a separate function that we explicitly call at around the same time we parse MCFG and just do it later in boot once it's ok to map the table, but that's a lot more work (though maybe not if we just call it from the pmap code at the time that pmap_initialized is set?) Reading the commit message was a bit confusing to me at first, so I think it might be useful to say that this makes those chipsets act more like other chipsets where we use MCFG as with MCFG we don't enable mmio access until part-way through the boot.

In fact, in the later response I proposed to remove the fallback at all. We require ACPI on amd64, and it seems that at least one BIOS is not so much broken to not provide MCFG.

I would probably be happy to just remove the fallback if we don't need it.

Hmm, actually, isn't this disabling MMIO entirely on these chipsets? Since cfgmech is set to CFG_MECH_1 before the return, the calls to pci_cfgregopen() in the future will all return right away and these chipsets will never use mmio.

I do not think so. acpi.c::acpi_enable_pcie() calls pciE_cfgregopen(), not pci_cfgregopen(). And pcie_cfgregopen() always set mode to CFGMECH_PCIE.

Ah, if it has MCFG, yes. However, if it doesn't have MCFG it does remove it. That is, my point is that your change is actually disabling the fallback entirely, but since you aren't removing it it doesn't like you are.

To preserve use of mmio you would need to do the callback method where we invoke pcie_cfgregopen to switch from CFG_MECH_1 to CFG_MECH_PCIE after the direct-map is setup. We should probably move the MCFG handling out of acpi.c as well and find the MCFG table the way we find the MADT tables and just have a single callback after the direct-map is setup that looks for the MCFG table to setup the mmio window and if that doesn't exist, tries the fallbacks for old chipsets so we can ensure MCFG always has preference.

I think this would be overkill.

Well, we should either remove the fallback outright, or we should fix it to actually be used. I think I would be fine with just removing it outright. We should remove it on i386 as well in that case since the same list of devices is duplicated in i386.

[kib]

In D19833#426071, @jhb wrote:

Well, we should either remove the fallback outright, or we should fix it to actually be used. I think I would be fine with just removing it outright.

Yes, I will update the patch with removal in a minute.

We should remove it on i386 as well in that case since the same list of devices is duplicated in i386.

I would not do this. For i386 we still allow to run without ACPI, I remember. If user did configured the machine this way, not having PCIe extended config accessible might break things.

[jhb]

sys/amd64/pci/pci_cfgreg.c

77 ↗

(On Diff #55967)

MCFG isn't required btw. I might reword this to say something like:

/*
  * For amd64 we assume that type 1 I/O port-based access always works.  If an ACPI MCFG table exists,
  * pcie_cfgregopen() will be called to switch to memory-mapped access.
  */

I would maybe even move that comment up above the cfgmech variable and have that variable default to being set to CFG_MECH_1 and have this function just be 'return (1)'.

For i386 I might be tempted in a followup to move the fallback thing into something that 'legacy0' calls from its attach routine. FWIW, you can still boot amd64 with ACPI disabled, and moving this into legacy.c would keep the fallback for both i386 and amd64 and do it in about the same place ACPI enables MMIO now.

[kib]

sys/amd64/pci/pci_cfgreg.c

77 ↗

(On Diff #55967)

By 'MCFG is not required' you mean that it could be that the machine does not have PCIe/PCI-X root bridge at all ? Because otherwise my reading of the combination of PCIe and PCIFW standards is that MCFG must be present:
PCIe 3.0: 7.2.2. PCI Express Enhanced Configuration Access Mechanism (ECAM)

For systems that are PC-compatible, or that do not implement a processor-architecture-specific firmware interface standard that allows access to the Configuration Space, the ECAM is required as defined in this section.

and PCIFW 3.1: 4.1.2. MCFG Table Description

The MCFG table is an ACPI table that is used to communicate the base addresses corresponding to the non-hot removable PCI Segment Groups range within a PCI Segment Group available to the operating system at boot. This is required for the PC-compatible systems.

[kib]

Move legacy match to legacy.c as suggested.

[jhb]

sys/amd64/pci/pci_cfgreg.c

77 ↗

(On Diff #55967)

I think you can drop the comment above this function now since it is just a stub.

sys/x86/include/pci_cfgreg.h

58 ↗

(On Diff #55989)

I think this entire header is already KERNEL only since hostb_alloc_start, etc. are all kernel-specific.

[kib]

sys/x86/include/pci_cfgreg.h

58 ↗

(On Diff #55989)

They are only function declarations, which are harmless for userspace. But external vars are not that obvious.

[kib]

Fix two nits according to jhb comments.

[imp]

sys/x86/x86/legacy.c

145 ↗

(On Diff #56009)

So is it these are old chips that need these things, or is it a generic thing for all graphics chips?

[kib]

sys/x86/x86/legacy.c

145 ↗

(On Diff #56009)

This has nothing to do with embedded graphics. I believe these chipsets are listed because
a. they were first intel chipsets with PCIe
b. at that time it was not obvious that MCFG ACPI table is the right way, or for some non-compliant bioses the table was missed.

BTW, e7520 MCH did not have GPU, as far as I see.