Re: [Qemu-devel] [PATCH v3 5/6] spapr_pci: fix boot-time device tree fields for pci hotplug

[David Gibson]

On Tue, May 05, 2015 at 02:23:55PM +0530, Nikunj A Dadhania wrote:
> From: Michael Roth <address@hidden>
> 
> We need to set the proper drc_index values in ibm,my-drc-index
> fields in order to allow a PCI device that was present at
> boot-time to be unplugged.
> 
> Previously SLOF handles this, but with QEMU handling the DT we
> need to do it there as well.
> 
> This patch slightly changes how SLOF handled it in the past,
> which was to allows add an ibm,my-drc-index value based on
> PCI slot/devices topology. Now we only add it when the slot
> supports hotplug and has a DR connector, which is more inline
> with PAPR.
> 
> Signed-off-by: Michael Roth <address@hidden>
> Signed-off-by: Nikunj A Dadhania <address@hidden>
> ---
>  hw/ppc/spapr_pci.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 103284a..cbd5661 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -951,7 +951,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, 
> void *fdt, int offset,
>          _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name,
>                           strlen(drc_name)));
>      }
> -    _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
> +    if (drc_index) {
> +        _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
> +    }
>  
>      _FDT(fdt_setprop_cell(fdt, offset, "#address-cells",
>                            RESOURCE_CELLS_ADDRESS));
> @@ -1483,6 +1485,20 @@ PCIHostState *spapr_create_phb(sPAPREnvironment 
> *spapr, int index)
>      return PCI_HOST_BRIDGE(dev);
>  }
>  
> +static uint32_t spapr_phb_get_pci_drc_index(sPAPRPHBState *phb,
> +                                            PCIDevice *pdev)
> +{
> +    sPAPRDRConnector *drc = spapr_phb_get_pci_drc(phb, pdev);
> +    sPAPRDRConnectorClass *drck;
> +
> +    if (!drc) {
> +        return 0;
> +    }
> +
> +    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> +    return drck->get_index(drc);
> +}
> +
>  typedef struct sPAPRFDT {
>      void *fdt;
>      int node_off;
> @@ -1499,6 +1515,7 @@ static void spapr_populate_pci_devices_dt(PCIBus *bus, 
> PCIDevice *pdev,
>      int func = PCI_FUNC(pdev->devfn);
>      char nodename[512];
>      sPAPRFDT s_fdt;
> +    uint32_t drc_index = spapr_phb_get_pci_drc_index(p->sphb, pdev);

The line above causes a compile error, since sPAPRFDT doesn't have an
sphb member.

Its fixed in the next patch, but can you adjust the series so it won't
break bisection.


>      if (func) {
>          sprintf(nodename, "address@hidden,%d", slot, func);
> @@ -1506,7 +1523,7 @@ static void spapr_populate_pci_devices_dt(PCIBus *bus, 
> PCIDevice *pdev,
>          sprintf(nodename, "address@hidden", slot);
>      }
>      offset = fdt_add_subnode(p->fdt, p->node_off, nodename);
> -    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->index, 0, 
> NULL);
> +    ret = spapr_populate_pci_child_dt(pdev, p->fdt, offset, p->index, 
> drc_index, NULL);
>      g_assert(!ret);
>  
>      if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) !=

-- 
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

pgpDyrdrfE7Cq.pgp
Description: PGP signature