From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57257) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZZZQc-0006Z8-5P for qemu-devel@nongnu.org; Wed, 09 Sep 2015 03:04:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZZZQb-0000UB-2a for qemu-devel@nongnu.org; Wed, 09 Sep 2015 03:04:22 -0400 Date: Wed, 9 Sep 2015 16:05:33 +1000 From: David Gibson Message-ID: <20150909060533.GC17641@voom.redhat.com> References: <1441606024-1238-1-git-send-email-bharata@linux.vnet.ibm.com> <20150908012250.GG6537@voom.redhat.com> <20150908210356.10296.36458@loki> <20150908220325.10296.24993@loki> <20150909040232.GB17433@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="WplhKdTI2c8ulnbP" Content-Disposition: inline In-Reply-To: <20150909040232.GB17433@in.ibm.com> Subject: Re: [Qemu-devel] [FIX PATCH] spapr_drc: Return correct state for logical DR in entity_sense() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: qemu-ppc@nongnu.org, Michael Roth , qemu-devel@nongnu.org --WplhKdTI2c8ulnbP Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Sep 09, 2015 at 09:32:32AM +0530, Bharata B Rao wrote: > On Tue, Sep 08, 2015 at 05:03:25PM -0500, Michael Roth wrote: > > Quoting Michael Roth (2015-09-08 16:03:56) > > > Quoting David Gibson (2015-09-07 20:22:50) > > > > On Mon, Sep 07, 2015 at 11:37:04AM +0530, Bharata B Rao wrote: > > > > > When drmgr is run in the guest to add a device for which device_a= dd > > > > > hasn't been issued in QEMU, configure-connector call fails. > > > > > When configure-connector call fails, the guest would release (*) > > > > > the previously acquired DRC by setting back the DRC isolation sta= te > > > > > to ISOLATED and allocation state to UNUSABLE. These calls will be= issued > > > > > only if get-sensor-state call returns PRESENT state. However curr= ently for > > > > > a logical DR, entity_sense() would unconditinally return UNUSABLE > > > > > state only. This prevents any subsequent hotplug of the device wi= th > > > > > that DRC. > > >=20 > > > This seems a little odd. I think we default to ALLOCATION_STATE_UNUSA= BLE > > > for logical DR, and it's up the guest to transition to USABLE, which > > > probably happens prior to the configure-connector calls. So I think t= he > > > net effect of this fix is that guest will see these unallocated/unatt= ached > > > resources the same way they would a resource that was actually attach= ed > > > via device_add, and all we're really doing is working around the > > > eventual configuration failure that that will lead to by pretending a > > > resource was actually there. > > >=20 > > > According to PAPR+ 2.7: > > >=20 > > > 13.7.3.1 Acquire Logical Resource from Resource Pool: > > >=20 > > > If the state is =E2=80=9Cunusable=E2=80=9D the OS issues set-indica= tor (allocation-state, usable) to attempt to allocate the re- > > > source. Similarly, if the state is =E2=80=9Cavailable for exchange= =E2=80=9D the OS issues set-indicator (allocation-state, ex- > > > change) to attempt to allocate the resource, and if the state is = =E2=80=9Cavailable for recovery=E2=80=9D the OS issues > > > set-indicator (allocation-state, recover) to attempt to allocate th= e resource. > > >=20 > > > and > > >=20 > > > 13.7 Logical Resource Dynamic Reconfiguration (LRDR): > > >=20 > > > The OS may use the get-sensor-state RTAS call with the dr-entity-se= nse token to deter- > > > mine if a given drc-index refers to a connector that is currently usa= ble for DR operations. If the connector is not > > > currently usable the return state is =E2=80=9CDR entity unusable=E2= =80=9D (2). A set-indicator (isolation state) RTAS call to an unusable > > > connector or (dr-indicator) to any logical resource connector results= in a =E2=80=9CNo such indicator implemented=E2=80=9D return sta- > > > tus. =20 > > >=20 > > > So I think maybe the proper fix is to make sure that > > > drc->set_indicator_state() fails with an error that indicates to RTAS= to > > > return NO_SENSOR (-3) for cases where we haven't attached a resource > > > to the DRC via device_add. > >=20 > > Patch incoming: > >=20 > > spapr_drc: don't allow 'empty' DRCs to be unisolated > >=20 > > applies to spapr-next but requires revert of this patch. >=20 > Yes, please revert this patch in favour of Michael's. Done. --=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 --WplhKdTI2c8ulnbP Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV78wtAAoJEGw4ysog2bOSuJ8P/A42v3Zsb7fGyvxpnAP+gIky YUug28Yn12mS7Cuim2WoOk2W59oD93IwDlLcz2dOHoI3EugnGw8ryd4QWtNUzBmZ rgnQtiXVAUWqcaSPsVPz5LpSB6ymUDljtjlw5zflfYsGwR6woqZtIqeifSPtXkkv H6+hjmV/W4hUCLJbgCGI9CY8Dy6OgV7O4/J0ZBYnatXCERmQs9L7o8NGkYVTJyk4 485IkjZ9iEyIoINlLpPtYXELRu2+Nxpc/j5ab80UzjoFR1+ef+ReFkXVFyZDDeoJ t2SzVUxpRBsTbvTFLJkz7xcI0y4mJ5PjPJ9B2gVw7UGr3K5cHqE9zzHDFeSY9elv E6OI37+eNpe6LAGr88+hN/AqQg2n+2n1RmHmj0PXDOsPwTbmCKQNqGsGWncdxw7R EEvaBVpUj6GuHrmdSlwns8EldJrv9d2BnCab4JuNsoeBZLeiXj+dFI5I+JXF7fHQ UN0Ozf1qf6zUtS9X7hvgFmoBVP6wZN4SJ2B+JFbe9PyvZinkdYKPNTCFCCzKC1z1 uePW+cxqxt1Y3w0UheNosv3ywwdP37lQWiwj1X/yNyvqjsNt1ZOY3eU7EzyVuoGX Pb3Ia3tskuBro14W85Z/UH8et+nMsslB/Tne1Cj8WvXf02aS24bzrfL8hO9IfCN8 FGBd+wYY5btcz4oYHS3l =KdeD -----END PGP SIGNATURE----- --WplhKdTI2c8ulnbP--