On 26.08.14 18:47, Michael Roth wrote:
> Quoting Alexander Graf (2014-08-26 06:11:24)
>> On 19.08.14 02:21, 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;
>>
>> magic number?
>>
>>> + }
>>> +}
>>> +
>>> +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];
>>> + break;
>>> + }
>>> + }
>>> +
>>> + if (found_drc) {
>>> + return found_drc;
>>> + }
>>> +
>>> + if (empty_drc) {
>>> + 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;
>>
>> Not endian safe.
>>
>>> + }
>>> +
>>> + 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");
>>> + }
>>> +
>>> + /* ibm,drc-power-domains */
>>> + memset(int_buf, 0, sizeof(int_buf));
>>> + int_buf[0] = SPAPR_DRC_TABLE_SIZE;
>>
>> Not endian safe.
>>
>>> +
>>> + for (i = 1; i <= SPAPR_DRC_TABLE_SIZE; i++) {
>>> + int_buf[i] = 0xffffffff;
>>> + }
>>
>> memset(-1) instead above?
>>
>>> +
>>> + 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;
>>
>> Not endian safe. I guess you get the idea. I'll stop looking for endian
>> problems here :).
>>
>>> + 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);
>>
>> I would really prefer if we can stick to always use the spapr as
>> function parameter, not use the global.
>>
>>> +
>>> _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 */);
>>> }
>>>
>>> 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
>>
>> Can we make this dynamic so that we can set it to 0 for pseries-2.0 (if
>> necessary) or have an easy tunable to extend the list later?
>
> We could introduce something like -machine pseries,max-dr-connectors=x maybe,
> and set the default based on current machine. Though it's worth noting future
> stuff like cpu/mem DRC entries will get allocated via the same top-level
> ibm,drc-indexes list property (before or after PHB entries), so
> the meaning of that option would change unless we name it something specific
> to PHBs entries, like max-phb-dr-connectors.
I don't think we'd have to expose this to the user at all, so the naming
doesn't have to stay consistent :).
Alex
