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

[Michael Roth <mdroth@linux.vnet.ibm.com>]

Quoting Bharata B Rao (2015-01-02 04:32:58)
> On Tue, Dec 23, 2014 at 6:00 PM, Michael Roth <mdroth@linux.vnet.ibm.com> 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 <mdroth@linux.vnet.ibm.com>
> > ---
> >  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      <mdroth@linux.vnet.ibm.com>
> > + *
> > + * 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)
> > +
> > +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;
> > +    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;
> > +}
> > +
> > +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.
> > +         */
> > +        return SPAPR_DR_ENTITY_SENSE_PRESENT;
> > +    }
> 
> Hotplugged CPU comes with drc->dev set (set during
> sPAPRDRConnectorClass->attach()) and hence ends up returning PRESENT
> but kernel expects UNUSABLE during get-sensor-state rtas call.

That does seem to be in agreement with the state machine in PAPR+ 13.4.

I've updated the check to return UNUSABLE/EMPTY if the allocation
state is UNUSABLE when dealing with logical DR devices. For PCI we
transition to PRESENT have plug/power-on regardless of allocation
state.

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