Quoting Alexey Kardashevskiy (2014-08-26 03:32:58)
> On 08/19/2014 10:21 AM, Michael Roth wrote:
> > Reserve 32 entries of type PCI in each PHB's initial FDT. This
> > advertises to guests that each PHB is DR-capable device with
> > physical hotpluggable slots. This is necessary for allowing
> > hotplugging of devices to it later via bus rescan or guest rpaphp
> > hotplug module.
> >
> > Each entry is assigned a name of "Slot <<bus_idx>*32 +1>",
> > advertised as a hotpluggable PCI slot, and assigned to power domain
> > -1 to indicate to the guest that power management is handled by the
> > hardware.
> >
> > This models a DR-capable PCI expansion device attached to a host/lpar
> > via a single PHB with 32 physical hotpluggable slots (as opposed to a
> > virtual bridge device with external management console). Hotplug will
> > be handled by the guest via bus rescan or the rpaphp hotplug module.
> >
> > Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com>
> > ---
> > hw/ppc/spapr.c | 3 +-
> > hw/ppc/spapr_pci.c | 102
> > ++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/pci-host/spapr.h | 1 +
> > 3 files changed, 105 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index d5e46c3..90b25b3 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -890,7 +890,8 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
> > QLIST_FOREACH(phb, &spapr->phbs, list) {
> > drc_entry = spapr_phb_to_drc_entry(phb->buid);
> > g_assert(drc_entry);
> > - ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt);
> > + ret = spapr_populate_pci_dt(phb, PHANDLE_XICP,
> > drc_entry->drc_index,
> > + fdt);
> > }
> >
> > if (ret < 0) {
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index e85134f..924d488 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -851,8 +851,104 @@ static int spapr_phb_children_dt(Object *child, void
> > *opaque)
> > return 1;
> > }
> >
> > +static void spapr_create_drc_phb_dt_entries(void *fdt, int bus_off, int
> > phb_index)
> > +{
> > + char char_buf[1024];
> > + uint32_t int_buf[SPAPR_DRC_PHB_SLOT_MAX + 1];
> > + uint32_t *entries;
> > + int i, ret, offset;
> > +
> > + /* ibm,drc-indexes */
> > + memset(int_buf, 0 , sizeof(int_buf));
> > + int_buf[0] = SPAPR_DRC_PHB_SLOT_MAX;
> > +
> > + for (i = 1; i <= SPAPR_DRC_PHB_SLOT_MAX; i++) {
> > + int_buf[i] = SPAPR_DRC_DEV_ID_BASE + (phb_index << 8) + ((i - 1)
> > << 3);
> > + }
> > +
> > + ret = fdt_setprop(fdt, bus_off, "ibm,drc-indexes", int_buf,
> > + sizeof(int_buf));
> > + if (ret) {
> > + fprintf(stderr, "error adding 'ibm,drc-indexes' field for PHB
> > FDT");
> > + }
> > +
> > + /* ibm,drc-power-domains */
> > + memset(int_buf, 0, sizeof(int_buf));
> > + int_buf[0] = SPAPR_DRC_PHB_SLOT_MAX;
> > +
> > + for (i = 1; i <= SPAPR_DRC_PHB_SLOT_MAX; i++) {
> > + int_buf[i] = 0xffffffff;
> > + }
> > +
> > + ret = fdt_setprop(fdt, bus_off, "ibm,drc-power-domains", int_buf,
> > + sizeof(int_buf));
> > + if (ret) {
> > + fprintf(stderr,
> > + "error adding 'ibm,drc-power-domains' field for PHB FDT");
>
> As before - return here and below.
>
> > + }
> > +
> > + /* ibm,drc-names */
> > + memset(char_buf, 0, sizeof(char_buf));
> > + entries = (uint32_t *)&char_buf[0];
> > + *entries = SPAPR_DRC_PHB_SLOT_MAX;
> > + offset = sizeof(*entries);
> > +
> > + for (i = 1; i <= SPAPR_DRC_PHB_SLOT_MAX; i++) {
> > + offset += sprintf(char_buf + offset, "Slot %d",
> > + (phb_index * SPAPR_DRC_PHB_SLOT_MAX) + i - 1);
>
> Mmmm. From 1 to <=MAX and (i-1) inside the loop when it could be
> traditional 0 to <MAX and (i) as it is done below :)
>
>
> > + char_buf[offset++] = '\0';
>
>
> sprintf() puts zero there itself, no? And as we are here, should not it be
> snprintf()?
You mean something like this?
snprintf(char_buf + offset, sizeof(char_buf) - offset, "Slot %d", ...)
I'm not sure what happens if sizeof(char_buf) - offset goes negative though, so
maybe:
for (i = 0; i < SPAPR_DRC_PHB_SLOT_MAX && offset < sizeof(char_buf); i++) {
if (offset >= sizeof(char_buf)) {
fprintf(stderr,
"error generating 'ibm,drc-names' field for PHB FDT");
return;
}
offset += snprintf(char_buf + offset, sizeof(char_buf) - offset,
"Slot %d", (phb_index * SPAPR_DRC_PHB_SLOT_MAX) + i);
offset++;
}
>
>
> > + }
>
>
> > + }
> > +
> > + ret = fdt_setprop(fdt, bus_off, "ibm,drc-names", char_buf, offset);
> > + if (ret) {
> > + fprintf(stderr, "error adding 'ibm,drc-names' field for PHB FDT");
> > + }
> > +
> > + /* ibm,drc-types */
> > + memset(char_buf, 0, sizeof(char_buf));
> > + entries = (uint32_t *)&char_buf[0];
> > + *entries = SPAPR_DRC_PHB_SLOT_MAX;
> > + offset = sizeof(*entries);
> > +
> > + for (i = 0; i < SPAPR_DRC_PHB_SLOT_MAX; i++) {
> > + offset += sprintf(char_buf + offset, "28");
> > + char_buf[offset++] = '\0';
> > + }
> > +
> > + ret = fdt_setprop(fdt, bus_off, "ibm,drc-types", char_buf, offset);
> > + if (ret) {
> > + fprintf(stderr, "error adding 'ibm,drc-types' field for PHB FDT");
> > + }
> > +
> > + /* we want the initial indicator state to be 0 - "empty", when we
> > + * hot-plug an adaptor in the slot, we need to set the indicator
> > + * to 1 - "present."
> > + */
> > +
> > + /* ibm,indicator-9003 */
> > + memset(int_buf, 0, sizeof(int_buf));
> > + int_buf[0] = SPAPR_DRC_PHB_SLOT_MAX;
> > +
> > + ret = fdt_setprop(fdt, bus_off, "ibm,indicator-9003", int_buf,
> > + sizeof(int_buf));
> > + if (ret) {
> > + fprintf(stderr, "error adding 'ibm,indicator-9003' field for PHB
> > FDT");
> > + }
> > +
> > + /* ibm,sensor-9003 */
> > + memset(int_buf, 0, sizeof(int_buf));
> > + int_buf[0] = SPAPR_DRC_PHB_SLOT_MAX;
> > +
> > + ret = fdt_setprop(fdt, bus_off, "ibm,sensor-9003", int_buf,
> > + sizeof(int_buf));
> > + if (ret) {
> > + fprintf(stderr, "error adding 'ibm,sensor-9003' field for PHB
> > FDT");
> > + }
> > +}
> > +
> > int spapr_populate_pci_dt(sPAPRPHBState *phb,
> > uint32_t xics_phandle,
> > + uint32_t drc_index,
> > void *fdt)
> > {
> > int bus_off, i, j;
> > @@ -934,6 +1030,12 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
> > object_child_foreach(OBJECT(phb), spapr_phb_children_dt,
> > &((sPAPRTCEDT){ .fdt = fdt, .node_off = bus_off
> > }));
> >
> > + spapr_create_drc_phb_dt_entries(fdt, bus_off, phb->index);
> > + if (drc_index) {
> > + _FDT(fdt_setprop(fdt, bus_off, "ibm,my-drc-index", &drc_index,
> > + sizeof(drc_index)));
> > + }
> > +
> > return 0;
> > }
> >
> > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> > index 32f0aa7..8f0a42f 100644
> > --- a/include/hw/pci-host/spapr.h
> > +++ b/include/hw/pci-host/spapr.h
> > @@ -116,6 +116,7 @@ PCIHostState *spapr_create_phb(sPAPREnvironment *spapr,
> > int index);
> >
> > int spapr_populate_pci_dt(sPAPRPHBState *phb,
> > uint32_t xics_phandle,
> > + uint32_t drc_index,
> > void *fdt);
> >
> > void spapr_pci_msi_init(sPAPREnvironment *spapr, hwaddr addr);
> >
>
>
> --
> Alexey
