| Message ID | 1424096872-29868-8-git-send-email-mdroth@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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 <mdroth@linux.vnet.ibm.com> > --- > 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. > + 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. > + 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)
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 <mdroth@linux.vnet.ibm.com> > > --- > > 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
On Tue, Feb 24, 2015 at 02:43:45PM -0600, Michael Roth wrote: > 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 <mdroth@linux.vnet.ibm.com> > > > --- > > > 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. Hmm. You should still be able to handle that with just 2 hooks and 1 bit of state right? Say "start_configuration" and "end_configuration" or similar. start_configuration gets the blob from the backend, then end_configuration is called once RTAS has finished streaming it out to the guest and updates to the cnofigured state.
Quoting David Gibson (2015-02-24 18:48:23) > On Tue, Feb 24, 2015 at 02:43:45PM -0600, Michael Roth wrote: > > 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 <mdroth@linux.vnet.ibm.com> > > > > --- > > > > 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. > > Hmm. You should still be able to handle that with just 2 hooks and 1 > bit of state right? Say "start_configuration" and "end_configuration" > or similar. start_configuration gets the blob from the backend, then > end_configuration is called once RTAS has finished streaming it out to > the guest and updates to the cnofigured state. {start,end}_configuration callbacks would work for handling the normal 'configured' state transitions induced by the call, but we'd also need hooks in the "other direction" for a couple cases: This scenario for instance: qemu: guest: add device0 to drc0 drmgr0: rtas-configure-connector drc0 drc0->start_configuration... drmgr0: rtas-configure-connector drc0 drmgr0: rtas-configure-connector drc0 ... drmgr0: rtas-configure-connector drc0 del device0 <device0 removal pending> drmgr0: rtas-configure-connector drc0 system_reset device0 removed by drc0 reset hook add device1 to drc0 drmgr0: rtas-configure-connector drc0 <begins fetching stale FDT> So I think we'd need at least a reset hook wired up to the RTAS state. It's also possible for the guest to force a transition out of the configured state by ISOLATE'ing the device. This can happen in the middle of the guests configure-connector calls if there's an error. If rtas-configure-connector is the one generating the error, it can anticipate this and automatically reset the state, but in some cases the error is guest internal: the get_node() in src/drmgr/rtas_calls.c can fail for memory allocation errors, or unexpected workarea structure, after which point it simply stops calling rtas-configure-connector and ISOLATEs the device. If we don't track that, a subsequent unplug/plug could also result in stale FDT fragments being sent to the guest. So we'd the RTAS state tracking code to provide an interface of some sort for the DRC code to call into, which results in a lot of duplicated state-tracking. It's a bit unintuitive, but those FDT bits are closely coupled to the DRC state, so in the end I think that ends up being the most straight-forward place to manage them. > > -- > 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
On Thu, Feb 26, 2015 at 04:21:55PM -0600, Michael Roth wrote: > Quoting David Gibson (2015-02-24 18:48:23) > > On Tue, Feb 24, 2015 at 02:43:45PM -0600, Michael Roth wrote: > > > Quoting David Gibson (2015-02-24 00:40:32) > > > > On Mon, Feb 16, 2015 at 08:27:43AM -0600, Michael Roth wrote: [snip] > > > > > + 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. > > > > Hmm. You should still be able to handle that with just 2 hooks and 1 > > bit of state right? Say "start_configuration" and "end_configuration" > > or similar. start_configuration gets the blob from the backend, then > > end_configuration is called once RTAS has finished streaming it out to > > the guest and updates to the cnofigured state. > > {start,end}_configuration callbacks would work for handling > the normal 'configured' state transitions induced by the call, but we'd also > need hooks in the "other direction" for a couple cases: > > This scenario for instance: > > qemu: guest: > add device0 to drc0 > drmgr0: rtas-configure-connector drc0 > drc0->start_configuration... > drmgr0: rtas-configure-connector drc0 > drmgr0: rtas-configure-connector drc0 > ... > drmgr0: rtas-configure-connector drc0 > del device0 <device0 removal pending> > drmgr0: rtas-configure-connector drc0 > system_reset > device0 removed by drc0 > reset hook > add device1 to drc0 drmgr0: rtas-configure-connector drc0 > <begins fetching stale FDT> > > So I think we'd need at least a reset hook wired up to the RTAS state. > > It's also possible for the guest to force a transition out of the > configured state by ISOLATE'ing the device. This can happen in the > middle of the guests configure-connector calls if there's an error. If > rtas-configure-connector is the one generating the error, it can anticipate > this and automatically reset the state, but in some cases the error is > guest internal: the get_node() in src/drmgr/rtas_calls.c can fail for > memory allocation errors, or unexpected workarea structure, after which > point it simply stops calling rtas-configure-connector and ISOLATEs the > device. If we don't track that, a subsequent unplug/plug could also > result in stale FDT fragments being sent to the guest. > > So we'd the RTAS state tracking code to provide an interface of some > sort for the DRC code to call into, which results in a lot of > duplicated state-tracking. > > It's a bit unintuitive, but those FDT bits are closely coupled to the DRC > state, so in the end I think that ends up being the most straight-forward > place to manage them. Hrm. I'm still not following. I mean, I see that the back-end needs to be aware of the three-valued state: CC-not-called / CC-calls-in-progress / CC-calls-completed, but I'm not seeing why it needs to be aware of the individual CC calls. Could we have instead a set_state() callback of some sort, that handles both some of the existing architected states like ISOLATED, and also special states like CC-in-progress?
Quoting David Gibson (2015-02-26 23:31:20) > On Thu, Feb 26, 2015 at 04:21:55PM -0600, Michael Roth wrote: > > Quoting David Gibson (2015-02-24 18:48:23) > > > On Tue, Feb 24, 2015 at 02:43:45PM -0600, Michael Roth wrote: > > > > Quoting David Gibson (2015-02-24 00:40:32) > > > > > On Mon, Feb 16, 2015 at 08:27:43AM -0600, Michael Roth wrote: > [snip] > > > > > > + 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. > > > > > > Hmm. You should still be able to handle that with just 2 hooks and 1 > > > bit of state right? Say "start_configuration" and "end_configuration" > > > or similar. start_configuration gets the blob from the backend, then > > > end_configuration is called once RTAS has finished streaming it out to > > > the guest and updates to the cnofigured state. > > > > {start,end}_configuration callbacks would work for handling > > the normal 'configured' state transitions induced by the call, but we'd also > > need hooks in the "other direction" for a couple cases: > > > > This scenario for instance: > > > > qemu: guest: > > add device0 to drc0 > > drmgr0: rtas-configure-connector drc0 > > drc0->start_configuration... > > drmgr0: rtas-configure-connector drc0 > > drmgr0: rtas-configure-connector drc0 > > ... > > drmgr0: rtas-configure-connector drc0 > > del device0 <device0 removal pending> > > drmgr0: rtas-configure-connector drc0 > > system_reset > > device0 removed by drc0 > > reset hook > > add device1 to drc0 drmgr0: rtas-configure-connector drc0 > > <begins fetching stale FDT> > > > > So I think we'd need at least a reset hook wired up to the RTAS state. > > > > It's also possible for the guest to force a transition out of the > > configured state by ISOLATE'ing the device. This can happen in the > > middle of the guests configure-connector calls if there's an error. If > > rtas-configure-connector is the one generating the error, it can anticipate > > this and automatically reset the state, but in some cases the error is > > guest internal: the get_node() in src/drmgr/rtas_calls.c can fail for > > memory allocation errors, or unexpected workarea structure, after which > > point it simply stops calling rtas-configure-connector and ISOLATEs the > > device. If we don't track that, a subsequent unplug/plug could also > > result in stale FDT fragments being sent to the guest. > > > > So we'd the RTAS state tracking code to provide an interface of some > > sort for the DRC code to call into, which results in a lot of > > duplicated state-tracking. > > > > It's a bit unintuitive, but those FDT bits are closely coupled to the DRC > > state, so in the end I think that ends up being the most straight-forward > > place to manage them. > > Hrm. I'm still not following. I mean, I see that the back-end needs > to be aware of the three-valued state: CC-not-called / > CC-calls-in-progress / CC-calls-completed, but I'm not seeing why it > needs to be aware of the individual CC calls. It's not so much that the backend needs to be aware of the individual CC calls, but that the front-end/RTAS needs to be aware of the transitions made by the back-end (as with the example scenarios above), which can affect the output of those individual CC calls at any point in time. But maybe I'm misunderstanding your original suggestion. Is this more to do with the awkward drc->connector_connector interface, rather than the fact that sPAPRDRConnector manages the FDT/offsets. Because if so... > > Could we have instead a set_state() callback of some sort, that > handles both some of the existing architected states like ISOLATED, > and also special states like CC-in-progress? I suppose we could leave the fdt/fdt_offset/configured values in the possession of sPAPRDRConnector, but provide a drc->get_cc_state(), which exposes the following to rtas_ibm_configure_connector(): typedef struct sPAPRDRCCState { /* public */ void *fdt; int fdt_offset; int fdt_depth; /* private (or just move these up into sPAPRDRConnector) */ int fdt_start_offset; bool configured; } sPAPRDRCCState; If we do that, we can drop the FDT traversal stuff from sPAPRDRConnector, but still allow it to reset fdt/fdt_offset/configured/etc when it needs to based on it's internal state transitions. And along with that provide a drc->set_configured(void) that rtas_ibm_configure_connector() can call when it successfully completes the traversal. Is that the sort of thing you had in mind? > > -- > 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
On Fri, Feb 27, 2015 at 11:37:56AM -0600, Michael Roth wrote: > Quoting David Gibson (2015-02-26 23:31:20) > > On Thu, Feb 26, 2015 at 04:21:55PM -0600, Michael Roth wrote: > > > Quoting David Gibson (2015-02-24 18:48:23) > > > > On Tue, Feb 24, 2015 at 02:43:45PM -0600, Michael Roth wrote: > > > > > Quoting David Gibson (2015-02-24 00:40:32) > > > > > > On Mon, Feb 16, 2015 at 08:27:43AM -0600, Michael Roth wrote: > > [snip] > > > > > > > + 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. > > > > > > > > Hmm. You should still be able to handle that with just 2 hooks and 1 > > > > bit of state right? Say "start_configuration" and "end_configuration" > > > > or similar. start_configuration gets the blob from the backend, then > > > > end_configuration is called once RTAS has finished streaming it out to > > > > the guest and updates to the cnofigured state. > > > > > > {start,end}_configuration callbacks would work for handling > > > the normal 'configured' state transitions induced by the call, but we'd also > > > need hooks in the "other direction" for a couple cases: > > > > > > This scenario for instance: > > > > > > qemu: guest: > > > add device0 to drc0 > > > drmgr0: rtas-configure-connector drc0 > > > drc0->start_configuration... > > > drmgr0: rtas-configure-connector drc0 > > > drmgr0: rtas-configure-connector drc0 > > > ... > > > drmgr0: rtas-configure-connector drc0 > > > del device0 <device0 removal pending> > > > drmgr0: rtas-configure-connector drc0 > > > system_reset > > > device0 removed by drc0 > > > reset hook > > > add device1 to drc0 drmgr0: rtas-configure-connector drc0 > > > <begins fetching stale FDT> > > > > > > So I think we'd need at least a reset hook wired up to the RTAS state. > > > > > > It's also possible for the guest to force a transition out of the > > > configured state by ISOLATE'ing the device. This can happen in the > > > middle of the guests configure-connector calls if there's an error. If > > > rtas-configure-connector is the one generating the error, it can anticipate > > > this and automatically reset the state, but in some cases the error is > > > guest internal: the get_node() in src/drmgr/rtas_calls.c can fail for > > > memory allocation errors, or unexpected workarea structure, after which > > > point it simply stops calling rtas-configure-connector and ISOLATEs the > > > device. If we don't track that, a subsequent unplug/plug could also > > > result in stale FDT fragments being sent to the guest. > > > > > > So we'd the RTAS state tracking code to provide an interface of some > > > sort for the DRC code to call into, which results in a lot of > > > duplicated state-tracking. > > > > > > It's a bit unintuitive, but those FDT bits are closely coupled to the DRC > > > state, so in the end I think that ends up being the most straight-forward > > > place to manage them. > > > > Hrm. I'm still not following. I mean, I see that the back-end needs > > to be aware of the three-valued state: CC-not-called / > > CC-calls-in-progress / CC-calls-completed, but I'm not seeing why it > > needs to be aware of the individual CC calls. > > It's not so much that the backend needs to be aware of the individual > CC calls, but that the front-end/RTAS needs to be aware of the transitions > made by the back-end (as with the example scenarios above), which can > affect the output of those individual CC calls at any point in time. > > But maybe I'm misunderstanding your original suggestion. Is this more to > do with the awkward drc->connector_connector interface, rather than the > fact that sPAPRDRConnector manages the FDT/offsets. Because if so... So, the conceptual operation that needs to take place here is that the device model needs to convey a device tree fragment to the guest. If we were designing an interface to do that today, the obvious method is to just send a blob of fdt. One operation, nice and simple. But PAPR doesn't do that, because it predates fdt, instead it has this rather complex and awkward sequence of calls to iterate over the dt fragment. My basic point here is that we have to live with the PAPR interface - but we should try to restrict the ugliness to the smallest portion of the code we can and use a nicer interface internally wherever we can. So I'd like to restrict knowledge of the CC interface to just the actual RTAS implementation. The idea it gets a device tree fragment from the device model as fdt, then handles the streaming out via multiple RTAS calls. > > > > Could we have instead a set_state() callback of some sort, that > > handles both some of the existing architected states like ISOLATED, > > and also special states like CC-in-progress? > > I suppose we could leave the fdt/fdt_offset/configured values in the > possession of sPAPRDRConnector, but provide a drc->get_cc_state(), > which exposes the following to rtas_ibm_configure_connector(): > > typedef struct sPAPRDRCCState { > /* public */ > void *fdt; > int fdt_offset; > int fdt_depth; > > /* private (or just move these up into sPAPRDRConnector) */ > int fdt_start_offset; > bool configured; > } sPAPRDRCCState; > > If we do that, we can drop the FDT traversal stuff from sPAPRDRConnector, > but still allow it to reset fdt/fdt_offset/configured/etc when it needs > to based on it's internal state transitions. > > And along with that provide a drc->set_configured(void) that > rtas_ibm_configure_connector() can call when it successfully completes > the traversal. > > Is that the sort of thing you had in mind? Um.. I'm not entirely following what you're suggesting. What I'm intending is: RTAS code <-> DTC / device model code * Interface is just "give me the fdt" and possibly "set_state" * What the RTAS code gets from there is just a void * fdt, no offsets guest <-> RTAS code * This portion fdt_offset and fdt_depth. It's per-DRC, obviously, so it might still live in a DRC linked structure, but it's only ever touched by the RTAS code, nothing device backend specific. * Clearly, this section of code will need a reset hook so it can update its values. I don't know if that's made anything clearer.
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); + 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); + + 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)
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 <mdroth@linux.vnet.ibm.com> --- hw/ppc/spapr_rtas.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+)