From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57886) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aiA1g-0000QD-If for qemu-devel@nongnu.org; Mon, 21 Mar 2016 20:18:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aiA1f-0000LV-G2 for qemu-devel@nongnu.org; Mon, 21 Mar 2016 20:18:24 -0400 Date: Tue, 22 Mar 2016 11:19:28 +1100 From: David Gibson Message-ID: <20160322001928.GP23586@voom.redhat.com> References: <1458568928-3055-1-git-send-email-clg@fr.ibm.com> <1458568928-3055-3-git-send-email-clg@fr.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="D3I0HgOdJ5+6n+7I" Content-Disposition: inline In-Reply-To: <1458568928-3055-3-git-send-email-clg@fr.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH 2/2] target-ppc: fix interrupt vectors address migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?C=E9dric?= Le Goater Cc: Mark Cave-Ayland , Thomas Huth , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Alexander Graf --D3I0HgOdJ5+6n+7I Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 21, 2016 at 03:02:08PM +0100, C=E9dric Le Goater wrote: > commit 2360b6e84f78 ("target-ppc: force update of msr bits in > cpu_post_load") introduced a change to restore env->excp_prefix of a > guest which could have altered its MSR_EP. To do this, cpu_post_load() > invalidates msr and then calls ppc_store_msr() with the expected value > in argument. >=20 > The problem is that ppc_store_msr() relies on a 'valid' current msr > before changing its value. The MSR_HVB and MSR_TGPR bits are excluded > from the msr reset to keep the checks valid but the MSR_IR, MSR_DR, > MSR_EP bits which are also used through the msr_{ir,dr,ep} macros, are > reseted. >=20 > This is an issue for CPUs not using MSR_EP, on the spapr platform for > instance but all book3s are impacted. If excp_prefix is restored to > some value, it will be reseted by this call, causing an ISEG exception > on spapr guests. >=20 > This patch proposal is to test the msr_mask before actually testing > the MSR_EP bit and protect excp_prefix. >=20 > Signed-off-by: C=E9dric Le Goater > --- >=20 > Should we just move the test in cpu_post_load() and not reset MSR_EP > if it is not present in msr_mask ? like this is done for MSR_HVB and > MSR_TGPR. I think this is making assumptions on what ppc_store_msr() > is up to though. >=20 > Maybe we could add a POWERPC_FLAGS_ for this purpose ? or test the > excp_model ? > =20 > There is room for improvement in ppc_store_msr(). It might need a new > helper like ppc_restore_msr() ? >=20 > Suggestions welcomed. So, IIUC, in spapr MSR[EP] can't be set with mtmsr or rfid, but can be set with H_SET_MODE? I think what we need to do is to make sure the full MSR value is migrated then, even if EP is not in the msr_mask. Once that's done, we should be able to correctly calculate excp_prefix from the MSR value after migration. Or am I missing something? > target-ppc/helper_regs.h | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) >=20 > diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h > index 271fddf17f0a..2a72e000ed83 100644 > --- a/target-ppc/helper_regs.h > +++ b/target-ppc/helper_regs.h > @@ -92,9 +92,11 @@ static inline int hreg_store_msr(CPUPPCState *env, tar= get_ulong value, > /* Swap temporary saved registers with GPRs */ > hreg_swap_gpr_tgpr(env); > } > - if (unlikely((value >> MSR_EP) & 1) !=3D msr_ep) { > - /* Change the exception prefix on PowerPC 601 */ > - env->excp_prefix =3D ((value >> MSR_EP) & 1) * 0xFFF00000; > + if ((env->msr_mask >> MSR_EP) & 1) { > + if (unlikely((value >> MSR_EP) & 1) !=3D msr_ep) { > + /* Change the exception prefix on PowerPC 601 */ > + env->excp_prefix =3D ((value >> MSR_EP) & 1) * 0xFFF00000; > + } > } > #endif > env->msr =3D value; --=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 --D3I0HgOdJ5+6n+7I Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW8I+QAAoJEGw4ysog2bOSJ2AQAM4g9rAvz2btEg68Dr9yauiR bIZR0Xo28V/gSW52PTppBryxAGgxY82cYhvBV8YLRDC9esr/oE0BT50Obi4ukj9B or8hKLViQKt9fUKGXIcZMhATHbtfRlht351rzAXevJR8MeYQWZWXM4C+J8qww8Zx ujXqUnDS2/f5NmfJP5Mv3ldqXCiAime1u9P9Hg6EUTbcTHRoMloCPghOObax2WjG nOP5dRUNCpMNkB1mzwu2oP26a5U07G4QK+901jZL/N105tiJTak8bo9DVKrPxDjF W6pR+rnVMj4eKLH/4CdsLRNykgvGFcewFqdocm7NQs3XwUxpamG8rpJYIx2eFiAd e7XTsSQjJwGbRJqzgZLMrX4TsYG3oUx2jZvD2xpXBtlWjGhC1BEeIiKotyo1xQX8 u6S72cuvUuE7XzLJljukO1aTPUjx/F8vGwjc/ccTTt5CvMNaDK6KInZh4vN/PiNC rGfZM97q0Ud0sRU222IBrmST3tBmTDgPNXyzB98lddtMqxC17OoAvTTILM2IEIKu 78X16lAU0BLUpy14n9PW03tIdL2bBYx8YT9lWlIVBkygHlhmkz2hlQOcSXkfo7zS BZjbiJPFhVivNiQhTcX3sIJbdAmo0ag2LqvIljkW/oCoFTlzgMssoNMStAlCEmEG 22+3gSc+1t2ONleWTV/G =BWXG -----END PGP SIGNATURE----- --D3I0HgOdJ5+6n+7I--