On Fri, Nov 10, 2023 at 12:04:24PM +0100, Gerd Hoffmann wrote: > Hi, > > > This only changes the placement of the PCI bars. The pci setup code is > > the only consumer of that variable, guess it makes sense to move the > > quirk to the pci code (as suggested by Kevin) to clarify this. > > i.e. like this: > > From d538dc7d4316e557ae302464252444d09de0681d Mon Sep 17 00:00:00 2001 > From: Gerd Hoffmann <kra...@redhat.com> > Date: Tue, 7 Nov 2023 13:49:31 +0100 > Subject: [PATCH] limit physical address space size > > For better compatibility with old linux kernels, > see source code comment. > > Related (same problem in ovmf): > https://github.com/tianocore/edk2/commit/c1e853769046 > > Reported-by: Claudio Fontana <cfont...@suse.de> > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > --- > src/fw/pciinit.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c > index c7084f5e397e..7aeea61bfd05 100644 > --- a/src/fw/pciinit.c > +++ b/src/fw/pciinit.c > @@ -52,6 +52,7 @@ u64 pcimem64_start = BUILD_PCIMEM64_START; > u64 pcimem64_end = BUILD_PCIMEM64_END; > u64 pci_io_low_end = 0xa000; > u32 pci_use_64bit = 0; > +u32 pci_phys_bits = 0;
FWIW, all these flags are getting a bit confusing. Maybe do something like the patch below. > > struct pci_region_entry { > struct pci_device *dev; > @@ -967,7 +968,7 @@ static int pci_bios_check_devices(struct pci_bus *busses) > if (hotplug_support == HOTPLUG_PCIE) > resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO); > if (hotplug_support && pci_use_64bit && is64 && (type == > PCI_REGION_TYPE_PREFMEM)) > - align = (u64)1 << (CPUPhysBits - 11); > + align = (u64)1 << (pci_phys_bits - 11); > if (align > sum && hotplug_support && !resource_optional) > sum = align; /* reserve min size for hot-plug */ > if (size > sum) { > @@ -1131,12 +1132,12 @@ static void pci_bios_map_devices(struct pci_bus > *busses) > r64_mem.base = > le64_to_cpu(romfile_loadint("etc/reserved-memory-end", 0)); > if (r64_mem.base < 0x100000000LL + RamSizeOver4G) > r64_mem.base = 0x100000000LL + RamSizeOver4G; > - if (CPUPhysBits) { > - u64 top = 1LL << CPUPhysBits; > + if (pci_phys_bits) { FYI, this is a change in behavior - previously this condition would have been taken even if CPULongMode or RamSizeOver4G is false. I'm not sure if this change is intentional. (My example patch below follows your lead here.) > + u64 top = 1LL << pci_phys_bits; > u64 size = (ALIGN(sum_mem, (1LL<<30)) + > ALIGN(sum_pref, (1LL<<30))); > if (pci_use_64bit) > - size = ALIGN(size, (1LL<<(CPUPhysBits-3))); > + size = ALIGN(size, (1LL<<(pci_phys_bits-3))); > if (r64_mem.base < top - size) { > r64_mem.base = top - size; > } > @@ -1181,8 +1182,16 @@ pci_setup(void) > > dprintf(3, "pci setup\n"); > > - if (CPUPhysBits >= 36 && CPULongMode && RamSizeOver4G) > + if (CPUPhysBits >= 36 && CPULongMode && RamSizeOver4G) { > pci_use_64bit = 1; > + pci_phys_bits = CPUPhysBits; > + if (pci_phys_bits > 46) { > + // Old linux kernels have trouble dealing with more than 46 > + // phys-bits, so avoid that for now. Seems to be a bug in the > + // virtio-pci driver. Reported: centos-7, ubuntu-18.04 > + pci_phys_bits = 46; > + } > + } > > dprintf(1, "=== PCI bus & bridge init ===\n"); > if (pci_probe_host() != 0) { > -- > 2.41.0 > Possible alternate variable naming: diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c index c7084f5..cd64d6e 100644 --- a/src/fw/pciinit.c +++ b/src/fw/pciinit.c @@ -46,12 +46,16 @@ static const char *region_type_name[] = { [ PCI_REGION_TYPE_PREFMEM ] = "prefmem", }; +// Memory ranges exported to legacy ACPI type table generation u64 pcimem_start = BUILD_PCIMEM_START; u64 pcimem_end = BUILD_PCIMEM_END; u64 pcimem64_start = BUILD_PCIMEM64_START; u64 pcimem64_end = BUILD_PCIMEM64_END; -u64 pci_io_low_end = 0xa000; -u32 pci_use_64bit = 0; + +// Memory resource allocation tracking +static u64 pci_io_low_end = 0xa000; +static u32 pci_use_64bit = 0; +static u64 pci_mem64_top = 0; struct pci_region_entry { struct pci_device *dev; @@ -966,8 +970,9 @@ static int pci_bios_check_devices(struct pci_bus *busses) int resource_optional = 0; if (hotplug_support == HOTPLUG_PCIE) resource_optional = pcie_cap && (type == PCI_REGION_TYPE_IO); - if (hotplug_support && pci_use_64bit && is64 && (type == PCI_REGION_TYPE_PREFMEM)) - align = (u64)1 << (CPUPhysBits - 11); + if (hotplug_support && pci_use_64bit && is64 + && (type == PCI_REGION_TYPE_PREFMEM)) + align = pci_mem64_top >> 11; if (align > sum && hotplug_support && !resource_optional) sum = align; /* reserve min size for hot-plug */ if (size > sum) { @@ -1131,14 +1136,13 @@ static void pci_bios_map_devices(struct pci_bus *busses) r64_mem.base = le64_to_cpu(romfile_loadint("etc/reserved-memory-end", 0)); if (r64_mem.base < 0x100000000LL + RamSizeOver4G) r64_mem.base = 0x100000000LL + RamSizeOver4G; - if (CPUPhysBits) { - u64 top = 1LL << CPUPhysBits; + if (pci_mem64_top) { u64 size = (ALIGN(sum_mem, (1LL<<30)) + ALIGN(sum_pref, (1LL<<30))); if (pci_use_64bit) - size = ALIGN(size, (1LL<<(CPUPhysBits-3))); - if (r64_mem.base < top - size) { - r64_mem.base = top - size; + size = ALIGN(size, pci_mem64_top >> 3); + if (r64_mem.base < pci_mem64_top - size) { + r64_mem.base = pci_mem64_top - size; } if (e820_is_used(r64_mem.base, size)) r64_mem.base -= size; @@ -1181,8 +1185,16 @@ pci_setup(void) dprintf(3, "pci setup\n"); - if (CPUPhysBits >= 36 && CPULongMode && RamSizeOver4G) + if (CPUPhysBits >= 36 && CPULongMode && RamSizeOver4G) { pci_use_64bit = 1; + pci_mem64_top = 1LL << CPUPhysBits; + if (CPUPhysBits > 46) { + // Old linux kernels have trouble dealing with more than 46 + // phys-bits, so avoid that for now. Seems to be a bug in the + // virtio-pci driver. Reported: centos-7, ubuntu-18.04 + pci_mem64_top = 1LL << 46; + } + } dprintf(1, "=== PCI bus & bridge init ===\n"); if (pci_probe_host() != 0) { Cheers, -Kevin