In commit 9fa99d2519cb ("hw/pci-host: Fix x86 Host Bridges 64bit PCI
hole", 2017-11-16), we meant to expose such a 64-bit PCI MMIO aperture in
the ACPI DSDT that would be at least as large as the new "pci-hole64-size"
property (2GB on i440fx, 32GB on q35). The goal was to offer "enough"
64-bit MMIO aperture to the guest OS for hotplug purposes.
In that commit, we added or modified five functions:
- pc_pci_hole64_start(): shared between i440fx and q35. Provides a default
64-bit base, which starts beyond the cold-plugged 64-bit RAM, and skips
the DIMM hotplug area too (if any).
- i440fx_pcihost_get_pci_hole64_start(), q35_host_get_pci_hole64_start():
board-specific 64-bit base property getters called abstractly by the
ACPI generator. Both of these fall back to pc_pci_hole64_start() if the
firmware didn't program any 64-bit hole (i.e. if the firmware didn't
assign a 64-bit GPA to any MMIO BAR on any device). Otherwise, they
honor the firmware's BAR assignments (i.e., they treat the lowest 64-bit
GPA programmed by the firmware as the base address for the aperture).
- i440fx_pcihost_get_pci_hole64_end(), q35_host_get_pci_hole64_end():
these intended to extend the aperture to our size recommendation,
calculated relative to the base of the aperture.
Despite the original intent, i440fx_pcihost_get_pci_hole64_end() and
q35_host_get_pci_hole64_end() currently only extend the aperture relative
to the default base (pc_pci_hole64_start()), ignoring any programming done
by the firmware. This means that our size recommendation may not be met.
Fix it by honoring the firmware's address assignments.
The strange extension sizes were spotted by Alex, in the log of a guest
kernel running on top of OVMF (which prefers to assign 64-bit GPAs to
64-bit BARs).
This change only affects DSDT generation, therefore no new compat property
is being introduced.
Using an i440fx OVMF guest with 5GB RAM, an example _CRS change is:
@@ -881,9 +881,9 @@
QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed,
Cacheable, ReadWrite,
0x0000000000000000, // Granularity
0x0000000800000000, // Range Minimum
- 0x000000080001C0FF, // Range Maximum
+ 0x000000087FFFFFFF, // Range Maximum
0x0000000000000000, // Translation Offset
- 0x000000000001C100, // Length
+ 0x0000000080000000, // Length
,, , AddressRangeMemory, TypeStatic)
})
Device (GPE0)
(On i440fx, the low RAM split is at 3GB, in this case. Therefore, with 5GB
guest RAM and no DIMM hotplug range, pc_pci_hole64_start() returns 4 +
(5-3) = 6 GB. Adding the 2GB extension to that yields 8GB, which is below
the firmware-programmed base of 32GB, before the patch. Therefore, before
the patch, the extension is ineffective. After the patch, we add the 2GB
extension to the firmware-programmed base, namely 32GB.)
Using a q35 OVMF guest with 5GB RAM, an example _CRS change is:
@@ -3162,9 +3162,9 @@
QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed,
Cacheable, ReadWrite,
0x0000000000000000, // Granularity
0x0000000800000000, // Range Minimum
- 0x00000009BFFFFFFF, // Range Maximum
+ 0x0000000FFFFFFFFF, // Range Maximum
0x0000000000000000, // Translation Offset
- 0x00000001C0000000, // Length
+ 0x0000000800000000, // Length
,, , AddressRangeMemory, TypeStatic)
})
Device (GPE0)
(On Q35, the low RAM split is at 2GB. Therefore, with 5GB guest RAM and no
DIMM hotplug range, pc_pci_hole64_start() returns 4 + (5-2) = 7 GB. Adding
the 32GB extension to that yields 39GB (0x0000_0009_BFFF_FFFF + 1), before
the patch. After the patch, we add the 32GB extension to the
firmware-programmed base, namely 32GB.)
The ACPI test data for the bios-tables-test case that we added earlier in
this series are corrected too, as follows:
@@ -3339,9 +3339,9 @@
QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed,
Cacheable, ReadWrite,
0x0000000000000000, // Granularity
0x0000000200000000, // Range Minimum
- 0x00000009BFFFFFFF, // Range Maximum
+ 0x00000009FFFFFFFF, // Range Maximum
0x0000000000000000, // Translation Offset
- 0x00000007C0000000, // Length
+ 0x0000000800000000, // Length
,, , AddressRangeMemory, TypeStatic)
})
Device (GPE0)
Cc: "Michael S. Tsirkin" <address@hidden>
Cc: Alex Williamson <address@hidden>
Cc: Gerd Hoffmann <address@hidden>
Cc: Igor Mammedov <address@hidden>
Cc: Marcel Apfelbaum <address@hidden>
Fixes: 9fa99d2519cbf71f871e46871df12cb446dc1c3e
Signed-off-by: Laszlo Ersek <address@hidden>
---
Notes:
v2:
- no code changes
- remove the incorrect claim from the commit message that SeaBIOS is not
affected [Alex]
- update test case data; include textual diff
- drop "Link:" tag pointing to now-stale original discussion
hw/pci-host/piix.c | 2 +-
hw/pci-host/q35.c | 2 +-
tests/acpi-test-data/q35/DSDT.mmio64 | Bin 8947 -> 8947 bytes
3 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 0df91e002076..fb4b0669ac9f 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -285,7 +285,7 @@ static void i440fx_pcihost_get_pci_hole64_end(Object *obj,
Visitor *v,
{
PCIHostState *h = PCI_HOST_BRIDGE(obj);
I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
- uint64_t hole64_start = pc_pci_hole64_start();
+ uint64_t hole64_start = i440fx_pcihost_get_pci_hole64_start_value(obj);
Range w64;
uint64_t value, hole64_end;
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 8acf942b5e65..1c5433161f14 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -145,7 +145,7 @@ static void q35_host_get_pci_hole64_end(Object *obj,
Visitor *v,
{
PCIHostState *h = PCI_HOST_BRIDGE(obj);
Q35PCIHost *s = Q35_HOST_DEVICE(obj);
- uint64_t hole64_start = pc_pci_hole64_start();
+ uint64_t hole64_start = q35_host_get_pci_hole64_start_value(obj);
Range w64;
uint64_t value, hole64_end;
diff --git a/tests/acpi-test-data/q35/DSDT.mmio64 b/tests/acpi-test-data/q35/DSDT.mmio64
index
498c535f2a85a058b76a0a5ebc67c3d5236c3265..a058ff2ee31a22a55b5b198bc1531c7f20b243f6
100644
GIT binary patch
delta 43
mcmezD`q`DsCD<k8vl0UX<Gqbs#}$SCb26X;j?Ir11sDMxoC+%d
delta 43
ncmezD`q`DsCD<k8vl0UXWBf+0<BCH2IT=vE0rt(06$Kap4zmg?