On Fri, Sep 27, 2019 at 12:26:51PM +1000, Alexey Kardashevskiy wrote: > QEMU does not allocate PCI resources (BARs) in any case - coldplug devices > are configured by the firmware and hotplug devices rely on the guest > system to do the assignment via the PCI rescan mechanism. Also in order > to create non empty "assigned-addresses", the device has to be enabled > (i.e. PCI_COMMAND needs the MMIO bit set) first as otherwise > io_regions[i].addr are -1, and devices are not enabled at this point. > > This removes "assigned-addresses" and leaves it to those who actually > do resource allocation. > > Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru> > --- > > This replaces: > [PATCH qemu] spapr-pci: Provide either correct assigned-addresses or > none
Applied to ppc-for-4.2, along with your earlier full fdt render patch,
which this now unbreaks.
>
> ---
> hw/ppc/spapr_pci.c | 42 +++++++-----------------------------------
> 1 file changed, 7 insertions(+), 35 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 6934d506a7e9..01ff41d4c43f 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -836,7 +836,7 @@ static char *spapr_phb_get_loc_code(SpaprPhbState *sphb,
> PCIDevice *pdev)
> #define b_fff(x) b_x((x), 8, 3) /* function number */
> #define b_rrrrrrrr(x) b_x((x), 0, 8) /* register number */
>
> -/* for 'reg'/'assigned-addresses' OF properties */
> +/* for 'reg' OF properties */
> #define RESOURCE_CELLS_SIZE 2
> #define RESOURCE_CELLS_ADDRESS 3
>
> @@ -850,17 +850,14 @@ typedef struct ResourceFields {
>
> typedef struct ResourceProps {
> ResourceFields reg[8];
> - ResourceFields assigned[7];
> uint32_t reg_len;
> - uint32_t assigned_len;
> } ResourceProps;
>
> -/* fill in the 'reg'/'assigned-resources' OF properties for
> +/* fill in the 'reg' OF properties for
> * a PCI device. 'reg' describes resource requirements for a
> - * device's IO/MEM regions, 'assigned-addresses' describes the
> - * actual resource assignments.
> + * device's IO/MEM regions.
> *
> - * the properties are arrays of ('phys-addr', 'size') pairs describing
> + * the property is an array of ('phys-addr', 'size') pairs describing
> * the addressable regions of the PCI device, where 'phys-addr' is a
> * RESOURCE_CELLS_ADDRESS-tuple of 32-bit integers corresponding to
> * (phys.hi, phys.mid, phys.lo), and 'size' is a
> @@ -889,18 +886,7 @@ typedef struct ResourceProps {
> * phys.mid and phys.lo correspond respectively to the hi/lo portions
> * of the actual address of the region.
> *
> - * how the phys-addr/size values are used differ slightly between
> - * 'reg' and 'assigned-addresses' properties. namely, 'reg' has
> - * an additional description for the config space region of the
> - * device, and in the case of QEMU has n=0 and phys.mid=phys.lo=0
> - * to describe the region as relocatable, with an address-mapping
> - * that corresponds directly to the PHB's address space for the
> - * resource. 'assigned-addresses' always has n=1 set with an absolute
> - * address assigned for the resource. in general, 'assigned-addresses'
> - * won't be populated, since addresses for PCI devices are generally
> - * unmapped initially and left to the guest to assign.
> - *
> - * note also that addresses defined in these properties are, at least
> + * note also that addresses defined in this property are, at least
> * for PAPR guests, relative to the PHBs IO/MEM windows, and
> * correspond directly to the addresses in the BARs.
> *
> @@ -914,8 +900,8 @@ static void populate_resource_props(PCIDevice *d,
> ResourceProps *rp)
> uint32_t dev_id = (b_bbbbbbbb(bus_num) |
> b_ddddd(PCI_SLOT(d->devfn)) |
> b_fff(PCI_FUNC(d->devfn)));
> - ResourceFields *reg, *assigned;
> - int i, reg_idx = 0, assigned_idx = 0;
> + ResourceFields *reg;
> + int i, reg_idx = 0;
>
> /* config space region */
> reg = &rp->reg[reg_idx++];
> @@ -944,21 +930,9 @@ static void populate_resource_props(PCIDevice *d,
> ResourceProps *rp)
> reg->phys_lo = 0;
> 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) {
> - 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->size_hi = reg->size_hi;
> - assigned->size_lo = reg->size_lo;
> }
>
> rp->reg_len = reg_idx * sizeof(ResourceFields);
> - rp->assigned_len = assigned_idx * sizeof(ResourceFields);
> }
>
> typedef struct PCIClass PCIClass;
> @@ -1472,8 +1446,6 @@ 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 (sphb->pcie_ecs && pci_is_express(dev)) {
> _FDT(fdt_setprop_cell(fdt, offset, "ibm,pci-config-space-type",
> 0x1));
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
