Re: [Qemu-devel] [PATCH v5 10/16] spapr_drc: add spapr_drc_populate_dt()

[Michael Roth]

Quoting David Gibson (2015-02-24 03:26:42)
> On Mon, Feb 16, 2015 at 08:27:46AM -0600, Michael Roth wrote:
> > This function handles generation of ibm,drc-* array device tree
> > properties to describe DRC topology to guests. This will by used
> > by the guest to direct RTAS calls to manage any dynamic resources
> > we associate with a particular DR Connector as part of
> > hotplug/unplug.
> > 
> > Since general management of boot-time device trees are handled
> > outside of sPAPRDRConnector, we insert these values blindly given
> > an FDT and offset. A mask of sPAPRDRConnector types is given to
> > instruct us on what types of connectors entries should be generated
> > for, since descriptions for different connectors may live in
> > different parts of the device tree.
> > 
> > Based on code originally written by Nathan Fontenot.
> 
> Shouldn't he have a S-o-b then?  1st S-o-b is usually the same as
> Author, but it doesn't have to be.

Hi Nathan,

Can we get your S-o-b for this patch?

This is based on the code you wrote to reserve entries for additional
PHBs in the top-level DRC list in the device tree.

  http://lists.gnu.org/archive/html/qemu-ppc/2014-08/msg00162.html

