[PATCH qemu] spapr-pci: Provide either correct assigned-addresses or none

Alexey Kardashevskiy posted 1 patch 4 years, 6 months ago
Test docker-clang@ubuntu passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test asan passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190923055119.31683-1-aik@ozlabs.ru
Maintainers: David Gibson <david@gibson.dropbear.id.au>
hw/ppc/spapr_pci.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
[PATCH qemu] spapr-pci: Provide either correct assigned-addresses or none
Posted by Alexey Kardashevskiy 4 years, 6 months ago
QEMU stores current BAR assignments in 2 places - in the raw config space
array and io_regions structs. Once set, the config space array remembers
these but BARs in io_regions are reset/restored every time the device is
disabled/enabled, i.e. when MMIO bit in the command register is flipped.

A sPAPR guest OS normally expects BARs to be assigned by the firmware
and reported to the guest via the "assigned-addresses" property
(below - "aa"). For hotplug devices QEMU creates "aa" and it only does
so if the device is enabled which is odd and relies on the guest linux
kernel ability to assign unassigned resources; other OSes may not do this.

For coldplugged devices QEMU does not provide "aa" as SLOF does BAR
allocation and creates "aa" from the config space values which is ok
for now but since we are going to implement full device tree update on
"ibm,client-architecture-support", we will be having transitions between
QEMU/SLOF/GRUB/Linux and we need to preserve BAR allocations done by SLOF.

This uses non-zero BAR addresses for "aa" ("Unimplemented Base Address
registers are hardwired to zero" says the PCI spec") which preserves
BARs during the boot process.

This only creates "aa" if any BAR was assigned. This violates the
"PCI Bus Binding to Open Firmware" spec from the last millennia which
states:
"If no resources were assigned address space, the "assigned-addresses"
property shall have a prop-encoded-array of zero length".
However this allows older guests to try allocating BARs if for some reason
QEMU or SLOF failed to do so.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr_pci.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 7b71ad7c74f1..2e5f5c52a33a 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -925,13 +925,17 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp)
     reg->size_lo = 0;
 
     for (i = 0; i < PCI_NUM_REGIONS; i++) {
+        hwaddr addr;
+        int bar;
+
         if (!d->io_regions[i].size) {
             continue;
         }
 
+        bar = pci_bar(d, i);
         reg = &rp->reg[reg_idx++];
 
-        reg->phys_hi = cpu_to_be32(dev_id | b_rrrrrrrr(pci_bar(d, i)));
+        reg->phys_hi = cpu_to_be32(dev_id | b_rrrrrrrr(bar));
         if (d->io_regions[i].type & PCI_BASE_ADDRESS_SPACE_IO) {
             reg->phys_hi |= cpu_to_be32(b_ss(1));
         } else if (d->io_regions[i].type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
@@ -944,14 +948,15 @@ static void populate_resource_props(PCIDevice *d, ResourceProps *rp)
         reg->size_hi = cpu_to_be32(d->io_regions[i].size >> 32);
         reg->size_lo = cpu_to_be32(d->io_regions[i].size);
 
-        if (d->io_regions[i].addr == PCI_BAR_UNMAPPED) {
+        addr = pci_get_long(d->config + bar) & ~(d->io_regions[i].size - 1);
+        if (!addr) {
             continue;
         }
 
         assigned = &rp->assigned[assigned_idx++];
         assigned->phys_hi = cpu_to_be32(be32_to_cpu(reg->phys_hi) | b_n(1));
-        assigned->phys_mid = cpu_to_be32(d->io_regions[i].addr >> 32);
-        assigned->phys_lo = cpu_to_be32(d->io_regions[i].addr);
+        assigned->phys_mid = cpu_to_be32(addr >> 32);
+        assigned->phys_lo = cpu_to_be32(addr);
         assigned->size_hi = reg->size_hi;
         assigned->size_lo = reg->size_lo;
     }
@@ -1471,8 +1476,10 @@ static int spapr_dt_pci_device(SpaprPhbState *sphb, PCIDevice *dev,
 
     populate_resource_props(dev, &rp);
     _FDT(fdt_setprop(fdt, offset, "reg", (uint8_t *)rp.reg, rp.reg_len));
-    _FDT(fdt_setprop(fdt, offset, "assigned-addresses",
-                     (uint8_t *)rp.assigned, rp.assigned_len));
+    if (rp.assigned_len) {
+        _FDT(fdt_setprop(fdt, offset, "assigned-addresses",
+                         (uint8_t *)rp.assigned, rp.assigned_len));
+    }
 
     if (sphb->pcie_ecs && pci_is_express(dev)) {
         _FDT(fdt_setprop_cell(fdt, offset, "ibm,pci-config-space-type", 0x1));
-- 
2.17.1