[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 3/6] spapr_pci: enumerate and add PCI device
From: |
Nikunj A Dadhania |
Subject: |
Re: [Qemu-devel] [PATCH v7 3/6] spapr_pci: enumerate and add PCI device tree |
Date: |
Wed, 17 Jun 2015 14:12:14 +0530 |
User-agent: |
Notmuch/0.17+27~gae47d61 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-redhat-linux-gnu) |
David Gibson <address@hidden> writes:
> On Thu, Jun 11, 2015 at 04:32:26PM +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 <address@hidden>
>> [ Squashed Michael's drc_index changes ]
>> Signed-off-by: Michael Roth <address@hidden>
>> Signed-off-by: Nikunj A Dadhania <address@hidden>
>> ---
>> hw/ppc/spapr_pci.c | 167
>> +++++++++++++++++++++++++++++++++++++++++++++--------
>> 1 file changed, 142 insertions(+), 25 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index 33254b3..6ef7f44 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"
>> @@ -946,8 +948,13 @@ 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)));
>> - _FDT(fdt_setprop_cell(fdt, offset, "ibm,my-drc-index", drc_index));
>> + if (drc_name) {
>> + _FDT(fdt_setprop(fdt, offset, "ibm,loc-code", drc_name,
>> + strlen(drc_name)));
>> + }
>> + 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));
>> @@ -964,30 +971,38 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev,
>> void *fdt, int offset,
>> return 0;
>> }
>>
>> +typedef struct sPAPRFDT {
>> + void *fdt;
>> + int node_off;
>> + sPAPRPHBState *sphb;
>> +} sPAPRFDT;
>
> I don't really like this structure - it seems a very ad-hoc collection
> of things. Even though it means there will be a lot of parameters to
> the function, I'd prefer passing them separately to
> spapr_create_pci_child_dt() rather than using this structure.
I added this structure with pci_for_each_device() in mind, which has
following prototype.
void pci_for_each_device(PCIBus *bus, int bus_num,
void (*fn)(PCIBus *bus, PCIDevice *d, void
*opaque),
void *opaque);
So per device we get this structure and populate PCI device tree entry
and scan and populate bridge recursively if needed. So I had continued
using this structure in spapr_create_pci_child_dt().
We cannot remove sPAPRFDT completely as we need it for PCI device tree
creation.
So if needed, I can change spapr_create_pci_child_dt() with more args.
And structure sPAPRFDT to be used by spapr_populate_pci_devices_dt()
called by pci_for_each_device().
>> @@ -997,24 +1012,26 @@ static void spapr_phb_add_pci_device(sPAPRDRConnector
>> *drc,
>> {
>> sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>> DeviceState *dev = DEVICE(pdev);
>> - int drc_index = drck->get_index(drc);
>> const char *drc_name = drck->get_name(drc);
>> - void *fdt = NULL;
>> - int fdt_start_offset = 0;
>> + int fdt_start_offset = 0, fdt_size;
>> + sPAPRFDT s_fdt = {NULL, 0, NULL};
>>
>> - /* 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);
>> + s_fdt.fdt = create_device_tree(&fdt_size);
>> + s_fdt.sphb = phb;
>> + s_fdt.node_off = 0;
>> + fdt_start_offset = spapr_create_pci_child_dt(pdev, &s_fdt,
>> drc_name);
>> + if (!fdt_start_offset) {
>> + error_setg(errp, "Failed to create pci child device tree node");
>> + goto out;
>> + }
>> }
>>
>> drck->attach(drc, DEVICE(pdev),
>> - fdt, fdt_start_offset, !dev->hotplugged, errp);
>> + s_fdt.fdt, fdt_start_offset, !dev->hotplugged, errp);
>> +out:
>> if (*errp) {
>> - g_free(fdt);
>> + g_free(s_fdt.fdt);
>> }
>> }
[SNIP]
>> +static void spapr_populate_pci_devices_dt(PCIBus *bus, PCIDevice *pdev,
>> + void *opaque)
>> +{
>> + PCIBus *sec_bus;
>> + sPAPRFDT *p = opaque;
[ SNIP ]
>> + sec_bus = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
>> + if (!sec_bus) {
>> + return;
>> + }
>> +
>> + s_fdt.fdt = p->fdt;
>> + s_fdt.node_off = offset;
>> + s_fdt.sphb = p->sphb;
>> + pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
>> + spapr_populate_pci_devices_dt,
>> + &s_fdt);
>> +}
[ SNIP ]
>> @@ -1523,6 +1626,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>> 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];
>> sPAPRTCETable *tcet;
>> + PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
>> + sPAPRFDT s_fdt;
>>
>> /* Start populating the FDT */
>> sprintf(nodename, "address@hidden" PRIx64, phb->buid);
>> @@ -1572,6 +1677,18 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>> tcet->liobn, tcet->bus_offset,
>> tcet->nb_table << tcet->page_shift);
>>
>> + /* Walk the bridges and program the bus numbers*/
>> + spapr_phb_pci_enumerate(phb);
>> + _FDT(fdt_setprop_cell(fdt, bus_off, "qemu,phb-enumerated", 0x1));
>> +
>> + /* Populate tree nodes with PCI devices attached */
>> + s_fdt.fdt = fdt;
>> + s_fdt.node_off = bus_off;
>> + s_fdt.sphb = phb;
>> + pci_for_each_device(bus, pci_bus_num(bus),
>> + spapr_populate_pci_devices_dt,
>> + &s_fdt);
>> +
>> ret = spapr_drc_populate_dt(fdt, bus_off, OBJECT(phb),
>> SPAPR_DR_CONNECTOR_TYPE_PCI);
>> if (ret) {
>
Regards,
Nikunj
- [Qemu-devel] [PATCH v7 0/6] spapr_pci: DT field fixes and PCI DT node creation in QEMU, Nikunj A Dadhania, 2015/06/11
- [Qemu-devel] [PATCH v7 2/6] spapr_pci: encode class code including Prog IF register, Nikunj A Dadhania, 2015/06/11
- [Qemu-devel] [PATCH v7 6/6] spapr_pci: drop redundant args in spapr_populate_pci_child_dt, Nikunj A Dadhania, 2015/06/11
- [Qemu-devel] [PATCH v7 5/6] spapr_pci: populate ibm,loc-code, Nikunj A Dadhania, 2015/06/11
- [Qemu-devel] [PATCH v7 1/6] spapr_pci: encode missing 64-bit memory address space, Nikunj A Dadhania, 2015/06/11
- [Qemu-devel] [PATCH v7 3/6] spapr_pci: enumerate and add PCI device tree, Nikunj A Dadhania, 2015/06/11
- [Qemu-devel] [PATCH v7 4/6] spapr_pci: set device node unit address as hex, Nikunj A Dadhania, 2015/06/11
- Re: [Qemu-devel] [PATCH v7 0/6] spapr_pci: DT field fixes and PCI DT node creation in QEMU, David Gibson, 2015/06/18