I attributed the original code to you when we ended up dropping that patch
(to remove code that's specific to PHB hotplug), since it was the template
I had used for the PCI-specific counterparts of the DRC lists in
the device tree.

> 
> > 
> > Signed-off-by: Michael Roth <address@hidden>
> > ---
> >  hw/ppc/spapr_drc.c         | 165 
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  include/hw/ppc/spapr_drc.h |   4 +-
> >  2 files changed, 168 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > index 8b01200..2f71696 100644
> > --- a/hw/ppc/spapr_drc.c
> > +++ b/hw/ppc/spapr_drc.c
> > @@ -569,3 +569,168 @@ sPAPRDRConnector 
> > *spapr_dr_connector_by_id(sPAPRDRConnectorType type,
> >              (get_type_shift(type) << DRC_INDEX_TYPE_SHIFT) |
> >              (id & DRC_INDEX_ID_MASK));
> >  }
> > +
> > +/* generate a string the describes the DRC to encode into the
> > + * device tree.
> > + *
> > + * as documented by PAPR+ v2.7, 13.5.2.6 and C.6.1
> > + */
> > +static char *spapr_drc_get_type_str(sPAPRDRConnectorType type)
> > +{
> > +    char *type_str = NULL;
> > +
> > +    switch (type) {
> > +    case SPAPR_DR_CONNECTOR_TYPE_CPU:
> > +        type_str = g_strdup_printf("CPU");
> > +        break;
> > +    case SPAPR_DR_CONNECTOR_TYPE_PHB:
> > +        type_str = g_strdup_printf("PHB");
> > +        break;
> > +    case SPAPR_DR_CONNECTOR_TYPE_VIO:
> > +        type_str = g_strdup_printf("SLOT");
> > +        break;
> > +    case SPAPR_DR_CONNECTOR_TYPE_PCI:
> > +        type_str = g_strdup_printf("28");
> > +        break;
> > +    case SPAPR_DR_CONNECTOR_TYPE_LMB:
> > +        type_str = g_strdup_printf("MEM");
> > +        break;
> > +    default:
> > +        g_assert(false);
> > +    }
> > +
> > +    return type_str;
> > +}
> > +
> > +/**
> > + * spapr_drc_populate_dt
> > + *
> > + * @fdt: libfdt device tree
> > + * @path: path in the DT to generate properties
> > + * @owner: parent Object/DeviceState for which to generate DRC
> > + *         descriptions for
> > + * @drc_type_mask: mask of sPAPRDRConnectorType values corresponding
> > + *   to the types of DRCs to generate entries for
> > + *
> > + * generate OF properties to describe DRC topology/indices to guests
> > + *
> > + * as documented in PAPR+ v2.1, 13.5.2
> > + */
> > +int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
> > +                          uint32_t drc_type_mask)
> > +{
> > +    Object *root_container;
> > +    ObjectProperty *prop;
> > +    uint32_t drc_count = 0;
> > +    GArray *drc_indexes, *drc_power_domains;
> > +    GString *drc_names, *drc_types;
> > +    int ret;
> > +
> > +    /* the first entry of each properties is a 32-bit integer encoding
> > +     * the number of elements in the array. we won't know this until
> > +     * we complete the iteration through all the matching DRCs, but
> > +     * reserve the space now and set the offsets accordingly so we
> > +     * can fill them in later.
> > +     */
> > +    drc_indexes = g_array_new(false, true, sizeof(uint32_t));
> > +    drc_indexes = g_array_set_size(drc_indexes, 1);
> > +    drc_power_domains = g_array_new(false, true, sizeof(uint32_t));
> > +    drc_power_domains = g_array_set_size(drc_power_domains, 1);
> > +    drc_names = g_string_set_size(g_string_new(NULL), sizeof(uint32_t));
> > +    drc_types = g_string_set_size(g_string_new(NULL), sizeof(uint32_t));
> > +
> > +    /* aliases for all DRConnector objects will be rooted in QOM
> > +     * composition tree at DRC_CONTAINER_PATH
> > +     */
> > +    root_container = container_get(object_get_root(), DRC_CONTAINER_PATH);
> > +
> > +    QTAILQ_FOREACH(prop, &root_container->properties, node) {
> > +        Object *obj;
> > +        sPAPRDRConnector *drc;
> > +        sPAPRDRConnectorClass *drck;
> > +        char *drc_type;
> > +        uint32_t drc_index, drc_power_domain;
> > +
> > +        if (!strstart(prop->type, "link<", NULL)) {
> > +            continue;
> > +        }
> > +
> > +        obj = object_property_get_link(root_container, prop->name, NULL);
> > +        drc = SPAPR_DR_CONNECTOR(obj);
> > +        drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +
> > +        if (owner && (drc->owner != owner)) {
> > +            continue;
> > +        }
> > +
> > +        if ((drc->type & drc_type_mask) == 0) {
> > +            continue;
> > +        }
> > +
> > +        drc_count++;
> > +
> > +        /* ibm,drc-indexes */
> > +        drc_index = cpu_to_be32(drck->get_index(drc));
> > +        g_array_append_val(drc_indexes, drc_index);
> > +
> > +        /* ibm,drc-power-domains */
> > +        drc_power_domain = cpu_to_be32(-1);
> > +        g_array_append_val(drc_power_domains, drc_power_domain);
> > +
> > +        /* ibm,drc-names */
> > +        drc_names = g_string_append(drc_names, drck->get_name(drc));
> > +        drc_names = g_string_insert_len(drc_names, -1, "\0", 1);
> > +
> > +        /* ibm,drc-types */
> > +        drc_type = spapr_drc_get_type_str(drc->type);
> > +        drc_types = g_string_append(drc_types, drc_type);
> > +        drc_types = g_string_insert_len(drc_types, -1, "\0", 1);
> > +        g_free(drc_type);
> > +    }
> > + 
> > +    /* now write the drc count into the space we reserved at the
> > +     * beginning of the arrays previously
> > +     */
> > +    *(uint32_t *)drc_indexes->data = cpu_to_be32(drc_count);
> > +    *(uint32_t *)drc_power_domains->data = cpu_to_be32(drc_count);
> > +    *(uint32_t *)drc_names->str = cpu_to_be32(drc_count);
> > +    *(uint32_t *)drc_types->str = cpu_to_be32(drc_count);
> > +
> > +    ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-indexes",
> > +                      drc_indexes->data,
> > +                      drc_indexes->len * sizeof(uint32_t));
> > +    if (ret) {
> > +        fprintf(stderr, "Couldn't create ibm,drc-indexes property\n");
> > +        goto out;
> > +    }
> > +
> > +    ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-power-domains",
> > +                      drc_power_domains->data,
> > +                      drc_power_domains->len * sizeof(uint32_t));
> > +    if (ret) {
> > +        fprintf(stderr, "Couldn't finalize ibm,drc-power-domains 
> > property\n");
> > +        goto out;
> > +    }
> > +
> > +    ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-names",
> > +                      drc_names->str, drc_names->len);
> > +    if (ret) {
> > +        fprintf(stderr, "Couldn't finalize ibm,drc-names property\n");
> > +        goto out;
> > +    }
> > +
> > +    ret = fdt_setprop(fdt, fdt_offset, "ibm,drc-types",
> > +                      drc_types->str, drc_types->len);
> > +    if (ret) {
> > +        fprintf(stderr, "Couldn't finalize ibm,drc-types property\n");
> > +        goto out;
> > +    }
> > +
> > +out:
> > +    g_array_free(drc_indexes, true);
> > +    g_array_free(drc_power_domains, true);
> > +    g_string_free(drc_names, true);
> > +    g_string_free(drc_types, true);
> > +
> > +    return ret;
> > +}
> > diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> > index d6cf597..5895534 100644
> > --- a/include/hw/ppc/spapr_drc.h
> > +++ b/include/hw/ppc/spapr_drc.h
> > @@ -197,9 +197,11 @@ typedef struct sPAPRDRConnectorClass {
> >  
> >  sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
> >                                           sPAPRDRConnectorType type,
> > -                                         uint32_t token);
> > +                                         uint32_t id);
> 
> This appears to be an unrelated change.
> 
> >  sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index);
> >  sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type,
> >                                             uint32_t id);
> > +int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
> > +                          uint32_t drc_type_mask);
> >  
> >  #endif /* __HW_SPAPR_DRC_H__ */
> 
> -- 
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                 | _way_ _around_!
> http://www.ozlabs.org/~dgibson