[01/12] spapr: populate DRC entries for root dt node

From: Nathan Fontenot <nfont@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 <nfont@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@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(+)

On 08/19/2014 10:21 AM, Michael Roth wrote:
> From: Nathan Fontenot <nfont@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 <nfont@linux.vnet.ibm.com>
> Signed-off-by: Michael Roth <mdroth@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?


> +
> +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__) */
>

On 08/26/2014 05:55 PM, Alexey Kardashevskiy wrote:
> On 08/19/2014 10:21 AM, Michael Roth wrote:
>> From: Nathan Fontenot <nfont@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 <nfont@linux.vnet.ibm.com>
>> Signed-off-by: Michael Roth <mdroth@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?


> 
>> +
>> +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__) */
>>
> 
>

On 19.08.14 02:21, Michael Roth wrote:
> From: Nathan Fontenot <nfont@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 <nfont@linux.vnet.ibm.com>
> Signed-off-by: Michael Roth <mdroth@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?


Alex

> +#define SPAPR_DRC_PHB_SLOT_MAX  32
> +#define SPAPR_DRC_DEV_ID_BASE   0x40000000
> +
> +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__) */
>

Quoting Alexey Kardashevskiy (2014-08-26 02:55:05)
> On 08/19/2014 10:21 AM, Michael Roth wrote:
> > From: Nathan Fontenot <nfont@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 <nfont@linux.vnet.ibm.com>
> > Signed-off-by: Michael Roth <mdroth@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];
> ?

That makes sense. Looking at this again though I think maybe
I should drop the found_drc stuff completely, or maybe even
assert if we attempt to re-add a phb. Current callers already
do spapr_phb_to_drc_entry beforehand anyway, which should
cover the case where it's already there. So maybe something
like this?

sPAPRDrcEntry *spapr_add_phb_to_drc_table(uint64_t buid, uint32_t state)
{
    sPAPRDrcEntry *empty_drc = NULL;
    int i, phb_index;

    for (i = 0; i < SPAPR_DRC_TABLE_SIZE; i++) {
        g_assert(spapr->drc_table[i].phb_buid != buid);
        if (spapr->drc_table[i].phb_buid == 0) {
            empty_drc = &spapr->drc_table[i];
            break;
        }
    }

    if (!empty_drc) {
        return NULL;
    }

    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;
}

> 
> 
> > +            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?

Yah, that's clearly more sensible. I suppose if we introduce an error exit case
this should stop being a void function as well.

> 
> > +    }
> > +
> > +    /* 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?

That's the 9003/"DR-entity-sense" for the DRC entry corresponding to the PHB
itself (as opposed to the sensors for each of its slots). In the case of the
slots, we advertise them as 'physical' DR resources in the PHB's "ibm,drc-types"
property. In the case of the PHBs we advertise them as 'logical'/dlpar DR
resources in the root DT node's "ibm,drc-types" property. We do not actually
support 'logical' DR operations in this implementation though (though we may
in the future to support PHB hotplug), so we've set this to 'unusable' for
now.

But according to PAPR 2.7 13.5.3.3 this corresponds to:

"Returned for logical DR entities when the DR entity is not currently available
to the OS, but may possibly be made available to the OS by calling set-indicator
with the allocation-state indicator, setting that indicator to usable."

So maybe it makes more sense to just set it to present/(1)?

I don't think the PHB sensors will actually get read unless we attempt dlpar
operations in the guest (as opposed to pci hp), so it's probably mostly unused
now, but seems like a more sensible default. Will test and change it if it
doesn't break anything.

Macros make sense... we actually already have:

#define INDICATOR_ENTITY_SENSE_EMPTY    0
#define INDICATOR_ENTITY_SENSE_PRESENT  1

so not adding 'unused' was an oversight. I'll add it either way for
completeness.

> 
> 
> 
> >      }
> >  
> >      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?
> 
> 
> > +
> > +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

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 <nfont@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 <nfont@linux.vnet.ibm.com>
> >> Signed-off-by: Michael Roth <mdroth@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

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 <nfont@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 <nfont@linux.vnet.ibm.com>
> > >> Signed-off-by: Michael Roth <mdroth@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.

Hmm, this all also applies to the 0x2000001 drc base used for the parent PHB DRC
indices that Alex mentioned.

If we want to do something a little more structured, we could take the hotplug 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 lookup
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 = 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

Quoting Alexander Graf (2014-08-26 06:11:24)
> On 19.08.14 02:21, Michael Roth wrote:
> > From: Nathan Fontenot <nfont@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 <nfont@linux.vnet.ibm.com>
> > Signed-off-by: Michael Roth <mdroth@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.

> 
> 
> Alex
> 
> > +#define SPAPR_DRC_PHB_SLOT_MAX  32
> > +#define SPAPR_DRC_DEV_ID_BASE   0x40000000
> > +
> > +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__) */
> >

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 <nfont@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 <nfont@linux.vnet.ibm.com>
>>> Signed-off-by: Michael Roth <mdroth@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

