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

[Nikunj A Dadhania]

David Gibson <address@hidden> writes:

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

Sure, will rearrange the patch.

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