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 <nf...@linux.vnet.ibm.com> > >> > >> 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<bus#>, 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 <nf...@linux.vnet.ibm.com> > >> Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com> > >> --- > >> 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?
Yes, it's somewhat ad-hoc...the only requirement I see in PAPR is that this value be globally unique across all DR resources. CPUs and memory and such might have 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 assignments 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. > > > > > >> + > >> +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