qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 02/17] spapr_drc: initial implementation of s


From: Michael Roth
Subject: Re: [Qemu-devel] [PATCH v4 02/17] spapr_drc: initial implementation of sPAPRDRConnector device
Date: Sun, 25 Jan 2015 22:01:32 -0600
User-agent: alot/0.3.4

Quoting David Gibson (2015-01-16 00:19:08)
> On Tue, Dec 23, 2014 at 06:30:16AM -0600, Michael Roth wrote:
> > This device emulates a firmware abstraction used by pSeries guests to
> > manage hotplug/dynamic-reconfiguration of host-bridges, PCI devices,
> > memory, and CPUs. It is conceptually similar to an SHPC device,
> > complete with LED indicators to identify individual slots to physical
> > physical users and indicate when it is safe to remove a device. In
> > some cases it is also used to manage virtualized resources, such a
> > memory, CPUs, and physical-host bridges, which in the case of pSeries
> > guests are virtualized resources where the physical components are
> > managed by the host.
> > 
> > Guests communicate with these DR Connectors using RTAS calls,
> > generally by addressing the unique DRC index associated with a
> > particular connector for a particular resource. For introspection
> > purposes we expose this state initially as QOM properties, and
> > in subsequent patches will introduce the RTAS calls that make use of
> > it. This constitutes to the 'guest' interface.
> > 
> > On the QEMU side we provide an attach/detach interface to associate
> > or cleanup a DeviceState with a particular sPAPRDRConnector in
> > response to hotplug/unplug, respectively. This constitutes the
> > 'physical' interface to the DR Connector.
> > 
> > Signed-off-by: Michael Roth <address@hidden>
> > ---
> >  hw/ppc/Makefile.objs       |   2 +-
> >  hw/ppc/spapr_drc.c         | 503 
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  include/hw/ppc/spapr_drc.h | 201 ++++++++++++++++++
> >  3 files changed, 705 insertions(+), 1 deletion(-)
> >  create mode 100644 hw/ppc/spapr_drc.c
> >  create mode 100644 include/hw/ppc/spapr_drc.h
> > 
> > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> > index 19d9920..ea010fd 100644
> > --- a/hw/ppc/Makefile.objs
> > +++ b/hw/ppc/Makefile.objs
> > @@ -3,7 +3,7 @@ obj-y += ppc.o ppc_booke.o
> >  # IBM pSeries (sPAPR)
> >  obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
> >  obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
> > -obj-$(CONFIG_PSERIES) += spapr_pci.o
> > +obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_drc.o
> >  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> >  obj-y += spapr_pci_vfio.o
> >  endif
> > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > new file mode 100644
> > index 0000000..f81c6d1
> > --- /dev/null
> > +++ b/hw/ppc/spapr_drc.c
> > @@ -0,0 +1,503 @@
> > +/*
> > + * QEMU SPAPR Dynamic Reconfiguration Connector Implementation
> > + *
> > + * Copyright IBM Corp. 2014
> > + *
> > + * Authors:
> > + *  Michael Roth      <address@hidden>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or 
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "hw/ppc/spapr_drc.h"
> > +#include "qom/object.h"
> > +#include "hw/qdev.h"
> > +#include "qapi/visitor.h"
> > +#include "qemu/error-report.h"
> > +
> > +/* #define DEBUG_SPAPR_DRC */
> > +
> > +#ifdef DEBUG_SPAPR_DRC
> > +#define DPRINTF(fmt, ...) \
> > +    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> > +#define DPRINTFN(fmt, ...) \
> > +    do { DPRINTF(fmt, ## __VA_ARGS__); fprintf(stderr, "\n"); } while (0)
> > +#else
> > +#define DPRINTF(fmt, ...) \
> > +    do { } while (0)
> > +#define DPRINTFN(fmt, ...) \
> > +    do { } while (0)
> > +#endif
> > +
> > +#define DRC_CONTAINER_PATH "/dr-connector"
> > +#define DRC_INDEX_TYPE_SHIFT 28
> > +#define DRC_INDEX_ID_MASK ~(~0 << DRC_INDEX_TYPE_SHIFT)
> 
> I'm not sure if there are actually any situations where it can break,
> but safest to put parens around the whole macro body, just in case of
> macro vs. precedence weirdness.
> 
> > +static int set_isolation_state(sPAPRDRConnector *drc,
> > +                               sPAPRDRIsolationState state)
> > +{
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +
> > +    DPRINTFN("set_isolation_state: %x", state);
> > +    drc->isolation_state = state;
> > +    if (drc->awaiting_release &&
> > +        drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
> > +        drck->detach(drc, DEVICE(drc->dev), drc->detach_cb,
> > +                     drc->detach_cb_opaque);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int set_indicator_state(sPAPRDRConnector *drc,
> > +                               sPAPRDRIndicatorState state)
> > +{
> > +    DPRINTFN("set_indicator_state: %x", state);
> > +    drc->indicator_state = state;
> > +    return 0;
> > +}
> > +
> > +static int set_allocation_state(sPAPRDRConnector *drc,
> > +                                sPAPRDRAllocationState state)
> > +{
> > +    DPRINTFN("set_allocation_state: %x", state);
> > +    drc->indicator_state = state;
> 
> This should be drc->allocation_state, surely.
> 
> > +    return 0;
> > +}
> > +
> > +static uint32_t get_id(sPAPRDRConnector *drc)
> > +{
> > +    /* this value is unique for DRCs of a particular type, but may
> > +     * overlap with the id's of other DRCs. the value is used both
> > +     * to calculate a unique (across all DRC types) index, as well
> > +     * as generating the ibm,drc-names OFDT property that describes
> > +     * DRCs
> > +     */
> > +    return drc->id;
> > +}
> 
> Since this is static anyway, why not just reference drc->id directly?
> 
> > +static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type)
> > +{
> > +    uint32_t shift = 0;
> > +
> > +    g_assert(type != SPAPR_DR_CONNECTOR_TYPE_ANY);
> > +    while (type != (1 << shift)) {
> > +        shift++;
> > +    }
> > +    return shift;
> > +}
> > +
> > +static uint32_t get_index(sPAPRDRConnector *drc)
> > +{
> > +    /* no set format for a drc index: it only needs to be globally
> > +     * unique. this is how we encode the DRC type on bare-metal
> > +     * however, so might as well do that here
> > +     */
> > +    return (get_type_shift(drc->type) << DRC_INDEX_TYPE_SHIFT) |
> > +            (drc->id & DRC_INDEX_ID_MASK);
> > +}
> > +
> > +static uint32_t get_type(sPAPRDRConnector *drc)
> > +{
> > +    return drc->type;
> > +}
> > +
> > +/*
> > + * dr-entity-sense sensor value
> > + * returned via get-sensor-state RTAS calls
> > + * as expected by state diagram in PAPR+ 2.7, 13.4
> > + * based on the current allocation/indicator/power states
> > + * for the DR connector.
> > + */
> > +static sPAPRDREntitySense entity_sense(sPAPRDRConnector *drc)
> > +{
> > +    if (drc->dev) {
> > +        /* this assumes all PCI devices are assigned to
> > +         * a 'live insertion' power domain, where QEMU
> > +         * manages power state automatically as opposed
> > +         * to the guest. present, non-PCI resources are
> > +         * unaffected by power state.
> > +         */
> 
> Is it possible to make an assert() to check that?

If we find a need to model anything other than live-insertion
domains, we could maybe track it via a link property of the DRC,
but until then it's basically a hard-coded value we only use for
device tree creation so there's not really any state to assert at
this point.

> 
> > +        return SPAPR_DR_ENTITY_SENSE_PRESENT;
> > +    }
> > +
> > +    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> > +        /* PCI devices, and only PCI devices, use PRESENT
> > +         * in cases where we'd otherwise use UNUSABLE
> > +         */
> > +        return SPAPR_DR_ENTITY_SENSE_EMPTY;
> > +    }
> > +    return SPAPR_DR_ENTITY_SENSE_UNUSABLE;
> > +}
> > +
> > +static sPAPRDRCCResponse configure_connector_common(sPAPRDRCCState *ccs,
> > +                            char **prop_name, const struct fdt_property 
> > **prop,
> 
> Maybe rename prop_name to name, since it's also used for node names.
> 
> > +                            int *prop_len)
> > +{
> > +    sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE;
> > +    int fdt_offset_next;
> > +
> > +    *prop_name = NULL;
> > +    *prop = NULL;
> > +    *prop_len = 0;
> > +
> > +    if (!ccs->fdt) {
> > +        return SPAPR_DR_CC_RESPONSE_ERROR;
> > +    }
> > +
> > +    while (resp == SPAPR_DR_CC_RESPONSE_CONTINUE) {
> > +        const char *name_cur;
> > +        uint32_t tag;
> > +        int name_cur_len;
> > +
> > +        tag = fdt_next_tag(ccs->fdt, ccs->fdt_offset, &fdt_offset_next);
> > +        switch (tag) {
> > +        case FDT_BEGIN_NODE:
> > +            ccs->fdt_depth++;
> > +            name_cur = fdt_get_name(ccs->fdt, ccs->fdt_offset, 
> > &name_cur_len);
> > +            *prop_name = g_strndup(name_cur, name_cur_len);
> > +            resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD;
> > +            break;
> > +        case FDT_END_NODE:
> > +            ccs->fdt_depth--;
> > +            if (ccs->fdt_depth == 0) {
> > +                resp = SPAPR_DR_CC_RESPONSE_SUCCESS;
> > +            } else {
> > +                resp = SPAPR_DR_CC_RESPONSE_PREV_PARENT;
> > +            }
> > +            break;
> > +        case FDT_PROP:
> > +            *prop = fdt_get_property_by_offset(ccs->fdt, ccs->fdt_offset,
> > +                                               prop_len);
> > +            name_cur = fdt_string(ccs->fdt, 
> > fdt32_to_cpu((*prop)->nameoff));
> > +            *prop_name = g_strdup(name_cur);
> > +            resp = SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY;
> > +            break;
> > +        case FDT_END:
> > +            resp = SPAPR_DR_CC_RESPONSE_ERROR;
> > +            break;
> 
> IIUC, the fdt fragment you're stepping through here is generated
> within qemu.  In which case shouldn't this be an assert, rather than
> reporting an error to the guest?

It is, but I think it's possible for a buggy guest to continue making calls to
traverse the FDT fragment even though we've already reported completion in a
previous RTAS response, so I think it makes sense to just report an error to
the guest.

I think it does make sense to assert resp != SPAPR_DR_CC_RESPONSE_ERROR in
prop_get_fdt user below on the basis you mentioned though, since that's QEMU
traversing the FDT in response to a QMP call. Will add for v5.

> 
> > +        default:
> > +            ccs->fdt_offset = fdt_offset_next;
> > +        }
> > +    }
> > +
> > +    ccs->fdt_offset = fdt_offset_next;
> > +    return resp;
> > +}
> > +
> > +static sPAPRDRCCResponse configure_connector(sPAPRDRConnector *drc,
> > +                                             char **prop_name,
> > +                                             const struct fdt_property 
> > **prop,
> > +                                             int *prop_len)
> > +{
> > +    return configure_connector_common(&drc->ccs, prop_name, prop, 
> > prop_len);
> > +}
> > +
> > +static void prop_get_id(Object *obj, Visitor *v, void *opaque,
> > +                                  const char *name, Error **errp)
> > +{
> > +    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
> > +    uint32_t value = get_id(drc);
> > +    visit_type_uint32(v, &value, name, errp);
> > +}
> > +
> > +static void prop_get_index(Object *obj, Visitor *v, void *opaque,
> > +                                  const char *name, Error **errp)
> > +{
> > +    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +    uint32_t value = (uint32_t)drck->get_index(drc);
> > +    visit_type_uint32(v, &value, name, errp);
> > +}
> > +
> > +static void prop_get_type(Object *obj, Visitor *v, void *opaque,
> > +                          const char *name, Error **errp)
> > +{
> > +    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +    uint32_t value = (uint32_t)drck->get_type(drc);
> > +    visit_type_uint32(v, &value, name, errp);
> > +}
> > +
> > +static void prop_get_entity_sense(Object *obj, Visitor *v, void *opaque,
> > +                                  const char *name, Error **errp)
> > +{
> > +    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +    uint32_t value = (uint32_t)drck->entity_sense(drc);
> > +    visit_type_uint32(v, &value, name, errp);
> > +}
> > +
> > +static void prop_get_fdt(Object *obj, Visitor *v, void *opaque,
> > +                        const char *name, Error **errp)
> > +{
> > +    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
> > +    sPAPRDRCCState ccs = { 0 };
> > +    sPAPRDRCCResponse resp;
> > +
> > +    ccs.fdt = drc->ccs.fdt;
> > +    ccs.fdt_offset = ccs.fdt_start_offset = drc->ccs.fdt_start_offset;
> > +
> > +    do {
> > +        char *prop_name = NULL;
> > +        const struct fdt_property *prop = NULL;
> > +        int prop_len;
> > +
> > +        resp = configure_connector_common(&ccs, &prop_name, &prop, 
> > &prop_len);
> > +
> > +        switch (resp) {
> > +        case SPAPR_DR_CC_RESPONSE_NEXT_CHILD:
> > +            visit_start_struct(v, NULL, NULL, prop_name, 0, NULL);
> > +            break;
> > +        case SPAPR_DR_CC_RESPONSE_PREV_PARENT:
> > +            visit_end_struct(v, NULL);
> > +            break;
> > +        case SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY: {
> > +            int i;
> > +            visit_start_list(v, prop_name, NULL);
> > +            for (i = 0; i < prop_len; i++) {
> > +                visit_type_uint8(v, (uint8_t *)&prop->data[i], NULL, NULL);
> > +            }
> > +            visit_end_list(v, NULL);
> > +            break;
> > +        }
> > +        default:
> > +            resp = SPAPR_DR_CC_RESPONSE_SUCCESS;
> > +            break;
> > +        }
> > +
> > +        g_free(prop_name);
> > +    } while (resp != SPAPR_DR_CC_RESPONSE_SUCCESS &&
> > +             resp != SPAPR_DR_CC_RESPONSE_ERROR);
> > +}
> > +
> > +static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
> > +                   int fdt_start_offset, bool coldplug)
> > +{
> > +    DPRINTFN("attach");
> > +
> > +    g_assert(drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED);
> > +    g_assert(drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE);
> > +    g_assert(drc->indicator_state == SPAPR_DR_INDICATOR_STATE_INACTIVE);
> > +    g_assert(fdt || coldplug);
> > +
> > +    /* NOTE: setting initial isolation state to UNISOLATED means we can't
> > +     * detach unless guest has a userspace/kernel that moves this state
> > +     * back to ISOLATED in response to an unplug event, or this is done
> > +     * manually by the admin prior. if we force things while the guest
> > +     * may be accessing the device, we can easily crash the guest, so we
> > +     * we defer completion of removal in such cases to the reset() hook.
> > +     */
> 
> Given that, would it make more sense to start in ISOLATED state?  Or
> is the initial state specified by PAPR?

From my read of the state machine in 13.4, it's possible to populate
a PCI slot and power on the device without immediately transitioning
to UNISOLATED, but in the case of QEMU, by the time we attach the
device via the PCI hotplug path it's already been exposed to the guest
in the sense that it can be probed/configured, so I think it's unsafe
to claim it's UNISOLATED at this point since there's a risk of
unplugging while a guest thinks there's something there.

One possible way to work around this might be to start off with
ISOLATED, but add hooks in rtas_ibm_{read,write}_pci_config that
immediately transition the state to UNISOLATED if the guest actually
accesses/configures it. Seems hacky though... the real-world benefit
is that a guest that isn't capable of PCI hotplug will still allow
for immediate attach/detach on the QEMU, as opposed to requiring a
reboot to complete the detach. I'm not sure if it's worth it, but
can look into that if this seems better.

> 
> > +    drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
> > +    drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE;
> > +    drc->indicator_state = SPAPR_DR_INDICATOR_STATE_ACTIVE;
> > +
> > +    drc->dev = d;
> > +    drc->ccs.fdt = fdt;
> > +    drc->ccs.fdt_offset = drc->ccs.fdt_start_offset = fdt_start_offset;
> > +    drc->ccs.fdt_depth = 0;
> > +
> > +    object_property_add_link(OBJECT(drc), "device",
> > +                             object_get_typename(OBJECT(drc->dev)),
> > +                             (Object **)(&drc->dev),
> > +                             NULL, 0, NULL);
> > +}
> > +
> > +static void detach(sPAPRDRConnector *drc, DeviceState *d,
> > +                   spapr_drc_detach_cb *detach_cb,
> > +                   void *detach_cb_opaque)
> > +{
> > +    DPRINTFN("detach");
> > +
> > +    drc->detach_cb = detach_cb;
> > +    drc->detach_cb_opaque = detach_cb_opaque;
> > +
> > +    if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_ISOLATED) {
> > +        DPRINTFN("awaiting transition to isolated state before removal");
> > +        drc->awaiting_release = true;
> > +        return;
> > +    }
> > +
> > +    drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_UNUSABLE;
> > +    drc->indicator_state = SPAPR_DR_INDICATOR_STATE_INACTIVE;
> > +
> > +    if (drc->detach_cb) {
> > +        drc->detach_cb(drc->dev, drc->detach_cb_opaque);
> > +    }
> > +
> > +    drc->awaiting_release = false;
> > +    g_free(drc->ccs.fdt);
> > +    drc->ccs.fdt = NULL;
> > +    drc->ccs.fdt_offset = drc->ccs.fdt_start_offset = drc->ccs.fdt_depth = 
> > 0;
> > +    object_property_del(OBJECT(drc), "device", NULL);
> > +    drc->dev = NULL;
> > +    drc->detach_cb = NULL;
> > +    drc->detach_cb_opaque = NULL;
> 
> Shouldn't all this code after the detach_cb call also be called from
> set_isolation_state in the case of a deferred detach?  In which case
> you probably want a helper.

set_isolation_state() actually calls drc->detach(), as opposed calling to
drc->detach_cb() directly, so we end up back here in both cases.

> 
> > +}
> > +
> > +static void reset(DeviceState *d)
> > +{
> > +    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d);
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +
> > +    DPRINTFN("drc reset: %x", drck->get_index(drc));
> > +    /* immediately upon reset we can safely assume DRCs whose devices are 
> > pending
> > +     * removal can be safely removed, and that they will subsequently be 
> > left in
> > +     * an ISOLATED state. move the DRC to this state in these cases (which 
> > will in
> > +     * turn complete any pending device removals)
> > +     */
> > +    if (drc->awaiting_release) {
> > +        drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_ISOLATED);
> > +    }
> > +}
> > +
> > +static void realize(DeviceState *d, Error **errp)
> > +{
> > +    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d);
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +    Object *root_container;
> > +    char link_name[256];
> > +    gchar *child_name;
> > +    Error *err = NULL;
> > +
> > +    DPRINTFN("drc realize: %x", drck->get_index(drc));
> > +    /* NOTE: we do this as part of realize/unrealize due to the fact
> > +     * that the guest will communicate with the DRC via RTAS calls
> > +     * referencing the global DRC index. By unlinking the DRC
> > +     * from DRC_CONTAINER_PATH/<drc_index> we effectively make it
> > +     * inaccessible by the guest, since lookups rely on this path
> > +     * existing in the composition tree
> > +     */
> > +    root_container = container_get(object_get_root(), DRC_CONTAINER_PATH);
> > +    snprintf(link_name, sizeof(link_name), "%x", drck->get_index(drc));
> > +    child_name = object_get_canonical_path_component(OBJECT(drc));
> > +    DPRINTFN("drc child name: %s", child_name);
> > +    object_property_add_alias(root_container, link_name,
> > +                              drc->owner, child_name, &err);
> > +    /*
> > +    object_property_add_link(root_container, name, TYPE_SPAPR_DR_CONNECTOR,
> > +                             (Object **)&drc, NULL,
> > +                             OBJ_PROP_LINK_UNREF_ON_RELEASE, &err);
> > +                             */
> > +    if (err) {
> > +        error_report("%s", error_get_pretty(err));
> > +        error_free(err);
> > +        object_unref(OBJECT(drc));
> > +    }
> > +    DPRINTFN("drc realize complete");
> > +}
> > +
> > +static void unrealize(DeviceState *d, Error **errp)
> > +{
> > +    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d);
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +    Object *root_container;
> > +    char name[256];
> > +    Error *err = NULL;
> > +
> > +    DPRINTFN("drc unrealize: %x", drck->get_index(drc));
> > +    root_container = container_get(object_get_root(), DRC_CONTAINER_PATH);
> > +    snprintf(name, sizeof(name), "%x", drck->get_index(drc));
> > +    object_property_del(root_container, name, &err);
> > +    if (err) {
> > +        error_report("%s", error_get_pretty(err));
> > +        error_free(err);
> > +        object_unref(OBJECT(drc));
> > +    }
> > +}
> > +
> > +sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
> > +                                         sPAPRDRConnectorType type,
> > +                                         uint32_t id)
> > +{
> > +    sPAPRDRConnector *drc =
> > +        SPAPR_DR_CONNECTOR(object_new(TYPE_SPAPR_DR_CONNECTOR));
> > +
> > +    g_assert(type);
> > +
> > +    drc->type = type;
> > +    drc->id = id;
> > +    drc->owner = owner;
> > +    object_property_add_child(owner, "dr-connector[*]", OBJECT(drc), NULL);
> > +    object_property_set_bool(OBJECT(drc), true, "realized", NULL);
> > +
> > +    return drc;
> > +}
> > +
> > +static void spapr_dr_connector_instance_init(Object *obj)
> > +{
> > +    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
> > +
> > +    object_property_add_uint32_ptr(obj, "isolation-state",
> > +                                   &drc->isolation_state, NULL);
> > +    object_property_add_uint32_ptr(obj, "indicator-state",
> > +                                   &drc->indicator_state, NULL);
> > +    object_property_add_uint32_ptr(obj, "allocation-state",
> > +                                   &drc->allocation_state, NULL);
> 
> Don't these QOM properties need to be bound to set_isolation_state
> etc. for the write side?  Or does add_uint32_ptr only allow reads?

Only reads on add_*_ptr properties. Easily worked around though,but in
this case we're only exposing the properties here for introspection so
it's not really needed since the accessors that RTAS uses to
read/write the DRC state don't use these accessors to do it, they use
object methods, similar to the prop_get_* accessors below. 

We could allow these to be writeable properties, but I can't think
of a use-case outside of debugging the emulation path (which is
probably better done via something like qtest).

> 
> > +    object_property_add(obj, "id", "uint32", prop_get_id,
> > +                        NULL, NULL, NULL, NULL);
> > +    object_property_add(obj, "index", "uint32", prop_get_index,
> > +                        NULL, NULL, NULL, NULL);
> > +    object_property_add(obj, "index", "uint32", prop_get_type,
> > +                        NULL, NULL, NULL, NULL);
> > +    object_property_add(obj, "entity-sense", "uint32", 
> > prop_get_entity_sense,
> > +                        NULL, NULL, NULL, NULL);
> > +    object_property_add(obj, "fdt", "struct", prop_get_fdt,
> > +                        NULL, NULL, NULL, NULL);
> > +}
> > +
> > +static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
> > +{
> > +    DeviceClass *dk = DEVICE_CLASS(k);
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
> > +
> > +    dk->reset = reset;
> > +    dk->realize = realize;
> > +    dk->unrealize = unrealize;
> > +    drck->set_isolation_state = set_isolation_state;
> > +    drck->set_indicator_state = set_indicator_state;
> > +    drck->set_allocation_state = set_allocation_state;
> > +    drck->get_index = get_index;
> > +    drck->get_type = get_type;
> > +    drck->entity_sense = entity_sense;
> > +    drck->configure_connector = configure_connector;
> > +    drck->attach = attach;
> > +    drck->detach = detach;
> > +}
> > +
> > +static const TypeInfo spapr_dr_connector_info = {
> > +    .name          = TYPE_SPAPR_DR_CONNECTOR,
> > +    .parent        = TYPE_DEVICE,
> > +    .instance_size = sizeof(sPAPRDRConnector),
> > +    .instance_init = spapr_dr_connector_instance_init,
> > +    .class_size    = sizeof(sPAPRDRConnectorClass),
> > +    .class_init    = spapr_dr_connector_class_init,
> > +};
> > +
> > +static void spapr_drc_register_types(void)
> > +{
> > +    type_register_static(&spapr_dr_connector_info);
> > +}
> > +
> > +type_init(spapr_drc_register_types)
> > +
> > +/* helper functions for external users */
> > +
> > +sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index)
> > +{
> > +    Object *obj;
> > +    char name[256];
> > +
> > +    snprintf(name, sizeof(name), "%s/%x", DRC_CONTAINER_PATH, index);
> > +    obj = object_resolve_path(name, NULL);
> > +
> > +    return !obj ? NULL : SPAPR_DR_CONNECTOR(obj);
> > +}
> > +
> > +sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type,
> > +                                           uint32_t id)
> > +{
> > +    return spapr_dr_connector_by_index(
> > +            (get_type_shift(type) << DRC_INDEX_TYPE_SHIFT) |
> > +            (id & DRC_INDEX_ID_MASK));
> > +}
> > diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> > new file mode 100644
> > index 0000000..63ec687
> > --- /dev/null
> > +++ b/include/hw/ppc/spapr_drc.h
> > @@ -0,0 +1,201 @@
> > +/*
> > + * QEMU SPAPR Dynamic Reconfiguration Connector Implementation
> > + *
> > + * Copyright IBM Corp. 2014
> > + *
> > + * Authors:
> > + *  Michael Roth      <address@hidden>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or 
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +#if !defined(__HW_SPAPR_DRC_H__)
> > +#define __HW_SPAPR_DRC_H__
> > +
> > +#include "qom/object.h"
> > +#include "hw/qdev.h"
> > +#include "libfdt.h"
> > +
> > +#define TYPE_SPAPR_DR_CONNECTOR "spapr-dr-connector"
> > +#define SPAPR_DR_CONNECTOR_GET_CLASS(obj) \
> > +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, 
> > TYPE_SPAPR_DR_CONNECTOR)
> > +#define SPAPR_DR_CONNECTOR_CLASS(klass) \
> > +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, \
> > +                           TYPE_SPAPR_DR_CONNECTOR)
> > +#define SPAPR_DR_CONNECTOR(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> > +                                             TYPE_SPAPR_DR_CONNECTOR)
> > +
> > +/*
> > + * Various hotplug types managed by sPAPRDRConnector
> > + *
> > + * these are somewhat arbitrary, but to make things easier
> > + * when generating DRC indexes later we've aligned the bit
> > + * positions with the values used to assign DRC indexes on
> > + * pSeries. we use those values as bit shifts to allow for
> > + * the OR'ing of these values in various QEMU routines, but
> > + * for values exposed to the guest (via DRC indexes for
> > + * instance) we will use the shift amounts.
> > + */
> > +typedef enum {
> > +    SPAPR_DR_CONNECTOR_TYPE_SHIFT_CPU = 1,
> > +    SPAPR_DR_CONNECTOR_TYPE_SHIFT_PHB = 2,
> > +    SPAPR_DR_CONNECTOR_TYPE_SHIFT_VIO = 3,
> > +    SPAPR_DR_CONNECTOR_TYPE_SHIFT_PCI = 4,
> > +    SPAPR_DR_CONNECTOR_TYPE_SHIFT_LMB = 8,
> > +} sPAPRDRConnectorTypeShift;
> > +
> > +typedef enum {
> > +    SPAPR_DR_CONNECTOR_TYPE_ANY = ~0,
> > +    SPAPR_DR_CONNECTOR_TYPE_CPU = 1 << SPAPR_DR_CONNECTOR_TYPE_SHIFT_CPU,
> > +    SPAPR_DR_CONNECTOR_TYPE_PHB = 1 << SPAPR_DR_CONNECTOR_TYPE_SHIFT_PHB,
> > +    SPAPR_DR_CONNECTOR_TYPE_VIO = 1 << SPAPR_DR_CONNECTOR_TYPE_SHIFT_VIO,
> > +    SPAPR_DR_CONNECTOR_TYPE_PCI = 1 << SPAPR_DR_CONNECTOR_TYPE_SHIFT_PCI,
> > +    SPAPR_DR_CONNECTOR_TYPE_LMB = 1 << SPAPR_DR_CONNECTOR_TYPE_SHIFT_LMB,
> > +} sPAPRDRConnectorType;
> > +
> > +/*
> > + * set via set-indicator RTAS calls
> > + * as documented by PAPR+ 2.7 13.5.3.4, Table 177
> > + *
> > + * isolated: put device under firmware control 
> > + * unisolated: claim OS control of device (may or may not be in use)
> > + */
> > +typedef enum {
> > +    SPAPR_DR_ISOLATION_STATE_ISOLATED   = 0,
> > +    SPAPR_DR_ISOLATION_STATE_UNISOLATED = 1
> > +} sPAPRDRIsolationState;
> > +
> > +/*
> > + * set via set-indicator RTAS calls
> > + * as documented by PAPR+ 2.7 13.5.3.4, Table 177
> > + *
> > + * unusable: mark device as unavailable to OS
> > + * usable: mark device as available to OS
> > + * exchange: (currently unused)
> > + * recover: (currently unused)
> > + */
> > +typedef enum {
> > +    SPAPR_DR_ALLOCATION_STATE_UNUSABLE  = 0,
> > +    SPAPR_DR_ALLOCATION_STATE_USABLE    = 1,
> > +    SPAPR_DR_ALLOCATION_STATE_EXCHANGE  = 2,
> > +    SPAPR_DR_ALLOCATION_STATE_RECOVER   = 3
> > +} sPAPRDRAllocationState;
> > +
> > +/*
> > + * LED/visual indicator state
> > + *
> > + * set via set-indicator RTAS calls
> > + * as documented by PAPR+ 2.7 13.5.3.4, Table 177,
> > + * and PAPR+ 2.7 13.5.4.1, Table 180
> > + *
> > + * inactive: hotpluggable entity inactive and safely removable
> > + * active: hotpluggable entity in use and not safely removable
> > + * identify: (currently unused)
> > + * action: (currently unused)
> > + */
> > +typedef enum {
> > +    SPAPR_DR_INDICATOR_STATE_INACTIVE   = 0,
> > +    SPAPR_DR_INDICATOR_STATE_ACTIVE     = 1,
> > +    SPAPR_DR_INDICATOR_STATE_IDENTIFY   = 2,
> > +    SPAPR_DR_INDICATOR_STATE_ACTION     = 3,
> > +} sPAPRDRIndicatorState;
> > +
> > +/*
> > + * returned via get-sensor-state RTAS calls
> > + * as documented by PAPR+ 2.7 13.5.3.3, Table 175:
> > + *
> > + * empty: connector slot empty (e.g. empty hotpluggable PCI slot)
> > + * present: connector slot populated and device available to OS
> > + * unusable: device not currently available to OS
> > + * exchange: (currently unused)
> > + * recover: (currently unused)
> > + */
> > +typedef enum {
> > +    SPAPR_DR_ENTITY_SENSE_EMPTY     = 0,
> > +    SPAPR_DR_ENTITY_SENSE_PRESENT   = 1,
> > +    SPAPR_DR_ENTITY_SENSE_UNUSABLE  = 2,
> > +    SPAPR_DR_ENTITY_SENSE_EXCHANGE  = 3,
> > +    SPAPR_DR_ENTITY_SENSE_RECOVER   = 4,
> > +} sPAPRDREntitySense;
> > +
> > +typedef enum {
> > +    SPAPR_DR_CC_RESPONSE_NEXT_SIB       = 1, /* currently unused */
> > +    SPAPR_DR_CC_RESPONSE_NEXT_CHILD     = 2,
> > +    SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY  = 3,
> > +    SPAPR_DR_CC_RESPONSE_PREV_PARENT    = 4,
> > +    SPAPR_DR_CC_RESPONSE_SUCCESS        = 0,
> > +    SPAPR_DR_CC_RESPONSE_ERROR          = -1,
> > +    SPAPR_DR_CC_RESPONSE_CONTINUE       = -2,
> > +} sPAPRDRCCResponse;
> > +
> > +typedef struct sPAPRDRCCState {
> > +    void *fdt;
> > +    int fdt_start_offset;
> > +    int fdt_offset;
> > +    int fdt_depth;
> > +} sPAPRDRCCState;
> > +
> > +typedef void (spapr_drc_detach_cb)(DeviceState *d, void *opaque);
> > +
> > +typedef struct sPAPRDRConnector {
> > +    /*< private >*/
> > +    DeviceState parent;
> > +
> > +    sPAPRDRConnectorType type;
> > +    uint32_t id;
> > +    Object *owner;
> > +
> > +    /* sensor/indicator states */
> > +    uint32_t isolation_state;
> > +    uint32_t allocation_state;
> > +    uint32_t indicator_state;
> > +
> > +    /* configure-connector state */
> > +    sPAPRDRCCState ccs;
> > +
> > +    bool awaiting_release;
> > +
> > +    /* device pointer, via link property */
> > +    DeviceState *dev;
> > +    spapr_drc_detach_cb *detach_cb;
> > +    void *detach_cb_opaque;
> > +} sPAPRDRConnector;
> > +
> > +typedef struct sPAPRDRConnectorClass {
> > +    /*< private >*/
> > +    DeviceClass parent;
> > +
> > +    /*< public >*/
> > +
> > +    /* accessors for guest-visible (generally via RTAS) DR state */
> > +    int (*set_isolation_state)(sPAPRDRConnector *drc,
> > +                               sPAPRDRIsolationState state);
> > +    int (*set_indicator_state)(sPAPRDRConnector *drc,
> > +                               sPAPRDRIndicatorState state);
> > +    int (*set_allocation_state)(sPAPRDRConnector *drc,
> > +                                sPAPRDRAllocationState state);
> > +    uint32_t (*get_index)(sPAPRDRConnector *drc);
> > +    uint32_t (*get_type)(sPAPRDRConnector *drc);
> > +
> > +    sPAPRDREntitySense (*entity_sense)(sPAPRDRConnector *drc);
> > +    sPAPRDRCCResponse (*configure_connector)(sPAPRDRConnector *drc,
> > +                                             char **prop_name,
> > +                                             const struct fdt_property 
> > **prop,
> > +                                             int *prop_len);
> > +
> > +    /* QEMU interfaces for managing hotplug operations */
> > +    void (*attach)(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
> > +                   int fdt_start_offset, bool coldplug);
> > +    void (*detach)(sPAPRDRConnector *drc, DeviceState *d,
> > +                   spapr_drc_detach_cb *detach_cb,
> > +                   void *detach_cb_opaque);
> > +} sPAPRDRConnectorClass;
> > +
> > +sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
> > +                                         sPAPRDRConnectorType type,
> > +                                         uint32_t token);
> > +sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index);
> > +sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type,
> > +                                           uint32_t id);
> > +
> > +#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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]