diff mbox

[v3,5/6] spapr_pci: fix boot-time device tree fields for pci hotplug

Message ID 1430816036-26408-6-git-send-email-nikunj@linux.vnet.ibm.com
State New
Headers show

Commit Message

Nikunj A Dadhania May 5, 2015, 8:53 a.m. UTC 
From: Michael Roth <mdroth@linux.vnet.ibm.com>

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 <mdroth@linux.vnet.ibm.com>
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/ppc/spapr_pci.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

Comments

David Gibson May 5, 2015, 1:09 p.m. UTC | #1 
On Tue, May 05, 2015 at 02:23:55PM +0530, Nikunj A Dadhania wrote:
> From: Michael Roth <mdroth@linux.vnet.ibm.com>
> 
> 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 <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  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, "pci@%d,%d", slot, func);
> @@ -1506,7 +1523,7 @@ static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev,
>          sprintf(nodename, "pci@%d", 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) !=
Nikunj A Dadhania May 6, 2015, 5:57 a.m. UTC | #2 
David Gibson <david@gibson.dropbear.id.au> writes:

> On Tue, May 05, 2015 at 02:23:55PM +0530, Nikunj A Dadhania wrote:
>> From: Michael Roth <mdroth@linux.vnet.ibm.com>
>> 
>> 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 <mdroth@linux.vnet.ibm.com>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>  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, "pci@%d,%d", slot, func);
>> @@ -1506,7 +1523,7 @@ static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev,
>>          sprintf(nodename, "pci@%d", 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
diff mbox

Patch

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);
 
     if (func) {
         sprintf(nodename, "pci@%d,%d", slot, func);
@@ -1506,7 +1523,7 @@  static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev,
         sprintf(nodename, "pci@%d", 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) !=