From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32939) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XMKMj-0005zA-4Z for qemu-devel@nongnu.org; Tue, 26 Aug 2014 13:17:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XMKMZ-0005hO-U0 for qemu-devel@nongnu.org; Tue, 26 Aug 2014 13:17:04 -0400 Received: from e39.co.us.ibm.com ([32.97.110.160]:54729) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XMKMZ-0005hA-Kz for qemu-devel@nongnu.org; Tue, 26 Aug 2014 13:16:55 -0400 Received: from /spool/local by e39.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 26 Aug 2014 11:16:54 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <53FC463A.8000408@ozlabs.ru> References: <1408407718-10835-1-git-send-email-mdroth@linux.vnet.ibm.com> <1408407718-10835-3-git-send-email-mdroth@linux.vnet.ibm.com> <53FC463A.8000408@ozlabs.ru> Message-ID: <20140826171641.21832.11939@loki> Date: Tue, 26 Aug 2014 12:16:41 -0500 Subject: Re: [Qemu-devel] [PATCH 02/12] spapr_pci: populate DRC dt entries for PHBs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , qemu-devel@nongnu.org Cc: ncmike@ncultra.org, nfont@linux.vnet.ibm.com, qemu-ppc@nongnu.org, agraf@suse.de, tyreld@linux.vnet.ibm.com 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 <*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 > > --- > > 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 *sp= apr, > > QLIST_FOREACH(phb, &spapr->phbs, list) { > > drc_entry =3D spapr_phb_to_drc_entry(phb->buid); > > g_assert(drc_entry); > > - ret =3D spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt); > > + ret =3D spapr_populate_pci_dt(phb, PHANDLE_XICP, drc_entry->dr= c_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, v= oid *opaque) > > return 1; > > } > > = > > +static void spapr_create_drc_phb_dt_entries(void *fdt, int bus_off, in= t 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] =3D SPAPR_DRC_PHB_SLOT_MAX; > > + > > + for (i =3D 1; i <=3D SPAPR_DRC_PHB_SLOT_MAX; i++) { > > + int_buf[i] =3D SPAPR_DRC_DEV_ID_BASE + (phb_index << 8) + ((i = - 1) << 3); > > + } > > + > > + ret =3D 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] =3D SPAPR_DRC_PHB_SLOT_MAX; > > + > > + for (i =3D 1; i <=3D SPAPR_DRC_PHB_SLOT_MAX; i++) { > > + int_buf[i] =3D 0xffffffff; > > + } > > + > > + ret =3D 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 FD= T"); > = > As before - return here and below. > = > > + } > > + > > + /* ibm,drc-names */ > > + memset(char_buf, 0, sizeof(char_buf)); > > + entries =3D (uint32_t *)&char_buf[0]; > > + *entries =3D SPAPR_DRC_PHB_SLOT_MAX; > > + offset =3D sizeof(*entries); > > + > > + for (i =3D 1; i <=3D SPAPR_DRC_PHB_SLOT_MAX; i++) { > > + offset +=3D sprintf(char_buf + offset, "Slot %d", > > + (phb_index * SPAPR_DRC_PHB_SLOT_MAX) + i - 1= ); > = > Mmmm. From 1 to <=3DMAX and (i-1) inside the loop when it could be > traditional 0 to = > = > > + char_buf[offset++] =3D '\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 =3D 0; i < SPAPR_DRC_PHB_SLOT_MAX && offset < sizeof(char_buf); = i++) { if (offset >=3D sizeof(char_buf)) { fprintf(stderr, "error generating 'ibm,drc-names' field for PHB FDT"); return; } offset +=3D snprintf(char_buf + offset, sizeof(char_buf) - offset, "Slot %d", (phb_index * SPAPR_DRC_PHB_SLOT_MAX) = + i); offset++; } > = > = > > + } > = > = > > + } > > + > > + ret =3D fdt_setprop(fdt, bus_off, "ibm,drc-names", char_buf, offse= t); > > + if (ret) { > > + fprintf(stderr, "error adding 'ibm,drc-names' field for PHB FD= T"); > > + } > > + > > + /* ibm,drc-types */ > > + memset(char_buf, 0, sizeof(char_buf)); > > + entries =3D (uint32_t *)&char_buf[0]; > > + *entries =3D SPAPR_DRC_PHB_SLOT_MAX; > > + offset =3D sizeof(*entries); > > + > > + for (i =3D 0; i < SPAPR_DRC_PHB_SLOT_MAX; i++) { > > + offset +=3D sprintf(char_buf + offset, "28"); > > + char_buf[offset++] =3D '\0'; > > + } > > + > > + ret =3D fdt_setprop(fdt, bus_off, "ibm,drc-types", char_buf, offse= t); > > + if (ret) { > > + fprintf(stderr, "error adding 'ibm,drc-types' field for PHB FD= T"); > > + } > > + > > + /* 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] =3D SPAPR_DRC_PHB_SLOT_MAX; > > + > > + ret =3D fdt_setprop(fdt, bus_off, "ibm,indicator-9003", int_buf, > > + sizeof(int_buf)); > > + if (ret) { > > + fprintf(stderr, "error adding 'ibm,indicator-9003' field for P= HB FDT"); > > + } > > + > > + /* ibm,sensor-9003 */ > > + memset(int_buf, 0, sizeof(int_buf)); > > + int_buf[0] =3D SPAPR_DRC_PHB_SLOT_MAX; > > + > > + ret =3D 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 =3D fdt, .node_off =3D b= us_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 *sp= apr, 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