| Message ID | 1429698934-28915-2-git-send-email-nikunj@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 22, 2015 at 04:05:33PM +0530, Nikunj A Dadhania wrote: > All the PCI enumeration and device node creation was off-loaded to > SLOF. With PCI hotplug support, code needed to be added to add device > node. This creates multiple copy of the code one in SLOF and other in > hotplug code. To unify this, the patch adds the pci device node > creation in Qemu. For backward compatibility, a flag > "qemu,phb-enumerated" is added to the phb, suggesting to SLOF to not > do device node creation. > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > --- > hw/ppc/spapr_pci.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 84 insertions(+), 9 deletions(-) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 3796d54..abf71f7 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -23,6 +23,7 @@ > * THE SOFTWARE. > */ > #include "hw/hw.h" > +#include "hw/sysbus.h" > #include "hw/pci/pci.h" > #include "hw/pci/msi.h" > #include "hw/pci/msix.h" > @@ -35,6 +36,7 @@ > #include "qemu/error-report.h" > #include "qapi/qmp/qerror.h" > > +#include "hw/pci/pci_bridge.h" > #include "hw/pci/pci_bus.h" > #include "hw/ppc/spapr_drc.h" > #include "sysemu/device_tree.h" > @@ -938,7 +940,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > * processed by OF beforehand > */ > _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); > - _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name))); > + if (drc_name) { > + _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)); > > _FDT(fdt_setprop_cell(fdt, offset, "#address-cells", > @@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector *drc, > void *fdt = NULL; > int fdt_start_offset = 0; > > - /* boot-time devices get their device tree node created by SLOF, but for > - * hotplugged devices we need QEMU to generate it so the guest can fetch > - * it via RTAS > - */ > if (dev->hotplugged) { > fdt = spapr_create_pci_child_dt(phb, pdev, drc_index, drc_name, > &fdt_start_offset); > @@ -1473,14 +1473,15 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index) > return PCI_HOST_BRIDGE(dev); > } > > -typedef struct sPAPRTCEDT { > +typedef struct sPAPRFDT { > void *fdt; > int node_off; > -} sPAPRTCEDT; > + uint32_t index; > +} sPAPRFDT; > > static int spapr_phb_children_dt(Object *child, void *opaque) > { > - sPAPRTCEDT *p = opaque; > + sPAPRFDT *p = opaque; > sPAPRTCETable *tcet; > > tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE); > @@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, void *opaque) > return 1; > } > > +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *opaque) > +{ > + sPAPRFDT *p = opaque; > + int ret, offset; > + int slot = PCI_SLOT(pdev->devfn); > + int func = PCI_FUNC(pdev->devfn); > + char nodename[512]; > + > + if (func) { > + sprintf(nodename, "pci@%d,%d", slot, func); > + } else { > + 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); > + g_assert(!ret); > + > + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) == > + PCI_HEADER_TYPE_BRIDGE)) { > + PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); > + if(sec_bus) { You have several minor indent style problems - please run through scripts/checkpatch.pl and correct accordingly. > + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), > + spapr_populate_pci_devices_dt, > + &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index })); The inline structure definition is pretty hard to read. I'd prefer to see that declared as a local. Otherwise, this looks ok. > + } > + } > + return; > +} > + > +static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, void *opaque) > +{ > + unsigned short *bus_no = (unsigned short *) opaque; > + unsigned short primary = *bus_no; > + unsigned short secondary; > + unsigned short subordinate = 0xff; > + > + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) == > + PCI_HEADER_TYPE_BRIDGE)) { > + PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); > + secondary = *bus_no + 1; > + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); > + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1); > + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, secondary, 1); > + *bus_no = *bus_no + 1; > + if (sec_bus) { > + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); > + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1); > + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1); > + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), > + spapr_phb_pci_enumerate_bridge, > + bus_no); > + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1); > + } > + } > +} > + > +static void spapr_phb_pci_enumerate(sPAPRPHBState *phb) > +{ > + PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus; > + unsigned short bus_no = 0; > + > + pci_for_each_device(bus, pci_bus_num(bus), > + spapr_phb_pci_enumerate_bridge, > + &bus_no); > + > +} > + > int spapr_populate_pci_dt(sPAPRPHBState *phb, > uint32_t xics_phandle, > void *fdt) > @@ -1534,6 +1602,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, > uint32_t interrupt_map_mask[] = { > cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)}; > uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7]; > + PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus; > > /* Start populating the FDT */ > sprintf(nodename, "pci@%" PRIx64, phb->buid); > @@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, > sizeof(interrupt_map))); > > object_child_foreach(OBJECT(phb), spapr_phb_children_dt, > - &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off })); > + &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off })); > + > + spapr_phb_pci_enumerate(phb); > + _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1)); > + pci_for_each_device(bus, pci_bus_num(bus), > + spapr_populate_pci_devices_dt, > + &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index })); > > ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb), > SPAPR_DR_CONNECTOR_TYPE_PCI);
David Gibson <david@gibson.dropbear.id.au> writes: > On Wed, Apr 22, 2015 at 04:05:33PM +0530, Nikunj A Dadhania wrote: >> All the PCI enumeration and device node creation was off-loaded to >> SLOF. With PCI hotplug support, code needed to be added to add device >> node. This creates multiple copy of the code one in SLOF and other in >> hotplug code. To unify this, the patch adds the pci device node >> creation in Qemu. For backward compatibility, a flag >> "qemu,phb-enumerated" is added to the phb, suggesting to SLOF to not >> do device node creation. >> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >> --- >> hw/ppc/spapr_pci.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++------ >> 1 file changed, 84 insertions(+), 9 deletions(-) >> >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >> index 3796d54..abf71f7 100644 >> --- a/hw/ppc/spapr_pci.c >> +++ b/hw/ppc/spapr_pci.c >> @@ -23,6 +23,7 @@ >> * THE SOFTWARE. >> */ >> #include "hw/hw.h" >> +#include "hw/sysbus.h" >> #include "hw/pci/pci.h" >> #include "hw/pci/msi.h" >> #include "hw/pci/msix.h" >> @@ -35,6 +36,7 @@ >> #include "qemu/error-report.h" >> #include "qapi/qmp/qerror.h" >> >> +#include "hw/pci/pci_bridge.h" >> #include "hw/pci/pci_bus.h" >> #include "hw/ppc/spapr_drc.h" >> #include "sysemu/device_tree.h" >> @@ -938,7 +940,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, >> * processed by OF beforehand >> */ >> _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); >> - _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name))); >> + if (drc_name) { >> + _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)); >> >> _FDT(fdt_setprop_cell(fdt, offset, "#address-cells", >> @@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector *drc, >> void *fdt = NULL; >> int fdt_start_offset = 0; >> >> - /* boot-time devices get their device tree node created by SLOF, but for >> - * hotplugged devices we need QEMU to generate it so the guest can fetch >> - * it via RTAS >> - */ >> if (dev->hotplugged) { >> fdt = spapr_create_pci_child_dt(phb, pdev, drc_index, drc_name, >> &fdt_start_offset); >> @@ -1473,14 +1473,15 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index) >> return PCI_HOST_BRIDGE(dev); >> } >> >> -typedef struct sPAPRTCEDT { >> +typedef struct sPAPRFDT { >> void *fdt; >> int node_off; >> -} sPAPRTCEDT; >> + uint32_t index; >> +} sPAPRFDT; >> >> static int spapr_phb_children_dt(Object *child, void *opaque) >> { >> - sPAPRTCEDT *p = opaque; >> + sPAPRFDT *p = opaque; >> sPAPRTCETable *tcet; >> >> tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE); >> @@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, void *opaque) >> return 1; >> } >> >> +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *opaque) >> +{ >> + sPAPRFDT *p = opaque; >> + int ret, offset; >> + int slot = PCI_SLOT(pdev->devfn); >> + int func = PCI_FUNC(pdev->devfn); >> + char nodename[512]; >> + >> + if (func) { >> + sprintf(nodename, "pci@%d,%d", slot, func); >> + } else { >> + 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); >> + g_assert(!ret); >> + >> + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) == >> + PCI_HEADER_TYPE_BRIDGE)) { >> + PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); >> + if(sec_bus) { > > You have several minor indent style problems - please run through > scripts/checkpatch.pl and correct accordingly. Sure, will do. > >> + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), >> + spapr_populate_pci_devices_dt, >> + &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index })); > > The inline structure definition is pretty hard to read. I'd prefer to > see that declared as a local. Will update it. > > Otherwise, this looks ok. > >> + } >> + } >> + return; >> +} >> + >> +static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, void *opaque) >> +{ >> + unsigned short *bus_no = (unsigned short *) opaque; >> + unsigned short primary = *bus_no; >> + unsigned short secondary; >> + unsigned short subordinate = 0xff; >> + >> + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) == >> + PCI_HEADER_TYPE_BRIDGE)) { >> + PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); >> + secondary = *bus_no + 1; >> + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); >> + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1); >> + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, secondary, 1); >> + *bus_no = *bus_no + 1; >> + if (sec_bus) { >> + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); >> + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1); >> + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1); >> + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), >> + spapr_phb_pci_enumerate_bridge, >> + bus_no); >> + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1); >> + } >> + } >> +} >> + >> +static void spapr_phb_pci_enumerate(sPAPRPHBState *phb) >> +{ >> + PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus; >> + unsigned short bus_no = 0; >> + >> + pci_for_each_device(bus, pci_bus_num(bus), >> + spapr_phb_pci_enumerate_bridge, >> + &bus_no); >> + >> +} >> + >> int spapr_populate_pci_dt(sPAPRPHBState *phb, >> uint32_t xics_phandle, >> void *fdt) >> @@ -1534,6 +1602,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, >> uint32_t interrupt_map_mask[] = { >> cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)}; >> uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7]; >> + PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus; >> >> /* Start populating the FDT */ >> sprintf(nodename, "pci@%" PRIx64, phb->buid); >> @@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, >> sizeof(interrupt_map))); >> >> object_child_foreach(OBJECT(phb), spapr_phb_children_dt, >> - &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off })); >> + &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off })); >> + >> + spapr_phb_pci_enumerate(phb); >> + _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1)); >> + pci_for_each_device(bus, pci_bus_num(bus), >> + spapr_populate_pci_devices_dt, >> + &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index })); >> >> ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb), >> SPAPR_DR_CONNECTOR_TYPE_PCI); > > -- > 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
On 04/22/2015 08:35 PM, Nikunj A Dadhania wrote: > All the PCI enumeration and device node creation was off-loaded to > SLOF. With PCI hotplug support, code needed to be added to add device > node. This creates multiple copy of the code one in SLOF and other in > hotplug code. To unify this, the patch adds the pci device node > creation in Qemu. For backward compatibility, a flag > "qemu,phb-enumerated" is added to the phb, suggesting to SLOF to not > do device node creation. > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > --- > hw/ppc/spapr_pci.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 84 insertions(+), 9 deletions(-) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 3796d54..abf71f7 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -23,6 +23,7 @@ > * THE SOFTWARE. > */ > #include "hw/hw.h" > +#include "hw/sysbus.h" > #include "hw/pci/pci.h" > #include "hw/pci/msi.h" > #include "hw/pci/msix.h" > @@ -35,6 +36,7 @@ > #include "qemu/error-report.h" > #include "qapi/qmp/qerror.h" > > +#include "hw/pci/pci_bridge.h" > #include "hw/pci/pci_bus.h" > #include "hw/ppc/spapr_drc.h" > #include "sysemu/device_tree.h" > @@ -938,7 +940,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, > * processed by OF beforehand > */ > _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); > - _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name))); > + if (drc_name) { Where did drc_name come from? By now, there is no "loc-code" patch in spapr-next, or there is? > + _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)); > > _FDT(fdt_setprop_cell(fdt, offset, "#address-cells", > @@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector *drc, > void *fdt = NULL; > int fdt_start_offset = 0; > > - /* boot-time devices get their device tree node created by SLOF, but for > - * hotplugged devices we need QEMU to generate it so the guest can fetch > - * it via RTAS > - */ > if (dev->hotplugged) { > fdt = spapr_create_pci_child_dt(phb, pdev, drc_index, drc_name, > &fdt_start_offset); > @@ -1473,14 +1473,15 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index) > return PCI_HOST_BRIDGE(dev); > } > > -typedef struct sPAPRTCEDT { > +typedef struct sPAPRFDT { > void *fdt; > int node_off; > -} sPAPRTCEDT; > + uint32_t index; > +} sPAPRFDT; You definitely will have to rebase it - the sPAPRTCEDT is gone in what I posted last time (and what I hope will get to spapr-next first :) ). > static int spapr_phb_children_dt(Object *child, void *opaque) > { > - sPAPRTCEDT *p = opaque; > + sPAPRFDT *p = opaque; > sPAPRTCETable *tcet; > > tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE); > @@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, void *opaque) > return 1; > } > > +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *opaque) > +{ > + sPAPRFDT *p = opaque; > + int ret, offset; > + int slot = PCI_SLOT(pdev->devfn); > + int func = PCI_FUNC(pdev->devfn); > + char nodename[512]; > + > + if (func) { > + sprintf(nodename, "pci@%d,%d", slot, func); > + } else { > + 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); > + g_assert(!ret); > + > + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) == > + PCI_HEADER_TYPE_BRIDGE)) { Since you'll be doing respin anyway, I suggest changing "==" to "!=", add return here and reduce indents. And below, and further down. > + PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); > + if(sec_bus) { > + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), > + spapr_populate_pci_devices_dt, > + &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index })); > + } > + } > + return; > +} > + > +static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, void *opaque) > +{ > + unsigned short *bus_no = (unsigned short *) opaque; > + unsigned short primary = *bus_no; > + unsigned short secondary; > + unsigned short subordinate = 0xff; > + > + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) == > + PCI_HEADER_TYPE_BRIDGE)) { > + PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); > + secondary = *bus_no + 1; > + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); > + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1); > + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, secondary, 1); > + *bus_no = *bus_no + 1; > + if (sec_bus) { > + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); > + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1); > + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1); > + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), > + spapr_phb_pci_enumerate_bridge, > + bus_no); > + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1); > + } > + } > +} > + > +static void spapr_phb_pci_enumerate(sPAPRPHBState *phb) > +{ > + PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus; > + unsigned short bus_no = 0; > + > + pci_for_each_device(bus, pci_bus_num(bus), > + spapr_phb_pci_enumerate_bridge, > + &bus_no); > + > +} > + > int spapr_populate_pci_dt(sPAPRPHBState *phb, > uint32_t xics_phandle, > void *fdt) > @@ -1534,6 +1602,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, > uint32_t interrupt_map_mask[] = { > cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)}; > uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7]; > + PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus; > > /* Start populating the FDT */ > sprintf(nodename, "pci@%" PRIx64, phb->buid); > @@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, > sizeof(interrupt_map))); > > object_child_foreach(OBJECT(phb), spapr_phb_children_dt, > - &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off })); > + &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off })); > + > + spapr_phb_pci_enumerate(phb); > + _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1)); Cannot SLOF see that there are nodes already and just not touch them? > + pci_for_each_device(bus, pci_bus_num(bus), > + spapr_populate_pci_devices_dt, > + &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index })); > > ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb), > SPAPR_DR_CONNECTOR_TYPE_PCI); >
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > On 04/22/2015 08:35 PM, Nikunj A Dadhania wrote: >> All the PCI enumeration and device node creation was off-loaded to >> SLOF. With PCI hotplug support, code needed to be added to add device >> node. This creates multiple copy of the code one in SLOF and other in >> hotplug code. To unify this, the patch adds the pci device node >> creation in Qemu. For backward compatibility, a flag >> "qemu,phb-enumerated" is added to the phb, suggesting to SLOF to not >> do device node creation. >> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >> --- >> hw/ppc/spapr_pci.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++------ >> 1 file changed, 84 insertions(+), 9 deletions(-) >> >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >> index 3796d54..abf71f7 100644 >> --- a/hw/ppc/spapr_pci.c >> +++ b/hw/ppc/spapr_pci.c >> @@ -23,6 +23,7 @@ >> * THE SOFTWARE. >> */ >> #include "hw/hw.h" >> +#include "hw/sysbus.h" >> #include "hw/pci/pci.h" >> #include "hw/pci/msi.h" >> #include "hw/pci/msix.h" >> @@ -35,6 +36,7 @@ >> #include "qemu/error-report.h" >> #include "qapi/qmp/qerror.h" >> >> +#include "hw/pci/pci_bridge.h" >> #include "hw/pci/pci_bus.h" >> #include "hw/ppc/spapr_drc.h" >> #include "sysemu/device_tree.h" >> @@ -938,7 +940,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, >> * processed by OF beforehand >> */ >> _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); >> - _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name))); >> + if (drc_name) { > > > Where did drc_name come from? By now, there is no "loc-code" patch in > spapr-next, or there is? loc-code is not there yet, but the hotplug code generates its own name and sends as drc_name. I am getting rid of this in the next patch. > > >> + _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)); >> >> _FDT(fdt_setprop_cell(fdt, offset, "#address-cells", >> @@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector *drc, >> void *fdt = NULL; >> int fdt_start_offset = 0; >> >> - /* boot-time devices get their device tree node created by SLOF, but for >> - * hotplugged devices we need QEMU to generate it so the guest can fetch >> - * it via RTAS >> - */ >> if (dev->hotplugged) { >> fdt = spapr_create_pci_child_dt(phb, pdev, drc_index, drc_name, >> &fdt_start_offset); >> @@ -1473,14 +1473,15 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index) >> return PCI_HOST_BRIDGE(dev); >> } >> >> -typedef struct sPAPRTCEDT { >> +typedef struct sPAPRFDT { >> void *fdt; >> int node_off; >> -} sPAPRTCEDT; >> + uint32_t index; >> +} sPAPRFDT; > > You definitely will have to rebase it - the sPAPRTCEDT is gone in what I > posted last time (and what I hope will get to spapr-next first :) ). Sure, that should be fine. > > > >> static int spapr_phb_children_dt(Object *child, void *opaque) >> { >> - sPAPRTCEDT *p = opaque; >> + sPAPRFDT *p = opaque; >> sPAPRTCETable *tcet; >> >> tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE); >> @@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, void *opaque) >> return 1; >> } >> >> +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *opaque) >> +{ >> + sPAPRFDT *p = opaque; >> + int ret, offset; >> + int slot = PCI_SLOT(pdev->devfn); >> + int func = PCI_FUNC(pdev->devfn); >> + char nodename[512]; >> + >> + if (func) { >> + sprintf(nodename, "pci@%d,%d", slot, func); >> + } else { >> + 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); >> + g_assert(!ret); >> + >> + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) == >> + PCI_HEADER_TYPE_BRIDGE)) { > > > Since you'll be doing respin anyway, I suggest changing "==" to "!=", add > return here and reduce indents. And below, and further down. Yes, will be better. > > > >> + PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); >> + if(sec_bus) { >> + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), >> + spapr_populate_pci_devices_dt, >> + &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index })); >> + } >> + } >> + return; >> +} >> + >> +static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, void *opaque) >> +{ >> + unsigned short *bus_no = (unsigned short *) opaque; >> + unsigned short primary = *bus_no; >> + unsigned short secondary; >> + unsigned short subordinate = 0xff; >> + >> + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) == >> + PCI_HEADER_TYPE_BRIDGE)) { >> + PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); >> + secondary = *bus_no + 1; >> + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); >> + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1); >> + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, secondary, 1); >> + *bus_no = *bus_no + 1; >> + if (sec_bus) { >> + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); >> + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1); >> + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1); >> + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), >> + spapr_phb_pci_enumerate_bridge, >> + bus_no); >> + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1); >> + } >> + } >> +} >> + >> +static void spapr_phb_pci_enumerate(sPAPRPHBState *phb) >> +{ >> + PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus; >> + unsigned short bus_no = 0; >> + >> + pci_for_each_device(bus, pci_bus_num(bus), >> + spapr_phb_pci_enumerate_bridge, >> + &bus_no); >> + >> +} >> + >> int spapr_populate_pci_dt(sPAPRPHBState *phb, >> uint32_t xics_phandle, >> void *fdt) >> @@ -1534,6 +1602,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, >> uint32_t interrupt_map_mask[] = { >> cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)}; >> uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7]; >> + PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus; >> >> /* Start populating the FDT */ >> sprintf(nodename, "pci@%" PRIx64, phb->buid); >> @@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, >> sizeof(interrupt_map))); >> >> object_child_foreach(OBJECT(phb), spapr_phb_children_dt, >> - &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off })); >> + &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off })); >> + >> + spapr_phb_pci_enumerate(phb); >> + _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1)); > > > Cannot SLOF see that there are nodes already and just not touch them? This is more explicit. So that in absence of this, I can kick in old SLOF enumeration code and in presence, start reading the nodes. > > >> + pci_for_each_device(bus, pci_bus_num(bus), >> + spapr_populate_pci_devices_dt, >> + &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index })); >> >> ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb), >> SPAPR_DR_CONNECTOR_TYPE_PCI); >> > > > -- > Alexey Regards Nikunj
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 3796d54..abf71f7 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -23,6 +23,7 @@ * THE SOFTWARE. */ #include "hw/hw.h" +#include "hw/sysbus.h" #include "hw/pci/pci.h" #include "hw/pci/msi.h" #include "hw/pci/msix.h" @@ -35,6 +36,7 @@ #include "qemu/error-report.h" #include "qapi/qmp/qerror.h" +#include "hw/pci/pci_bridge.h" #include "hw/pci/pci_bus.h" #include "hw/ppc/spapr_drc.h" #include "sysemu/device_tree.h" @@ -938,7 +940,9 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, * processed by OF beforehand */ _FDT(fdt_setprop_string(fdt, offset, "name", "pci")); - _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name, strlen(drc_name))); + if (drc_name) { + _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)); _FDT(fdt_setprop_cell(fdt, offset, "#address-cells", @@ -994,10 +998,6 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector *drc, void *fdt = NULL; int fdt_start_offset = 0; - /* boot-time devices get their device tree node created by SLOF, but for - * hotplugged devices we need QEMU to generate it so the guest can fetch - * it via RTAS - */ if (dev->hotplugged) { fdt = spapr_create_pci_child_dt(phb, pdev, drc_index, drc_name, &fdt_start_offset); @@ -1473,14 +1473,15 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index) return PCI_HOST_BRIDGE(dev); } -typedef struct sPAPRTCEDT { +typedef struct sPAPRFDT { void *fdt; int node_off; -} sPAPRTCEDT; + uint32_t index; +} sPAPRFDT; static int spapr_phb_children_dt(Object *child, void *opaque) { - sPAPRTCEDT *p = opaque; + sPAPRFDT *p = opaque; sPAPRTCETable *tcet; tcet = (sPAPRTCETable *) object_dynamic_cast(child, TYPE_SPAPR_TCE_TABLE); @@ -1496,6 +1497,73 @@ static int spapr_phb_children_dt(Object *child, void *opaque) return 1; } +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev, void *opaque) +{ + sPAPRFDT *p = opaque; + int ret, offset; + int slot = PCI_SLOT(pdev->devfn); + int func = PCI_FUNC(pdev->devfn); + char nodename[512]; + + if (func) { + sprintf(nodename, "pci@%d,%d", slot, func); + } else { + 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); + g_assert(!ret); + + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) == + PCI_HEADER_TYPE_BRIDGE)) { + PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); + if(sec_bus) { + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), + spapr_populate_pci_devices_dt, + &((sPAPRFDT){ .fdt = p->fdt, .node_off = offset , .index = p->index })); + } + } + return; +} + +static void spapr_phb_pci_enumerate_bridge(PCIBus *bus, PCIDevice *pdev, void *opaque) +{ + unsigned short *bus_no = (unsigned short *) opaque; + unsigned short primary = *bus_no; + unsigned short secondary; + unsigned short subordinate = 0xff; + + if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) == + PCI_HEADER_TYPE_BRIDGE)) { + PCIBus *sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); + secondary = *bus_no + 1; + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1); + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, secondary, 1); + *bus_no = *bus_no + 1; + if (sec_bus) { + pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1); + pci_default_write_config(pdev, PCI_SECONDARY_BUS, secondary, 1); + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1); + pci_for_each_device(sec_bus, pci_bus_num(sec_bus), + spapr_phb_pci_enumerate_bridge, + bus_no); + pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1); + } + } +} + +static void spapr_phb_pci_enumerate(sPAPRPHBState *phb) +{ + PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus; + unsigned short bus_no = 0; + + pci_for_each_device(bus, pci_bus_num(bus), + spapr_phb_pci_enumerate_bridge, + &bus_no); + +} + int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void *fdt) @@ -1534,6 +1602,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t interrupt_map_mask[] = { cpu_to_be32(b_ddddd(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)}; uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7]; + PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus; /* Start populating the FDT */ sprintf(nodename, "pci@%" PRIx64, phb->buid); @@ -1579,7 +1648,13 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, sizeof(interrupt_map))); object_child_foreach(OBJECT(phb), spapr_phb_children_dt, - &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off })); + &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off })); + + spapr_phb_pci_enumerate(phb); + _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1)); + pci_for_each_device(bus, pci_bus_num(bus), + spapr_populate_pci_devices_dt, + &((sPAPRFDT){ .fdt = fdt, .node_off = bus_off, .index = phb->index })); ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb), SPAPR_DR_CONNECTOR_TYPE_PCI);
All the PCI enumeration and device node creation was off-loaded to SLOF. With PCI hotplug support, code needed to be added to add device node. This creates multiple copy of the code one in SLOF and other in hotplug code. To unify this, the patch adds the pci device node creation in Qemu. For backward compatibility, a flag "qemu,phb-enumerated" is added to the phb, suggesting to SLOF to not do device node creation. Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> --- hw/ppc/spapr_pci.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 84 insertions(+), 9 deletions(-)