From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49835) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFYGi-0002fN-Cd for qemu-devel@nongnu.org; Tue, 21 Jun 2016 22:51:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bFYGf-0001N4-ND for qemu-devel@nongnu.org; Tue, 21 Jun 2016 22:51:55 -0400 Date: Wed, 22 Jun 2016 12:46:22 +1000 From: David Gibson Message-ID: <20160622024622.GK17957@voom.fritz.box> References: <1466545735-2555-1-git-send-email-clg@kaod.org> <1466545735-2555-2-git-send-email-clg@kaod.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="tKtFalx5NIx0HZ72" Content-Disposition: inline In-Reply-To: <1466545735-2555-2-git-send-email-clg@kaod.org> Subject: Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?C=E9dric?= Le Goater Cc: qemu-ppc@nongnu.org, Alexander Graf , qemu-devel@nongnu.org, Benjamin Herrenschmidt --tKtFalx5NIx0HZ72 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jun 21, 2016 at 11:48:46PM +0200, C=E9dric Le Goater wrote: > From: Benjamin Herrenschmidt >=20 > This reworks emulation of the various "rfi" variants. I removed > some masking bits that I couldn't make sense of, the only bit that > I am aware we should mask here is POW, the CPU's MSR mask should > take care of the rest. >=20 > This also fixes some problems when running 32-bit userspace under > a 64-bit kernel. >=20 > This patch broke 32bit OpenBIOS when run under a 970 cpu. A fix was > proposed here : >=20 > https://www.coreboot.org/pipermail/openbios/2016-June/009452.html >=20 > Signed-off-by: Benjamin Herrenschmidt > Reviewed-by: David Gibson > [clg: updated the commit log with the reference of the openbios fix ] > Signed-off-by: C=E9dric Le Goater I'm not comfortable merging this until the openbios change is pulled back into the qemu tree (submodule and pre-built binary). Again - sure you don't want to apply this with rfi still enabled for 64-bit for now, letting the rest of this series go in as well, then clean up the rfi/64 behaviour later? > --- >=20 > target-ppc/excp_helper.c | 51 +++++++++++++++++++-----------------------= ------ > target-ppc/translate.c | 8 ++++++++ > 2 files changed, 28 insertions(+), 31 deletions(-) >=20 > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > index 30e960e30b63..aa0b63f4b0de 100644 > --- a/target-ppc/excp_helper.c > +++ b/target-ppc/excp_helper.c > @@ -922,25 +922,20 @@ void helper_store_msr(CPUPPCState *env, target_ulon= g val) > } > } > =20 > -static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulo= ng msr, > - target_ulong msrm, int keep_msrh) > +static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulo= ng msr) > { > CPUState *cs =3D CPU(ppc_env_get_cpu(env)); > =20 > + /* MSR:POW cannot be set by any form of rfi */ > + msr &=3D ~(1ULL << MSR_POW); > + > #if defined(TARGET_PPC64) > - if (msr_is_64bit(env, msr)) { > - nip =3D (uint64_t)nip; > - msr &=3D (uint64_t)msrm; > - } else { > + /* Switching to 32-bit ? Crop the nip */ > + if (!msr_is_64bit(env, msr)) { > nip =3D (uint32_t)nip; > - msr =3D (uint32_t)(msr & msrm); > - if (keep_msrh) { > - msr |=3D env->msr & ~((uint64_t)0xFFFFFFFF); > - } > } > #else > nip =3D (uint32_t)nip; > - msr &=3D (uint32_t)msrm; > #endif > /* XXX: beware: this is false if VLE is supported */ > env->nip =3D nip & ~((target_ulong)0x00000003); > @@ -959,26 +954,24 @@ static inline void do_rfi(CPUPPCState *env, target_= ulong nip, target_ulong msr, > =20 > void helper_rfi(CPUPPCState *env) > { > - if (env->excp_model =3D=3D POWERPC_EXCP_BOOKE) { > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > - ~((target_ulong)0), 0); > - } else { > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > - ~((target_ulong)0x783F0000), 1); > - } > + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful); > } > =20 > +#define MSR_BOOK3S_MASK > #if defined(TARGET_PPC64) > void helper_rfid(CPUPPCState *env) > { > - do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1], > - ~((target_ulong)0x783F0000), 0); > + /* The architeture defines a number of rules for which bits > + * can change but in practice, we handle this in hreg_store_msr() > + * which will be called by do_rfi(), so there is no need to filter > + * here > + */ > + do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1]); > } > =20 > void helper_hrfid(CPUPPCState *env) > { > - do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1], > - ~((target_ulong)0x783F0000), 0); > + do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]); > } > #endif > =20 > @@ -986,28 +979,24 @@ void helper_hrfid(CPUPPCState *env) > /* Embedded PowerPC specific helpers */ > void helper_40x_rfci(CPUPPCState *env) > { > - do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3], > - ~((target_ulong)0xFFFF0000), 0); > + do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3]); > } > =20 > void helper_rfci(CPUPPCState *env) > { > - do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1], > - ~((target_ulong)0), 0); > + do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1]); > } > =20 > void helper_rfdi(CPUPPCState *env) > { > /* FIXME: choose CSRR1 or DSRR1 based on cpu type */ > - do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1], > - ~((target_ulong)0), 0); > + do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1]); > } > =20 > void helper_rfmci(CPUPPCState *env) > { > /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */ > - do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1], > - ~((target_ulong)0), 0); > + do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]); > } > #endif > =20 > @@ -1045,7 +1034,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1,= target_ulong arg2, > =20 > void helper_rfsvc(CPUPPCState *env) > { > - do_rfi(env, env->lr, env->ctr, 0x0000FFFF, 0); > + do_rfi(env, env->lr, env->ctr & 0x0000FFFF); > } > =20 > /* Embedded.Processor Control */ > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index b6894751e8df..81481955a589 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -4087,6 +4087,14 @@ static void gen_rfi(DisasContext *ctx) > #if defined(CONFIG_USER_ONLY) > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > #else > + /* This instruction doesn't exist anymore on 64-bit server > + * processors compliant with arch 2.x > + */ > + if (ctx->insns_flags & PPC_SEGMENT_64B) { > + gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL); > + return; > + } > + > /* Restore CPU state */ > if (unlikely(ctx->pr)) { > gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); --=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 --tKtFalx5NIx0HZ72 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXafv+AAoJEGw4ysog2bOSZe0QAMfEONFxAZgmslOwrA3XFowd H1MPz7yy4SoeBIbJL3QDOyTP3aP6moUq92eDerv0XhtlitLTylHQy0ozfbkeiMPv w3rymj6tDvyclt6blzchpRy8RhZUeIvW1750YjhKE11PA6MfG6E2KfhIbjKPv7bl muB0NAXMtngB1p/o3YeX+MgTIAlyX7MF3O5sfnvCrKbHcvWm8FDkIJK7XVl5DO5F z0rsuXHRPG6nOs0dKL93HTq55A9yyOGDPik9LcNctVwwxUHSff7OZObVtelegE9j nBPlTVatwBYdpD1SSSNdNHanL67vAqFWmrgFA7qrdSe4aesIzhVmzz35/RNZaZqW bBO1/7ISaVek+snP1T5ZPT7CftvjcFf1cFCBNX/7ea79wkdF+4wt2J8VP+bRyzgm RBFI4BA3mB9Gs4Uq4RS/hot5/bCWSReskLKD3eHkY9qXgmdjPCzqstvhCWJ7n095 UtCB44bqZ7XbuRW6uHXn2k5Crc0VjwRgmtycw4nb6R6nQs/cn1xbKo0pAOWCFhP/ 3ECaZeN6ty3S4jJVLlRzUAdv8YqZxHVsM7MRZCFNOuh28YyH61cSf49zaqRdb2Uk ktmKr4w50scmw12WBR5RIfzkXdgW9FPKNIxIcyVEb5skR1G2kPLyEbyIfE141TCV rXzZa2jNq3T0wGCf3Hqv =kWYI -----END PGP SIGNATURE----- --tKtFalx5NIx0HZ72--