qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH qemu] spapr-pci: Provide either correct assigned-addresses or none
@ 2019-09-23  5:51 Alexey Kardashevskiy
  0 siblings, 0 replies; only message in thread
From: Alexey Kardashevskiy @ 2019-09-23  5:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Sam Bobroff, qemu-ppc, David Gibson

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



^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2019-09-23  5:52 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-23  5:51 [PATCH qemu] spapr-pci: Provide either correct assigned-addresses or none Alexey Kardashevskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).