From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58278) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YSg0A-0000cm-QH for qemu-devel@nongnu.org; Tue, 03 Mar 2015 01:08:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YSfzz-00027I-Jf for qemu-devel@nongnu.org; Tue, 03 Mar 2015 01:08:18 -0500 Date: Tue, 3 Mar 2015 16:33:39 +1100 From: David Gibson Message-ID: <20150303053339.GN29409@voom.fritz.box> References: <1425006675-19976-1-git-send-email-mdroth@linux.vnet.ibm.com> <1425006675-19976-8-git-send-email-mdroth@linux.vnet.ibm.com> <20150302070246.GH29409@voom.fritz.box> <20150303044016.27171.3218@loki> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="i6vqABX3nJKXLk01" Content-Disposition: inline In-Reply-To: <20150303044016.27171.3218@loki> Subject: Re: [Qemu-devel] [PATCH v6 07/15] spapr_rtas: add ibm, configure-connector RTAS interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: aik@ozlabs.ru, qemu-devel@nongnu.org, agraf@suse.de, ncmike@ncultra.org, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, bharata.rao@gmail.com, nfont@linux.vnet.ibm.com --i6vqABX3nJKXLk01 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 describe= s a > > > newly-attached device to guest. It is called multiple times to walk t= he > > > device-tree node and fetch individual properties into a 'workarea'/bu= ffer > > > provided by the guest. > > >=20 > > > The device-tree is generated by QEMU and passed to an sPAPRDRConnecto= r during > > > the initial hotplug operation, and the state of these RTAS calls is t= racked by > > > the sPAPRDRConnector. When the last of these properties is successful= ly > > > fetched, we report as special return value to the guest and transition > > > the device to a 'configured' state on the QEMU/DRC side. > > >=20 > > > See docs/specs/ppc-spapr-hotplug.txt for a complete description of > > > this interface. > > >=20 > > > Signed-off-by: Michael Roth > >=20 > >=20 > > 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: > >=20 > > struct DRCCCState { > > void *fdt; > > int offset; > > int depth; > > }; > >=20 > > rtas_configure_connector() > > { > > ... > > DRCCCState *ccstate; > > ... > >=20 > > /* check parameters, retrieve drc */ > > ccstate =3D drc->ccstate; > >=20 > > if (!ccstate) { > > /* Haven't started configuring yet */ > > ccstate =3D malloc(...); > > /* Retrieve the dt fragment from the backend */ > > ccstate->fdt =3D drck->get_dt(...); > > ccstate->offset =3D 0; > > } > >=20 > > while (get next tag from fdt) { > > switch (tag) > > case FDT_PROPERTY: > > /* Translate property into rtas return values */ > > return SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY; > >=20 > > /* other cases ... */ > > } > > =20 > > /* Fall through only if we've completed streaming out the dt > > */ > >=20 > > /* Tell the back end we've finished configuring */ > > drck->cc_completed(...); > > return SPAPR_DR_CC_RESPONSE_SUCCESS; > > } > >=20 > > On reset, or anything else which interrupts the configuration process, > > just blow away drc->ccstate. >=20 > Ok, that seems reasonable. I took a stab at it here: >=20 > https://github.com/mdroth/qemu/commit/79ce372743da1b63a6fa33e3de1f1da= ba8ea1fdc > 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. --=20 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 --i6vqABX3nJKXLk01 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJU9UeyAAoJEGw4ysog2bOSpxkQANmKGtb30xN3MZRAMpF/SUS5 uCo0OCpePC/YPgw3zVLCWf5vAJr5JVlBUmIjOTjVh5mnDaZPJfSLynURynoB/lxX rnSrkngjruzL9jqJMtkjN2IYZ9xwXyDA+jaDbqWNWlpmm73P0FluLG0OFPzKm5nQ ZkXd8PIEOetKCOXQCE8ABnNpbKbQzpD/d9XfwbXPkdxRngOwPFqwju/8SDmDD83q v2zfHtvODQoa5f7la8VRWvD/bmQsVFR8dBTJYpBL/78pj9ix8C1IJTTYdPU0cqKz 0UJEDsrdv4jj4mHX1KMxCGPil6vqZkAEmYxwZYPu0AieyLWjxM6XLdsgRjXI9nfU IT4XBFzW7xHjRw5xgU8rFMLb3ykg4mE7OGmrpff7ocI/B9/PZ/bA8lUos2tpnaSH 4gs1MgkL1GFgCDCqm3zBXwEwX83OFMKg9HJD0cx/MlLvjZGnFTvPXx/Kg7XaoApJ O5JLfji4OzJoVl6k7pErYYwcjHThyqzsLd2qJLbNZ1xgPUbOTT6iqLbrV8k1V7Rr aA80q7IBGPmNlJCM1npYHV5M9aqB5cW0Kw5rj/ecomg7Y61GyEuxkb6TzB1zBEaI 0XZzEaKUr+twR1x1gdVggvj2CztR+jYJfudUeV1VqRaCLlGtTgB5/1eUk6FHpX8o vX1uhj+6HLwSO/g791Mq =QfrW -----END PGP SIGNATURE----- --i6vqABX3nJKXLk01--