From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51499) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XolWV-0004SD-Rw for qemu-devel@nongnu.org; Wed, 12 Nov 2014 22:56:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XolWT-0001Im-4h for qemu-devel@nongnu.org; Wed, 12 Nov 2014 22:56:43 -0500 Date: Thu, 13 Nov 2014 14:57:52 +1100 From: David Gibson Message-ID: <20141113035752.GI7291@voom.fritz.box> References: <20141105071019.26196.93729.stgit@aravindap> <20141111032421.GH15270@voom.redhat.com> <5461B779.3050003@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="QDIl5R72YNOeCxaP" Content-Disposition: inline In-Reply-To: <5461B779.3050003@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v3 0/4] target-ppc: Add FWNMI support in qemu for powerKVM guests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aravinda Prasad Cc: aik@au1.ibm.com, benh@au1.ibm.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, paulus@samba.org --QDIl5R72YNOeCxaP Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 11, 2014 at 12:45:05PM +0530, Aravinda Prasad wrote: >=20 >=20 > On Tuesday 11 November 2014 08:54 AM, David Gibson wrote: > > On Wed, Nov 05, 2014 at 12:42:03PM +0530, Aravinda Prasad wrote: > >> This series of patches add support for fwnmi in powerKVM guests. > >> > >> Currently upon machine check exception, if the address in > >> error belongs to guest then KVM invokes guest's NMI interrupt > >> vector 0x200. > >> > >> This patch series adds functionality where the guest's 0x200 > >> interrupt vector is patched such that QEMU gets control. QEMU > >> then builds error log and reports the error to OS registered > >> machine check handlers through RTAS space. > >> > >> Apart from this, the patch series also takes care of synchronization > >> when multiple processors encounter machine check at or about the > >> same time. > >> > >> The patch set was tested by simulating a machine check error in > >> the guest. > >> > >> Changes in v3: > >> - Incorporated review comments > >> - Byte codes in patch 4/4 are now moved to > >> pc-bios/spapr-rtas/spapr-rtas.S as instructions. > >> - Defined the RTAS blob in-memory layout. > >> - FIX: save and restore cr register in the trampoline > >> > >> Changes in v2: > >> - Re-based to github.com/agraf/qemu.git branch: ppc-next > >> - Merged patches 4 and 5. > >> - Incorporated other review comments > >=20 > > So, this may not still be possible depending on whether the KVM side > > of this is already merged, but it occurs to me that there's a simpler > > way. >=20 > The KVM part is already merged. Commit ID: 74845bc Ok, that makes life harder, though I guess without the qemu code merged, no-one would be using yet, so it's not impossible to change still. > > Rather than mucking about with having to update the hypervisor on the > > RTAS location, they have qemu copy the code out of RTAS, patch it and > > copy it back into the vector, you could instead do this: >=20 > Though this is possible, I have coupe of comments below >=20 > >=20 > > 1. Make KVM instead of immediately delivering a 0x200 for a guest > > machine check, cause a special exit to qemu. > >=20 > > 2. Have the register-nmi RTAS call store the guest side MC handler > > address in the spapr structure, but perform no actual guest code > > patching. > >=20 > > 3. Allocate the error log buffer independently from the RTAS blob, > > so qemu always knows where it is. >=20 > As per PAPR, the error log buffer should be part of RTAS blob and the > guest kernel explicitly checks if error log is inside RTAS blob. > This requires qemu to know the updated RTAS location by the OS which is > handled in patch 2/4. Ugh, ok. That's a pretty stupid interface requirement, even by PAPR standards, but I guess we're stuck with it. > > 4. When qemu gets the MC exit condition, instead of going via a > > patched 0x200 vector, just directly set the guest register state and > > jump straight into the guest side MC handler. >=20 > PAPR mentions: >=20 > "R1=E2=80=937.3.14=E2=80=938: Once the OS has registered for NMI notifica= tion, the > platform firmware must intercept all System Reset Interrupts on all of > the OS=E2=80=99s processors." >=20 > So do we need to go via 0x200? I don't see why. The hypervisor is already intercepting system resets and machine checks because it's a hypervisor, and from the PAPR guest's point of view, all it cares about is that you enter its registered handler with the expected information available. I don't see that the guest cares whether you bounce via a vector in guest space or directly enter the guest supplied handler using hypervisor magic. Patching the guest's vector actually seems a pretty awful hack that would only be necessary to work around limitations in the virtualization capabilities which I don't think we have as of POWER8. Btw, isn't a "System Reset Interrupt" vector 0x100, not vector 0x200? --=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 --QDIl5R72YNOeCxaP Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUZCxAAAoJEGw4ysog2bOSJtUP/RsSbWQDmFhkxGLM82wgTpqa 80OngosmrjpGgaTUR2Y65xIlQ7xxLelAWKz00b3k9QmHez1clHBAy1ERZtnDdMaR ZN4WISkzn415AZt/AfHMlFW7MqNQt2kb0pg78A043j9e795r9RciLt51LPakXfTZ TTPb+tFN4k9X7Ftbo77jPNde1onqI0gLt31DAJTDrNb+H5tNc7IviQhCnuLXo87T Ed44iuVKp4vRP5cmO5lgZG7V/x3CYwn0rALuPoyyElmKuCQ00LqaxLFuD1jlx6P4 tkbAZ8tObcqmcy1UlaEWprJY1nOEaFErw0J+VaZ3tdgFVR8G0zwZ/K90+X/iOI6r V2a1ubps+90Ryia0bYOS4PhQY4mHA6pINgEg0v+Jx5NxeEGw40h6b2VQMO1mOt2s JIXk763zDWxEAzdY51ZFzPTozZxtBKI7QOFPkzPgO/ceHzGGWP4fgHoTOAltOeH1 K6CvW/QFs4yMhL8aXzZ/cSC7T7EYrkJSmK+G6m8acBcHEeVt8mZeICyxjSdDg50v zz+oLuBX26ixAWUNeZ4GQZlT+2+Mmn3KYo20Xy/+f56IoOrltFa0Yazijb79uCjf Xjrye1XoC2BaHCClrT6SreSWuPr1RQq5lMCIWGHFC+Wb5SrPIlxeRhze97xiPygg FhGMxsapxwdG/iSTqmRD =U3Gj -----END PGP SIGNATURE----- --QDIl5R72YNOeCxaP--