From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36653) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cCHJw-0003QH-NF for qemu-devel@nongnu.org; Wed, 30 Nov 2016 21:42:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cCHJt-0005lX-G8 for qemu-devel@nongnu.org; Wed, 30 Nov 2016 21:42:00 -0500 Date: Thu, 1 Dec 2016 13:41:32 +1100 From: David Gibson Message-ID: <20161201024132.GJ19891@umbus> References: <1480547134-18590-1-git-send-email-mdroth@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="XigHxYirkHk2Kxsx" Content-Disposition: inline In-Reply-To: <1480547134-18590-1-git-send-email-mdroth@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH for-2.8] spapr: fix default DRC state for coldplugged LMBs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Bharata B Rao , Greg Kurz --XigHxYirkHk2Kxsx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 30, 2016 at 05:05:34PM -0600, Michael Roth wrote: > Currently we set the initial isolation/allocation state for DRCs > associated with coldplugged LMBs to ISOLATED/UNUSABLE, > respectively, under the assumption that the guest will move this > state to UNISOLATED/USABLE. >=20 > In fact, this is only the case for LMBs added via hotplug. For > coldplugged LMBs, the guest actually assumes the initial state to > be UNISOLATED/USABLE. >=20 > In practice, this only becomes an issue when we attempt to unplug > one of these LMBs, where the guest kernel will issue an > rtas-get-sensor-state call to check that the corresponding DRC is > in an USABLE state before it will release the LMB back to > QEMU. If the returned state is otherwise, the guest will assume no > further action is needed, which bypasses the QEMU-side cleanup that > occurs during the USABLE->UNUSABLE transition. This results in > LMBs and their corresponding pc-dimm devices to stick around > indefinitely. >=20 > This patch fixes the issue by manually setting DRCs associated with > cold-plugged LMBs to UNISOLATED/ALLOCATED, but leaving the hotplug > state untouched. As it turns out, this is analogous to the handling > for cold-plugged CPUs in spapr_core_plug(). >=20 > Cc: qemu-ppc@nongnu.org > Cc: David Gibson > Cc: Bharata B Rao > Cc: Greg Kurz > Signed-off-by: Michael Roth > --- > This patch is a revision to the previously posted series: >=20 > [PATCH for-2.8 0/3] spapr: fix breakage of memory unplug after migration >=20 > That patchset introduced DRC migration as a means to address DRC state > synchronization between source/target to allow memory unplug after > migration. It turns out that unplug was also not working for LMBs > that were defined on the source via '-device pc-dimm'. The real issue > was that the default coldplug state on *both* the source and target was > wrong, and by fixing that default state, we no longer need DRC migration > for the scenario mentioned in the previous patchset since, incidentally, > coldplug state on the target now matches post-hotplug DRC state on the > source. >=20 > This patch is very late for 2.8, but I'm hoping it can still make it > in for rc3/2.8. >=20 > I've tested forward/backward migration for all possible combinations > (other than known case unsupported case 2.8->2.7) of > v2.6/v2.7/v2.8, pseries-2.6/pseries-2.7/pseries-2.8, and > modern-hotplug-events=3Dtrue/false, with various combinations of > CPU / LMB hotplug/unplug before/after migration. I've merged this to ppc-for-2.8. I'll begin my test cycle and see if we can send a pull request in time. > --- > hw/ppc/spapr.c | 5 +++++ > 1 file changed, 5 insertions(+) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index c3269c7..208ef7b 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -2290,6 +2290,11 @@ static void spapr_add_lmbs(DeviceState *dev, uint6= 4_t addr_start, uint64_t size, > drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp); > addr +=3D SPAPR_MEMORY_BLOCK_SIZE; > + if (!dev->hotplugged) { > + /* guests expect coldplugged LMBs to be pre-allocated */ > + drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_US= ABLE); > + drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_UNIS= OLATED); > + } > } > /* send hotplug notification to the > * guest only in case of hotplugged memory --=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 --XigHxYirkHk2Kxsx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYP43ZAAoJEGw4ysog2bOSmz0QAIUoDKEdzRtnl1TvLG73Xo+C 8ByQOKSjMEfHCKf6cqiWYTSPFHDtM7FDEH8YiDaH4pGqZB8BKoOTq9mfPXLdMqBy cpzebowtasAcKSodZUWvuU9jduOSelB1X99jrvZWlVdGVvTg9IiN5W7/wBFFD4Dl DXQ/CVzm7a6rhQovZLC2/ybUrR8h4a90ZXhVwyzUY9BfkoIU9FeZLeXP9y+x8uO2 QdaZVUoV5zIjhl4pZQJ+oA3i52Ys5UiXUdk6nYG5hh0L9W7gkQ3M7OxbFv4Hygw4 XTf3f82+Sxa71+hlXeaKAj0OPJK6pshRD9MMh/1MMDSXWCxsUDZq+D94Isebp4tT B7iK9FJMNEB6aaNomfQHDbLomanQAG1+HVDzNcEYsR1WeDOB4Du1vEnIzlEWsPJv /gjuXAgrU+ShEf201ZMfp041rv9hQ7EQMyKG3Ks3Ju6QL3oVaEifb7FDbzMVAXsy xFt/lCVQsWb0iBpxddyLVzv2zsp/3yt1TJnXGO+XNSUdFP60ZZS+Eb+cLle8GYLd btN8Oe/SQfBcKu8YqOsdtdA5EsGv8bFWbnk03HABwtgNZi+GxUVUwHdq/CicdE2s Fiy/J/pqTURkzwuNQbYTnOm+VXfNQIdO5l/ZBPhvg5v7zWjaP4zbpJ62n1zwW5vq 1Mh9MbULdrG+8vf/FJPu =Qm2H -----END PGP SIGNATURE----- --XigHxYirkHk2Kxsx--