On 08/26/2014 08:25 AM, Michael Roth wrote:
> 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 <nfont@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 <nfont@linux.vnet.ibm.com>
>>>> Signed-off-by: Michael Roth <mdroth@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.

It is ture that PAPR only requires that these values be unique, and I'm
not currently aware of guest tools that make assumptions about the DRC
values for different DRC connectors. However, seeing as we are emulating
a pseries guest I picked base DRC values that matched those used by PHYP.

CPU	0x10000000
VIO	0x30000000
LMB	0x80000000
PHB	0x20000000
PCI	0x40000000

-Tyrel

> 
>>
>>
>>>
>>>> +
>>>> +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

On 29.08.14 20:27, Tyrel Datwyler wrote:
> On 08/26/2014 08:25 AM, Michael Roth wrote:
>> 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 <nfont@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 <nfont@linux.vnet.ibm.com>
>>>>> Signed-off-by: Michael Roth <mdroth@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.
> 
> It is ture that PAPR only requires that these values be unique, and I'm
> not currently aware of guest tools that make assumptions about the DRC
> values for different DRC connectors. However, seeing as we are emulating
> a pseries guest I picked base DRC values that matched those used by PHYP.
> 
> CPU	0x10000000
> VIO	0x30000000
> LMB	0x80000000
> PHB	0x20000000
> PCI	0x40000000

Maybe we should keep these offsets (and boundary checks?) inside a
single spot, so that people can easily spot what the number space is
divided into.


Alex

On Tue, Aug 19, 2014 at 5:51 AM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> From: Nathan Fontenot <nfont@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 <nfont@linux.vnet.ibm.com>
> Signed-off-by: Michael Roth <mdroth@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];
> +            break;
> +        }
> +    }
> +
> +    if (found_drc) {
> +        return found_drc;
> +    }
> +
> +    if (empty_drc) {
> +        empty_drc->phb_buid = buid;
> +        empty_drc->state = state;

Shouldn't this be

empty_drc->state = state << INDICATOR_ENTITY_SENSE_SHIFT ?

Regards,
Bharata.

On 08/26/2014 12:55 AM, Alexey Kardashevskiy wrote:
> On 08/19/2014 10:21 AM, Michael Roth wrote:
>> From: Nathan Fontenot <nfont@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 <nfont@linux.vnet.ibm.com>
>> Signed-off-by: Michael Roth <mdroth@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?
> 

It is the state of the dr-entity-sensor. Definitions of those states can
be found in PAPR 13.5.3.3 in reference to the get-sensor-state rtas call
requirements for dynamic reconfiguration. That rtas call is introduced
later in this patchset, but oddly a the unusable state is left undefined
in the later patch. The get-sensor-state implementation has dependencies
on the DRC patches. Either, need to add the unusable definition into the
get-sensor-state patch and fix the magic number there as well, or the
sensor value defines could be split out into an earlier patch.

-Tyrel

> 
> 
>>      }
>>  
>>      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?
> 
> 
>> +
>> +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__) */
>>
> 
>

On 08/18/2014 05:21 PM, Michael Roth wrote:
> From: Nathan Fontenot <nfont@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 <nfont@linux.vnet.ibm.com>
> Signed-off-by: Michael Roth <mdroth@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 can be changed to SPAPR_DRC_PHB_ID_BASE. See below.

> +
> +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;
> +    }
> +
> +    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;
> +
> +    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 */);
>      }
> 
>      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

Change this to SPAPR_DRC_PCI_ID_BASE.

Add

#define SPAPR_DRC_PHB_ID_BASE  0x20000000

-Tyrel

> +
> +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__) */
>