From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43888) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XMC3C-0006Di-Qm for qemu-devel@nongnu.org; Tue, 26 Aug 2014 04:24:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XMC35-0000Qw-5L for qemu-devel@nongnu.org; Tue, 26 Aug 2014 04:24:22 -0400 Received: from mail-pa0-f48.google.com ([209.85.220.48]:52550) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XMC34-0000Qm-Sm for qemu-devel@nongnu.org; Tue, 26 Aug 2014 04:24:15 -0400 Received: by mail-pa0-f48.google.com with SMTP id et14so22700405pad.7 for ; Tue, 26 Aug 2014 01:24:14 -0700 (PDT) Message-ID: <53FC4428.5000904@ozlabs.ru> Date: Tue, 26 Aug 2014 18:24:08 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 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> In-Reply-To: <53FC3D59.1020702@ozlabs.ru> Content-Type: text/plain; charset=koi8-r Content-Transfer-Encoding: 7bit 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: Michael Roth , 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 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 = 0; i < SPAPR_DRC_TABLE_SIZE; i++) { >> + if (spapr->drc_table[i].phb_buid == 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 = 0; i < SPAPR_DRC_TABLE_SIZE; i++) { >> + spapr->drc_table[i].drc_index = 0x2000001 + i; >> + } >> +} >> + >> +sPAPRDrcEntry *spapr_add_phb_to_drc_table(uint64_t buid, uint32_t state) >> +{ >> + sPAPRDrcEntry *empty_drc = NULL; >> + sPAPRDrcEntry *found_drc = NULL; >> + int i, phb_index; >> + >> + for (i = 0; i < SPAPR_DRC_TABLE_SIZE; i++) { >> + if (spapr->drc_table[i].phb_buid == 0) { >> + empty_drc = &spapr->drc_table[i]; >> + } >> + >> + if (spapr->drc_table[i].phb_buid == buid) { >> + found_drc = &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 = buid; >> + empty_drc->state = state; >> + empty_drc->cc_state.fdt = NULL; >> + empty_drc->cc_state.offset = 0; >> + empty_drc->cc_state.depth = 0; >> + empty_drc->cc_state.state = CC_STATE_IDLE; >> + empty_drc->child_entries = >> + g_malloc0(sizeof(sPAPRDrcEntry) * SPAPR_DRC_PHB_SLOT_MAX); >> + phb_index = buid - SPAPR_PCI_BASE_BUID; >> + for (i = 0; i < SPAPR_DRC_PHB_SLOT_MAX; i++) { >> + empty_drc->child_entries[i].drc_index = >> + 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 = fdt_path_offset(fdt, "/"); >> + >> + /* ibm,drc-indexes */ >> + memset(int_buf, 0, sizeof(int_buf)); >> + int_buf[0] = SPAPR_DRC_TABLE_SIZE; >> + >> + for (i = 1; i <= SPAPR_DRC_TABLE_SIZE; i++) { >> + int_buf[i] = spapr->drc_table[i-1].drc_index; >> + } >> + >> + ret = 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] = SPAPR_DRC_TABLE_SIZE; >> + >> + for (i = 1; i <= SPAPR_DRC_TABLE_SIZE; i++) { >> + int_buf[i] = 0xffffffff; >> + } >> + >> + ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-power-domains", int_buf, >> + sizeof(int_buf)); >> + if (ret) { >> + fprintf(stderr, "Couldn't finalize ibm,drc-power-domains property\n"); >> + } >> + >> + /* ibm,drc-names */ >> + memset(char_buf, 0, sizeof(char_buf)); >> + entries = (uint32_t *)&char_buf[0]; >> + *entries = SPAPR_DRC_TABLE_SIZE; >> + offset = sizeof(*entries); >> + >> + for (i = 0; i < SPAPR_DRC_TABLE_SIZE; i++) { >> + offset += sprintf(char_buf + offset, "PHB %d", i + 1); >> + char_buf[offset++] = '\0'; >> + } >> + >> + ret = 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 = (uint32_t *)&char_buf[0]; >> + *entries = SPAPR_DRC_TABLE_SIZE; >> + offset = sizeof(*entries); >> + >> + for (i = 0; i < SPAPR_DRC_TABLE_SIZE; i++) { >> + offset += sprintf(char_buf + offset, "PHB"); >> + char_buf[offset++] = '\0'; >> + } >> + >> + ret = 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 = (exp); \ >> @@ -731,6 +868,7 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr, >> char *bootlist; >> void *fdt; >> sPAPRPHBState *phb; >> + sPAPRDrcEntry *drc_entry; >> >> fdt = g_malloc(FDT_MAX_SIZE); >> >> @@ -750,6 +888,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); >> } >> >> @@ -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 *machine) >> spapr_pci_msi_init(spapr, SPAPR_PCI_MSI_WINDOW); >> spapr_pci_rtas_init(); >> >> + spapr_init_drc_table(); >> phb = spapr_create_phb(spapr, 0); >> >> for (i = 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 = windows_base + SPAPR_PCI_MMIO_WIN_OFF; >> sphb->io_win_addr = 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 == -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 that > 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" DRC type? > >> + >> +typedef struct sPAPRConfigureConnectorState { >> + void *fdt; >> + int offset_start; >> + int offset; >> + int depth; >> + PCIDevice *dev; >> + enum { >> + CC_STATE_IDLE = 0, >> + CC_STATE_PENDING = 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 state); >> +sPAPRDrcEntry *spapr_phb_to_drc_entry(uint64_t buid); >> >> #endif /* !defined (__HW_SPAPR_H__) */ >> > > -- Alexey