[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v5 07/16] spapr_rtas: add ibm, configure-connector
From: |
Michael Roth |
Subject: |
Re: [Qemu-ppc] [PATCH v5 07/16] spapr_rtas: add ibm, configure-connector RTAS interface |
Date: |
Tue, 24 Feb 2015 14:43:45 -0600 |
User-agent: |
alot/0.3.4 |
Quoting David Gibson (2015-02-24 00:40:32)
> On Mon, Feb 16, 2015 at 08:27:43AM -0600, Michael Roth wrote:
> > This interface is used to fetch an OF device-tree nodes that describes a
> > newly-attached device to guest. It is called multiple times to walk the
> > device-tree node and fetch individual properties into a 'workarea'/buffer
> > provided by the guest.
> >
> > The device-tree is generated by QEMU and passed to an sPAPRDRConnector
> > during
> > the initial hotplug operation, and the state of these RTAS calls is tracked
> > by
> > the sPAPRDRConnector. When the last of these properties is successfully
> > fetched, we report as special return value to the guest and transition
> > the device to a 'configured' state on the QEMU/DRC side.
> >
> > See docs/specs/ppc-spapr-hotplug.txt for a complete description of
> > this interface.
> >
> > Signed-off-by: Michael Roth <address@hidden>
> > ---
> > hw/ppc/spapr_rtas.c | 81
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 81 insertions(+)
> >
> > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > index f80beb2..b8551b0 100644
> > --- a/hw/ppc/spapr_rtas.c
> > +++ b/hw/ppc/spapr_rtas.c
> > @@ -418,6 +418,85 @@ static void rtas_get_sensor_state(PowerPCCPU *cpu,
> > sPAPREnvironment *spapr,
> > rtas_st(rets, 1, entity_sense);
> > }
> >
> > +/* configure-connector work area offsets, int32_t units for field
> > + * indexes, bytes for field offset/len values.
> > + *
> > + * as documented by PAPR+ v2.7, 13.5.3.5
> > + */
> > +#define CC_IDX_NODE_NAME_OFFSET 2
> > +#define CC_IDX_PROP_NAME_OFFSET 2
> > +#define CC_IDX_PROP_LEN 3
> > +#define CC_IDX_PROP_DATA_OFFSET 4
> > +#define CC_VAL_DATA_OFFSET ((CC_IDX_PROP_DATA_OFFSET + 1) * 4)
> > +#define CC_WA_LEN 4096
> > +
> > +static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
> > + sPAPREnvironment *spapr,
> > + uint32_t token, uint32_t nargs,
> > + target_ulong args, uint32_t nret,
> > + target_ulong rets)
> > +{
> > + uint64_t wa_addr = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args,
> > 0);
>
> You need to check nargs and nret first.
Could've sworn I'd fixed all those cases. Will make sure to take care of it on
the next one.
>
> > + uint64_t wa_offset;
> > + uint32_t drc_index;
> > + sPAPRDRConnector *drc;
> > + sPAPRDRConnectorClass *drck;
> > + sPAPRDRCCResponse resp;
> > + const struct fdt_property *prop = NULL;
> > + char *prop_name = NULL;
> > + int prop_len, rc;
> > +
> > + drc_index = rtas_ld(wa_addr, 0);
> > + drc = spapr_dr_connector_by_index(drc_index);
> > + if (!drc) {
> > + DPRINTF("rtas_ibm_configure_connector: invalid sensor/DRC index:
> > %xh\n",
> > + drc_index);
> > + rc = RTAS_OUT_PARAM_ERROR;
> > + goto out;
> > + }
> > + drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > + resp = drck->configure_connector(drc, &prop_name, &prop, &prop_len);
>
> You may have answered this last time round, but if so I forgot the
> reason.
>
> Why does the awkward iteration need to go down to the drck callback?
> Coudln't the drck callback part just supply the fdt fragment blob,
> then have generic code which streams it out via iteration?
>
> Obviously we have to support the horrid PAPR interface, but it would
> be nice to confine the PAPR derived horridness to as small an area as
> we can.
That horrid interface percolates all the way up the QEMU stack,
unfortunately :)
Upon successfully having it's device tree node received, a DRC transitions
to a 'configured' state that's defined in the DR state machine (PAPR+ 13.4).
We need to track that state, since it's used to differentiate between
a case where a device is set to 'isolated' as part of entity-sense/device
configuration, as opposed to 'isolated' as part the unplug path. The
overlap between the 2 can occur if we do device_add followed by an immediate
device_del, but since the 'configured' transition must occur before the latter,
it becomes unambiguous.
It's also possible that a guest might be reset in the middle of a series of
calls to configure-connector, in which case that state needs to be reset. This
is currently handled by sPAPRDRConnector's reset hook, so if we moved that
aspect out I think we'd need to wire up a reset hook for the configure-connector
state, which is kinda messy. We'd also need a list of some sort, keyed by
the DRC indexes, to handle the subsequent call-per-iteration's (no guarantee
only one device configuration is 'in-flight' at a time), so we end up
duplicating a lot of tracking/functionality.
>
>
> > + switch (resp) {
> > + case SPAPR_DR_CC_RESPONSE_NEXT_CHILD:
> > + /* provide the name of the next OF node */
> > + wa_offset = CC_VAL_DATA_OFFSET;
> > + rtas_st(wa_addr, CC_IDX_NODE_NAME_OFFSET, wa_offset);
> > + rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
> > + (uint8_t *)prop_name, strlen(prop_name) + 1);
> > + break;
> > + case SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY:
> > + /* provide the name of the next OF property */
> > + wa_offset = CC_VAL_DATA_OFFSET;
> > + rtas_st(wa_addr, CC_IDX_PROP_NAME_OFFSET, wa_offset);
> > + rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
> > + (uint8_t *)prop_name, strlen(prop_name) + 1);
> > +
> > + /* provide the length and value of the OF property. data gets
> > placed
> > + * immediately after NULL terminator of the OF property's name
> > string
> > + */
> > + wa_offset += strlen(prop_name) + 1,
> > + rtas_st(wa_addr, CC_IDX_PROP_LEN, prop_len);
> > + rtas_st(wa_addr, CC_IDX_PROP_DATA_OFFSET, wa_offset);
> > + rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
> > + (uint8_t *)((struct fdt_property
> > *)prop)->data,
> > + prop_len);
> > + break;
> > + case SPAPR_DR_CC_RESPONSE_PREV_PARENT:
> > + case SPAPR_DR_CC_RESPONSE_ERROR:
> > + case SPAPR_DR_CC_RESPONSE_SUCCESS:
> > + break;
> > + default:
> > + /* drck->configure_connector() should not return anything else */
> > + g_assert(false);
> > + }
> > +
> > + rc = resp;
> > +out:
> > + g_free(prop_name);
> > + rtas_st(rets, 0, rc);
> > +}
> > +
> > static struct rtas_call {
> > const char *name;
> > spapr_rtas_fn fn;
> > @@ -551,6 +630,8 @@ static void core_rtas_register_types(void)
> > rtas_set_indicator);
> > spapr_rtas_register(RTAS_GET_SENSOR_STATE, "get-sensor-state",
> > rtas_get_sensor_state);
> > + spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR,
> > "ibm,configure-connector",
> > + rtas_ibm_configure_connector);
> > }
> >
> > type_init(core_rtas_register_types)
>
> --
> 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
- Re: [Qemu-ppc] [PATCH v5 04/16] spapr_rtas: add set-indicator RTAS interface, (continued)
- [Qemu-ppc] [PATCH v5 02/16] spapr_drc: initial implementation of sPAPRDRConnector device, Michael Roth, 2015/02/16
- [Qemu-ppc] [PATCH v5 06/16] spapr: add rtas_st_buffer_direct() helper, Michael Roth, 2015/02/16
- [Qemu-ppc] [PATCH v5 03/16] spapr_rtas: add get/set-power-level RTAS interfaces, Michael Roth, 2015/02/16
- [Qemu-ppc] [PATCH v5 05/16] spapr_rtas: add get-sensor-state RTAS interface, Michael Roth, 2015/02/16
- [Qemu-ppc] [PATCH v5 07/16] spapr_rtas: add ibm, configure-connector RTAS interface, Michael Roth, 2015/02/16
- Re: [Qemu-ppc] [PATCH v5 07/16] spapr_rtas: add ibm, configure-connector RTAS interface, David Gibson, 2015/02/24
- Re: [Qemu-ppc] [PATCH v5 07/16] spapr_rtas: add ibm, configure-connector RTAS interface,
Michael Roth <=
- Re: [Qemu-ppc] [PATCH v5 07/16] spapr_rtas: add ibm, configure-connector RTAS interface, David Gibson, 2015/02/24
- Re: [Qemu-ppc] [PATCH v5 07/16] spapr_rtas: add ibm, configure-connector RTAS interface, Michael Roth, 2015/02/26
- Re: [Qemu-ppc] [PATCH v5 07/16] spapr_rtas: add ibm, configure-connector RTAS interface, David Gibson, 2015/02/27
- Re: [Qemu-ppc] [PATCH v5 07/16] spapr_rtas: add ibm, configure-connector RTAS interface, Michael Roth, 2015/02/27
[Qemu-ppc] [PATCH v5 08/16] spapr_events: re-use EPOW event infrastructure for hotplug events, Michael Roth, 2015/02/16