From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42594) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XMIsd-0003on-SI for qemu-devel@nongnu.org; Tue, 26 Aug 2014 11:42:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XMIsL-0007LA-71 for qemu-devel@nongnu.org; Tue, 26 Aug 2014 11:41:55 -0400 Received: from e35.co.us.ibm.com ([32.97.110.153]:33442) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XMIsK-0007L0-V3 for qemu-devel@nongnu.org; Tue, 26 Aug 2014 11:41:37 -0400 Received: from /spool/local by e35.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 26 Aug 2014 09:41:36 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <20140826152513.21832.23270@loki> References: <1408407718-10835-1-git-send-email-mdroth@linux.vnet.ibm.com> <1408407718-10835-2-git-send-email-mdroth@linux.vnet.ibm.com> <53FC3D59.1020702@ozlabs.ru> <53FC4428.5000904@ozlabs.ru> <20140826152513.21832.23270@loki> Message-ID: <20140826154128.21832.37295@loki> Date: Tue, 26 Aug 2014 10:41:28 -0500 Subject: Re: [Qemu-devel] [PATCH 01/12] spapr: populate DRC entries for root dt node 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 Michael Roth (2014-08-26 10:25:13) > Quoting Alexey Kardashevskiy (2014-08-26 03:24:08) > > On 08/26/2014 05:55 PM, Alexey Kardashevskiy wrote: > > > On 08/19/2014 10:21 AM, Michael Roth wrote: > > >> From: Nathan Fontenot > > >> > > >> This add entries to the root OF node to advertise our PHBs as being > > >> DR-capable in according with PAPR specification. > > >> > > >> Each PHB is given a name of PHB, advertised as a PHB type, > > >> and associated with a power domain of -1 (indicating to guests that > > >> power management is handled automatically by hardware). > > >> > > >> We currently allocate entries for up to 32 DR-capable PHBs, though > > >> this limit can be increased later. > > >> > > >> DrcEntry objects to track the state of the DR-connector associated > > >> with each PHB are stored in a 32-entry array, and each DrcEntry has > > >> in turn have a dynamically-sized number of child DR-connectors, > > >> which we will use later to track the state of DR-connectors > > >> associated with a PHB's physical slots. > > >> > > >> Signed-off-by: Nathan Fontenot > > >> Signed-off-by: Michael Roth > > >> --- > > >> hw/ppc/spapr.c | 143 ++++++++++++++++++++++++++++++++++++++= +++++++++++ > > >> hw/ppc/spapr_pci.c | 1 + > > >> include/hw/ppc/spapr.h | 35 ++++++++++++ > > >> 3 files changed, 179 insertions(+) > > >> > > >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > >> index 5c92707..d5e46c3 100644 > > >> --- a/hw/ppc/spapr.c > > >> +++ b/hw/ppc/spapr.c > > >> @@ -296,6 +296,143 @@ static hwaddr spapr_node0_size(void) > > >> return ram_size; > > >> } > > >> = > > >> +sPAPRDrcEntry *spapr_phb_to_drc_entry(uint64_t buid) > > >> +{ > > >> + int i; > > >> + > > >> + for (i =3D 0; i < SPAPR_DRC_TABLE_SIZE; i++) { > > >> + if (spapr->drc_table[i].phb_buid =3D=3D buid) { > > >> + return &spapr->drc_table[i]; > > >> + } > > >> + } > > >> + > > >> + return NULL; > > >> +} > > >> + > > >> +static void spapr_init_drc_table(void) > > >> +{ > > >> + int i; > > >> + > > >> + memset(spapr->drc_table, 0, sizeof(spapr->drc_table)); > > >> + > > >> + /* For now we only care about PHB entries */ > > >> + for (i =3D 0; i < SPAPR_DRC_TABLE_SIZE; i++) { > > >> + spapr->drc_table[i].drc_index =3D 0x2000001 + i; > > >> + } > > >> +} > > >> + > > >> +sPAPRDrcEntry *spapr_add_phb_to_drc_table(uint64_t buid, uint32_t s= tate) > > >> +{ > > >> + sPAPRDrcEntry *empty_drc =3D NULL; > > >> + sPAPRDrcEntry *found_drc =3D NULL; > > >> + int i, phb_index; > > >> + > > >> + for (i =3D 0; i < SPAPR_DRC_TABLE_SIZE; i++) { > > >> + if (spapr->drc_table[i].phb_buid =3D=3D 0) { > > >> + empty_drc =3D &spapr->drc_table[i]; > > >> + } > > >> + > > >> + if (spapr->drc_table[i].phb_buid =3D=3D buid) { > > >> + found_drc =3D &spapr->drc_table[i]; > > > = > > > return &spapr->drc_table[i]; > > > ? > > > = > > > = > > >> + break; > > >> + } > > >> + } > > >> + > > >> + if (found_drc) { > > >> + return found_drc; > > >> + } > > > = > > > if (!empty_drc) { > > > return NULL; > > > } > > > = > > > ? > > > = > > > = > > >> + > > >> + if (empty_drc) { > > > = > > > and no need in this :) > > > = > > > = > > >> + empty_drc->phb_buid =3D buid; > > >> + empty_drc->state =3D state; > > >> + empty_drc->cc_state.fdt =3D NULL; > > >> + empty_drc->cc_state.offset =3D 0; > > >> + empty_drc->cc_state.depth =3D 0; > > >> + empty_drc->cc_state.state =3D CC_STATE_IDLE; > > >> + empty_drc->child_entries =3D > > >> + g_malloc0(sizeof(sPAPRDrcEntry) * SPAPR_DRC_PHB_SLOT_MA= X); > > >> + phb_index =3D buid - SPAPR_PCI_BASE_BUID; > > >> + for (i =3D 0; i < SPAPR_DRC_PHB_SLOT_MAX; i++) { > > >> + empty_drc->child_entries[i].drc_index =3D > > >> + SPAPR_DRC_DEV_ID_BASE + (phb_index << 8) + (i << 3); > > >> + } > > >> + return empty_drc; > > >> + } > > >> + > > >> + return NULL; > > >> +} > > >> + > > >> +static void spapr_create_drc_dt_entries(void *fdt) > > >> +{ > > >> + char char_buf[1024]; > > >> + uint32_t int_buf[SPAPR_DRC_TABLE_SIZE + 1]; > > >> + uint32_t *entries; > > >> + int offset, fdt_offset; > > >> + int i, ret; > > >> + > > >> + fdt_offset =3D fdt_path_offset(fdt, "/"); > > >> + > > >> + /* ibm,drc-indexes */ > > >> + memset(int_buf, 0, sizeof(int_buf)); > > >> + int_buf[0] =3D SPAPR_DRC_TABLE_SIZE; > > >> + > > >> + for (i =3D 1; i <=3D SPAPR_DRC_TABLE_SIZE; i++) { > > >> + int_buf[i] =3D spapr->drc_table[i-1].drc_index; > > >> + } > > >> + > > >> + ret =3D fdt_setprop(fdt, fdt_offset, "ibm,drc-indexes", int_buf, > > >> + sizeof(int_buf)); > > >> + if (ret) { > > >> + fprintf(stderr, "Couldn't finalize ibm,drc-indexes property= \n"); > > > = > > > return here and below in the same error cases? > > > = > > >> + } > > >> + > > >> + /* ibm,drc-power-domains */ > > >> + memset(int_buf, 0, sizeof(int_buf)); > > >> + int_buf[0] =3D SPAPR_DRC_TABLE_SIZE; > > >> + > > >> + for (i =3D 1; i <=3D SPAPR_DRC_TABLE_SIZE; i++) { > > >> + int_buf[i] =3D 0xffffffff; > > >> + } > > >> + > > >> + ret =3D fdt_setprop(fdt, fdt_offset, "ibm,drc-power-domains", i= nt_buf, > > >> + sizeof(int_buf)); > > >> + if (ret) { > > >> + fprintf(stderr, "Couldn't finalize ibm,drc-power-domains pr= operty\n"); > > >> + } > > >> + > > >> + /* ibm,drc-names */ > > >> + memset(char_buf, 0, sizeof(char_buf)); > > >> + entries =3D (uint32_t *)&char_buf[0]; > > >> + *entries =3D SPAPR_DRC_TABLE_SIZE; > > >> + offset =3D sizeof(*entries); > > >> + > > >> + for (i =3D 0; i < SPAPR_DRC_TABLE_SIZE; i++) { > > >> + offset +=3D sprintf(char_buf + offset, "PHB %d", i + 1); > > >> + char_buf[offset++] =3D '\0'; > > >> + } > > >> + > > >> + ret =3D fdt_setprop(fdt, fdt_offset, "ibm,drc-names", char_buf,= offset); > > >> + if (ret) { > > >> + fprintf(stderr, "Couldn't finalize ibm,drc-names property\n= "); > > >> + } > > >> + > > >> + /* ibm,drc-types */ > > >> + memset(char_buf, 0, sizeof(char_buf)); > > >> + entries =3D (uint32_t *)&char_buf[0]; > > >> + *entries =3D SPAPR_DRC_TABLE_SIZE; > > >> + offset =3D sizeof(*entries); > > >> + > > >> + for (i =3D 0; i < SPAPR_DRC_TABLE_SIZE; i++) { > > >> + offset +=3D sprintf(char_buf + offset, "PHB"); > > >> + char_buf[offset++] =3D '\0'; > > >> + } > > >> + > > >> + ret =3D fdt_setprop(fdt, fdt_offset, "ibm,drc-types", char_buf,= offset); > > >> + if (ret) { > > >> + fprintf(stderr, "Couldn't finalize ibm,drc-types property\n= "); > > >> + } > > >> +} > > >> + > > >> #define _FDT(exp) \ > > >> do { \ > > >> int ret =3D (exp); = \ > > >> @@ -731,6 +868,7 @@ static void spapr_finalize_fdt(sPAPREnvironment = *spapr, > > >> char *bootlist; > > >> void *fdt; > > >> sPAPRPHBState *phb; > > >> + sPAPRDrcEntry *drc_entry; > > >> = > > >> fdt =3D g_malloc(FDT_MAX_SIZE); > > >> = > > >> @@ -750,6 +888,8 @@ static void spapr_finalize_fdt(sPAPREnvironment = *spapr, > > >> } > > >> = > > >> 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); > > >> } > > >> = > > >> @@ -789,6 +929,8 @@ static void spapr_finalize_fdt(sPAPREnvironment = *spapr, > > >> spapr_populate_chosen_stdout(fdt, spapr->vio_bus); > > >> } > > >> = > > >> + spapr_create_drc_dt_entries(fdt); > > >> + > > >> _FDT((fdt_pack(fdt))); > > >> = > > >> if (fdt_totalsize(fdt) > FDT_MAX_SIZE) { > > >> @@ -1443,6 +1585,7 @@ static void ppc_spapr_init(MachineState *machi= ne) > > >> spapr_pci_msi_init(spapr, SPAPR_PCI_MSI_WINDOW); > > >> spapr_pci_rtas_init(); > > >> = > > >> + spapr_init_drc_table(); > > >> phb =3D spapr_create_phb(spapr, 0); > > >> = > > >> for (i =3D 0; i < nb_nics; i++) { > > >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > > >> index 9ed39a9..e85134f 100644 > > >> --- a/hw/ppc/spapr_pci.c > > >> +++ b/hw/ppc/spapr_pci.c > > >> @@ -531,6 +531,7 @@ static void spapr_phb_realize(DeviceState *dev, = Error **errp) > > >> + sphb->index * SPAPR_PCI_WINDOW_SPACING; > > >> sphb->mem_win_addr =3D windows_base + SPAPR_PCI_MMIO_WIN_OF= F; > > >> sphb->io_win_addr =3D windows_base + SPAPR_PCI_IO_WIN_OFF; > > >> + spapr_add_phb_to_drc_table(sphb->buid, 2 /* Unusable */); > > > = > > > = > > > What exactly does "unusable" mean here? Macro? > > > = > > > = > > > = > > >> } > > >> = > > >> if (sphb->buid =3D=3D -1) { > > >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > > >> index 36e8e51..c93794b 100644 > > >> --- a/include/hw/ppc/spapr.h > > >> +++ b/include/hw/ppc/spapr.h > > >> @@ -10,6 +10,36 @@ struct sPAPRNVRAM; > > >> = > > >> #define HPTE64_V_HPTE_DIRTY 0x0000000000000040ULL > > >> = > > >> +/* For dlparable/hotpluggable slots */ > > >> +#define SPAPR_DRC_TABLE_SIZE 32 > > >> +#define SPAPR_DRC_PHB_SLOT_MAX 32 > > >> +#define SPAPR_DRC_DEV_ID_BASE 0x40000000 > > > = > > > = > > > Is this SPAPR_DRC_DEV_ID_BASE really necessary (can it be zero)? Is t= hat > > > global id or per PCI bus or per PHB? > > = > > = > > Ah. Got it. If it was like below, I would not even ask :) > > = > > #define SPAPR_DRC_DEV_ID(phb_index, slot) \ > > (0x40000000 | ((phb_index)<<8) | ((slot)<<3)) > > = > > Still not clear why you need this 0x40000000 for. Is it kind of "PHB" D= RC type? > = > Yes, it's somewhat ad-hoc...the only requirement I see in PAPR is that th= is value be > globally unique across all DR resources. CPUs and memory and such might h= ave different > ways to compute their DRC indices (so a slot-based macro would need to be= specific > to PCI DR entries). I'm not sure where the 0x40000000 originated honestly= . I'm not > sure it matters for QEMU, since we hold a monopoly on all DRC index assig= nments and > don't have to deal with hard-coded firmware values. > = > I will say that a base somewhat less common than 0 may prove useful from = a debugging > standpoint, all other things being equal. > = > So not sure what best to do here. If we choose to leave it as is, I could= at least > make sure to add a comment about this. Hmm, this all also applies to the 0x2000001 drc base used for the parent PH= B DRC indices that Alex mentioned. If we want to do something a little more structured, we could take the hotp= lug types introduced later: #define RTAS_LOG_V6_HP_TYPE_CPU 1 #define RTAS_LOG_V6_HP_TYPE_MEMORY 2 #define RTAS_LOG_V6_HP_TYPE_SLOT 3 #define RTAS_LOG_V6_HP_TYPE_PHB 4 #define RTAS_LOG_V6_HP_TYPE_PCI 5 As the DRC index base. A macro representation would basically be: #define SPAPR_DRC_DEV_ID_BASE(dr_type) (dr_type << 28) I don't really like that 'dr_type' doesn't actually correspond to the ibm,drc-types ofdt property though, which are string values, unfortunately. ("PHB" for PHB, and "28" for PCI slot...). We could do a string->dr_type lo= okup in the macro, but passing around "28" doesn't seem very readable either... > = > > = > > = > > > = > > >> + > > >> +typedef struct sPAPRConfigureConnectorState { > > >> + void *fdt; > > >> + int offset_start; > > >> + int offset; > > >> + int depth; > > >> + PCIDevice *dev; > > >> + enum { > > >> + CC_STATE_IDLE =3D 0, > > >> + CC_STATE_PENDING =3D 1, > > >> + CC_STATE_ACTIVE, > > >> + } state; > > >> +} sPAPRConfigureConnectorState; > > >> + > > >> +typedef struct sPAPRDrcEntry sPAPRDrcEntry; > > >> + > > >> +struct sPAPRDrcEntry { > > >> + uint32_t drc_index; > > >> + uint64_t phb_buid; > > >> + void *fdt; > > >> + int fdt_offset; > > >> + uint32_t state; > > >> + sPAPRConfigureConnectorState cc_state; > > >> + sPAPRDrcEntry *child_entries; > > >> +}; > > >> + > > >> typedef struct sPAPREnvironment { > > >> struct VIOsPAPRBus *vio_bus; > > >> QLIST_HEAD(, sPAPRPHBState) phbs; > > >> @@ -39,6 +69,9 @@ typedef struct sPAPREnvironment { > > >> int htab_save_index; > > >> bool htab_first_pass; > > >> int htab_fd; > > >> + > > >> + /* state for Dynamic Reconfiguration Connectors */ > > >> + sPAPRDrcEntry drc_table[SPAPR_DRC_TABLE_SIZE]; > > >> } sPAPREnvironment; > > >> = > > >> #define H_SUCCESS 0 > > >> @@ -481,5 +514,7 @@ int spapr_dma_dt(void *fdt, int node_off, const = char *propname, > > >> uint32_t liobn, uint64_t window, uint32_t size); > > >> int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname, > > >> sPAPRTCETable *tcet); > > >> +sPAPRDrcEntry *spapr_add_phb_to_drc_table(uint64_t buid, uint32_t s= tate); > > >> +sPAPRDrcEntry *spapr_phb_to_drc_entry(uint64_t buid); > > >> = > > >> #endif /* !defined (__HW_SPAPR_H__) */ > > >> > > > = > > > = > > = > > = > > -- = > > Alexey