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 <nf...@linux.vnet.ibm.com>
>>>>>
>>>>> This add entries to the root OF node to advertise our PHBs as being
>>>>> DR-capable in according with PAPR specification.
>>>>>
>>>>> Each PHB is given a name of PHB<bus#>, advertised as a PHB type,
>>>>> and associated with a power domain of -1 (indicating to guests that
>>>>> power management is handled automatically by hardware).
>>>>>
>>>>> We currently allocate entries for up to 32 DR-capable PHBs, though
>>>>> this limit can be increased later.
>>>>>
>>>>> DrcEntry objects to track the state of the DR-connector associated
>>>>> with each PHB are stored in a 32-entry array, and each DrcEntry has
>>>>> in turn have a dynamically-sized number of child DR-connectors,
>>>>> which we will use later to track the state of DR-connectors
>>>>> associated with a PHB's physical slots.
>>>>>
>>>>> Signed-off-by: Nathan Fontenot <nf...@linux.vnet.ibm.com>
>>>>> Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com>
>>>>> ---
>>>>> hw/ppc/spapr.c | 143
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> hw/ppc/spapr_pci.c | 1 +
>>>>> include/hw/ppc/spapr.h | 35 ++++++++++++
>>>>> 3 files changed, 179 insertions(+)
>>>>>
>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>>> index 5c92707..d5e46c3 100644
>>>>> --- a/hw/ppc/spapr.c
>>>>> +++ b/hw/ppc/spapr.c
>>>>> @@ -296,6 +296,143 @@ static hwaddr spapr_node0_size(void)
>>>>> return ram_size;
>>>>> }
>>>>>
>>>>> +sPAPRDrcEntry *spapr_phb_to_drc_entry(uint64_t buid)
>>>>> +{
>>>>> + int i;
>>>>> +
>>>>> + for (i = 0; i < SPAPR_DRC_TABLE_SIZE; i++) {
>>>>> + if (spapr->drc_table[i].phb_buid == buid) {
>>>>> + return &spapr->drc_table[i];
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + return NULL;
>>>>> +}
>>>>> +
>>>>> +static void spapr_init_drc_table(void)
>>>>> +{
>>>>> + int i;
>>>>> +
>>>>> + memset(spapr->drc_table, 0, sizeof(spapr->drc_table));
>>>>> +
>>>>> + /* For now we only care about PHB entries */
>>>>> + for (i = 0; i < SPAPR_DRC_TABLE_SIZE; i++) {
>>>>> + spapr->drc_table[i].drc_index = 0x2000001 + i;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +sPAPRDrcEntry *spapr_add_phb_to_drc_table(uint64_t buid, uint32_t state)
>>>>> +{
>>>>> + sPAPRDrcEntry *empty_drc = NULL;
>>>>> + sPAPRDrcEntry *found_drc = NULL;
>>>>> + int i, phb_index;
>>>>> +
>>>>> + for (i = 0; i < SPAPR_DRC_TABLE_SIZE; i++) {
>>>>> + if (spapr->drc_table[i].phb_buid == 0) {
>>>>> + empty_drc = &spapr->drc_table[i];
>>>>> + }
>>>>> +
>>>>> + if (spapr->drc_table[i].phb_buid == buid) {
>>>>> + found_drc = &spapr->drc_table[i];
>>>>
>>>> return &spapr->drc_table[i];
>>>> ?
>>>>
>>>>
>>>>> + break;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + if (found_drc) {
>>>>> + return found_drc;
>>>>> + }
>>>>
>>>> if (!empty_drc) {
>>>> return NULL;
>>>> }
>>>>
>>>> ?
>>>>
>>>>
>>>>> +
>>>>> + if (empty_drc) {
>>>>
>>>> and no need in this :)
>>>>
>>>>
>>>>> + empty_drc->phb_buid = buid;
>>>>> + empty_drc->state = state;
>>>>> + empty_drc->cc_state.fdt = NULL;
>>>>> + empty_drc->cc_state.offset = 0;
>>>>> + empty_drc->cc_state.depth = 0;
>>>>> + empty_drc->cc_state.state = CC_STATE_IDLE;
>>>>> + empty_drc->child_entries =
>>>>> + g_malloc0(sizeof(sPAPRDrcEntry) * SPAPR_DRC_PHB_SLOT_MAX);
>>>>> + phb_index = buid - SPAPR_PCI_BASE_BUID;
>>>>> + for (i = 0; i < SPAPR_DRC_PHB_SLOT_MAX; i++) {
>>>>> + empty_drc->child_entries[i].drc_index =
>>>>> + SPAPR_DRC_DEV_ID_BASE + (phb_index << 8) + (i << 3);
>>>>> + }
>>>>> + return empty_drc;
>>>>> + }
>>>>> +
>>>>> + return NULL;
>>>>> +}
>>>>> +
>>>>> +static void spapr_create_drc_dt_entries(void *fdt)
>>>>> +{
>>>>> + char char_buf[1024];
>>>>> + uint32_t int_buf[SPAPR_DRC_TABLE_SIZE + 1];
>>>>> + uint32_t *entries;
>>>>> + int offset, fdt_offset;
>>>>> + int i, ret;
>>>>> +
>>>>> + fdt_offset = fdt_path_offset(fdt, "/");
>>>>> +
>>>>> + /* ibm,drc-indexes */
>>>>> + memset(int_buf, 0, sizeof(int_buf));
>>>>> + int_buf[0] = SPAPR_DRC_TABLE_SIZE;
>>>>> +
>>>>> + for (i = 1; i <= SPAPR_DRC_TABLE_SIZE; i++) {
>>>>> + int_buf[i] = spapr->drc_table[i-1].drc_index;
>>>>> + }
>>>>> +
>>>>> + ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-indexes", int_buf,
>>>>> + sizeof(int_buf));
>>>>> + if (ret) {
>>>>> + fprintf(stderr, "Couldn't finalize ibm,drc-indexes property\n");
>>>>
>>>> return here and below in the same error cases?
>>>>
>>>>> + }
>>>>> +
>>>>> + /* ibm,drc-power-domains */
>>>>> + memset(int_buf, 0, sizeof(int_buf));
>>>>> + int_buf[0] = SPAPR_DRC_TABLE_SIZE;
>>>>> +
>>>>> + for (i = 1; i <= SPAPR_DRC_TABLE_SIZE; i++) {
>>>>> + int_buf[i] = 0xffffffff;
>>>>> + }
>>>>> +
>>>>> + ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-power-domains", int_buf,
>>>>> + sizeof(int_buf));
>>>>> + if (ret) {
>>>>> + fprintf(stderr, "Couldn't finalize ibm,drc-power-domains
>>>>> property\n");
>>>>> + }
>>>>> +
>>>>> + /* ibm,drc-names */
>>>>> + memset(char_buf, 0, sizeof(char_buf));
>>>>> + entries = (uint32_t *)&char_buf[0];
>>>>> + *entries = SPAPR_DRC_TABLE_SIZE;
>>>>> + offset = sizeof(*entries);
>>>>> +
>>>>> + for (i = 0; i < SPAPR_DRC_TABLE_SIZE; i++) {
>>>>> + offset += sprintf(char_buf + offset, "PHB %d", i + 1);
>>>>> + char_buf[offset++] = '\0';
>>>>> + }
>>>>> +
>>>>> + ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-names", char_buf,
>>>>> offset);
>>>>> + if (ret) {
>>>>> + fprintf(stderr, "Couldn't finalize ibm,drc-names property\n");
>>>>> + }
>>>>> +
>>>>> + /* ibm,drc-types */
>>>>> + memset(char_buf, 0, sizeof(char_buf));
>>>>> + entries = (uint32_t *)&char_buf[0];
>>>>> + *entries = SPAPR_DRC_TABLE_SIZE;
>>>>> + offset = sizeof(*entries);
>>>>> +
>>>>> + for (i = 0; i < SPAPR_DRC_TABLE_SIZE; i++) {
>>>>> + offset += sprintf(char_buf + offset, "PHB");
>>>>> + char_buf[offset++] = '\0';
>>>>> + }
>>>>> +
>>>>> + ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-types", char_buf,
>>>>> offset);
>>>>> + if (ret) {
>>>>> + fprintf(stderr, "Couldn't finalize ibm,drc-types property\n");
>>>>> + }
>>>>> +}
>>>>> +
>>>>> #define _FDT(exp) \
>>>>> do { \
>>>>> int ret = (exp); \
>>>>> @@ -731,6 +868,7 @@ static void spapr_finalize_fdt(sPAPREnvironment
>>>>> *spapr,
>>>>> char *bootlist;
>>>>> void *fdt;
>>>>> sPAPRPHBState *phb;
>>>>> + sPAPRDrcEntry *drc_entry;
>>>>>
>>>>> fdt = g_malloc(FDT_MAX_SIZE);
>>>>>
>>>>> @@ -750,6 +888,8 @@ static void spapr_finalize_fdt(sPAPREnvironment
>>>>> *spapr,
>>>>> }
>>>>>
>>>>> QLIST_FOREACH(phb, &spapr->phbs, list) {
>>>>> + drc_entry = spapr_phb_to_drc_entry(phb->buid);
>>>>> + g_assert(drc_entry);
>>>>> ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt);
>>>>> }
>>>>>
>>>>> @@ -789,6 +929,8 @@ static void spapr_finalize_fdt(sPAPREnvironment
>>>>> *spapr,
>>>>> spapr_populate_chosen_stdout(fdt, spapr->vio_bus);
>>>>> }
>>>>>
>>>>> + spapr_create_drc_dt_entries(fdt);
>>>>> +
>>>>> _FDT((fdt_pack(fdt)));
>>>>>
>>>>> if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
>>>>> @@ -1443,6 +1585,7 @@ static void ppc_spapr_init(MachineState *machine)
>>>>> spapr_pci_msi_init(spapr, SPAPR_PCI_MSI_WINDOW);
>>>>> spapr_pci_rtas_init();
>>>>>
>>>>> + spapr_init_drc_table();
>>>>> phb = spapr_create_phb(spapr, 0);
>>>>>
>>>>> for (i = 0; i < nb_nics; i++) {
>>>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>>>> index 9ed39a9..e85134f 100644
>>>>> --- a/hw/ppc/spapr_pci.c
>>>>> +++ b/hw/ppc/spapr_pci.c
>>>>> @@ -531,6 +531,7 @@ static void spapr_phb_realize(DeviceState *dev, Error
>>>>> **errp)
>>>>> + sphb->index * SPAPR_PCI_WINDOW_SPACING;
>>>>> sphb->mem_win_addr = windows_base + SPAPR_PCI_MMIO_WIN_OFF;
>>>>> sphb->io_win_addr = windows_base + SPAPR_PCI_IO_WIN_OFF;
>>>>> + spapr_add_phb_to_drc_table(sphb->buid, 2 /* Unusable */);
>>>>
>>>>
>>>> What exactly does "unusable" mean here? Macro?
>>>>
>>>>
>>>>
>>>>> }
>>>>>
>>>>> if (sphb->buid == -1) {
>>>>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>>>>> index 36e8e51..c93794b 100644
>>>>> --- a/include/hw/ppc/spapr.h
>>>>> +++ b/include/hw/ppc/spapr.h
>>>>> @@ -10,6 +10,36 @@ struct sPAPRNVRAM;
>>>>>
>>>>> #define HPTE64_V_HPTE_DIRTY 0x0000000000000040ULL
>>>>>
>>>>> +/* For dlparable/hotpluggable slots */
>>>>> +#define SPAPR_DRC_TABLE_SIZE 32
>>>>> +#define SPAPR_DRC_PHB_SLOT_MAX 32
>>>>> +#define SPAPR_DRC_DEV_ID_BASE 0x40000000
>>>>
>>>>
>>>> Is this SPAPR_DRC_DEV_ID_BASE really necessary (can it be zero)? Is that
>>>> global id or per PCI bus or per PHB?
>>>
>>>
>>> Ah. Got it. If it was like below, I would not even ask :)
>>>
>>> #define SPAPR_DRC_DEV_ID(phb_index, slot) \
>>> (0x40000000 | ((phb_index)<<8) | ((slot)<<3))
>>>
>>> Still not clear why you need this 0x40000000 for. Is it kind of "PHB" DRC
>>> type?
>>
>> Yes, it's somewhat ad-hoc...the only requirement I see in PAPR is that this
>> value be
>> globally unique across all DR resources. CPUs and memory and such might have
>> different
>> ways to compute their DRC indices (so a slot-based macro would need to be
>> specific
>> to PCI DR entries). I'm not sure where the 0x40000000 originated honestly.
>> I'm not
>> sure it matters for QEMU, since we hold a monopoly on all DRC index
>> assignments and
>> don't have to deal with hard-coded firmware values.
>>
>> I will say that a base somewhat less common than 0 may prove useful from a
>> debugging
>> standpoint, all other things being equal.
>>
>> So not sure what best to do here. If we choose to leave it as is, I could at
>> least
>> make sure to add a comment about this.
>
> 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
