On Mon, Mar 02, 2015 at 10:40:16PM -0600, Michael Roth wrote: > Quoting David Gibson (2015-03-02 01:02:46) > > On Thu, Feb 26, 2015 at 09:11:07PM -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 <mdr...@linux.vnet.ibm.com> > > > > > > So, actually, here's probably the best place to explain what I had in > > mind for changing the internal interface for this stuff. I was > > thinking something like this pseudocode: > > > > struct DRCCCState { > > void *fdt; > > int offset; > > int depth; > > }; > > > > rtas_configure_connector() > > { > > ... > > DRCCCState *ccstate; > > ... > > > > /* check parameters, retrieve drc */ > > ccstate = drc->ccstate; > > > > if (!ccstate) { > > /* Haven't started configuring yet */ > > ccstate = malloc(...); > > /* Retrieve the dt fragment from the backend */ > > ccstate->fdt = drck->get_dt(...); > > ccstate->offset = 0; > > } > > > > while (get next tag from fdt) { > > switch (tag) > > case FDT_PROPERTY: > > /* Translate property into rtas return values */ > > return SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY; > > > > /* other cases ... */ > > } > > > > /* Fall through only if we've completed streaming out the dt > > */ > > > > /* Tell the back end we've finished configuring */ > > drck->cc_completed(...); > > return SPAPR_DR_CC_RESPONSE_SUCCESS; > > } > > > > On reset, or anything else which interrupts the configuration process, > > just blow away drc->ccstate. > > Ok, that seems reasonable. I took a stab at it here: > > > https://github.com/mdroth/qemu/commit/79ce372743da1b63a6fa33e3de1f1daba8ea1fdc > https://github.com/mdroth/qemu/commits/spapr-hotplug-pci
It's looking pretty close now, thanks for the rework. > It exposes the ccstate as you suggested, via drck->get_cc_state(), and in > place of drck->cc_completed() I have drck->set_configured() which serves > roughly the same purpose I think. I opted not to let RTAS handle > allocation, since it seemed to imply RTAS owns it and not the DRC. So, that was intentional; basically RTAS *does* own the CCstate. But for convenience of index we need connect it to the DRC. Think of it like an rtas_priv field in the DRC. In particular I think the CCstate should be opaque to everything except the RTAS code itself, which means initializing the offset and depth in RTAS, not in a drck callback. As far as the drck callback is concerned, it's supplying a dt fragment, but it doesn't care about the details of how the upper layer communicates that through to the guest. -- 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
pgpD0hdppm_rR.pgp
Description: PGP signature