From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42180) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDjvw-0001lX-TU for qemu-devel@nongnu.org; Thu, 16 Jun 2016 22:55:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bDjvs-0003Vv-Ke for qemu-devel@nongnu.org; Thu, 16 Jun 2016 22:54:59 -0400 Date: Fri, 17 Jun 2016 12:27:45 +1000 From: David Gibson Message-ID: <20160617022731.GA19581@voom.fritz.box> References: <1465795496-15071-1-git-send-email-clg@kaod.org> <1465795496-15071-2-git-send-email-clg@kaod.org> <20160616010702.GI28087@voom.fritz.box> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="+QahgC5+KEYLbs62" Content-Disposition: inline In-Reply-To: <20160616010702.GI28087@voom.fritz.box> Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 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, qemu-devel@nongnu.org --+QahgC5+KEYLbs62 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 16, 2016 at 11:07:02AM +1000, David Gibson wrote: > On Mon, Jun 13, 2016 at 07:24:47AM +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 > > Signed-off-by: Benjamin Herrenschmidt > > Reviewed-by: David Gibson >=20 > I've merged this patch to ppc-for-2.7. =2E.and now I've removed it again. It seems that this breaks Thomas' new test that OpenBIOS runs on the mac machine types. Not sure why, but we need to figure that out before I apply. > The reset of the series I'd like to see a respin for, even if it's > just to clean up the commit messages. >=20 > > --- > > target-ppc/excp_helper.c | 51 +++++++++++++++++++---------------------= -------- > > target-ppc/translate.c | 7 +++++++ > > 2 files changed, 27 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_ul= ong val) > > } > > } > > =20 > > -static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_u= long msr, > > - target_ulong msrm, int keep_msrh) > > +static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_u= long 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, targe= t_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 arg= 1, 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..a02ddf52bfe6 100644 > > --- a/target-ppc/translate.c > > +++ b/target-ppc/translate.c > > @@ -4087,6 +4087,13 @@ 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 --=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 --+QahgC5+KEYLbs62 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXY2AQAAoJEGw4ysog2bOSpggQAMy2R9uMTWSmx610xfahZ57p XkFkwxJg7aAdz0JL4edFkTqSAfCOCLeTJFK4aKZ6m/f9vC0sF0Q8tQZ5DDPYvLoh X2Qpovzkxa2ap1mrkF5E3lfPFzWxT8HCJtKZGaKM/pkJVowSQLopmcd1Gg6FleF+ 2U2Hl3HgxT4TQe7OBTtGc0ltj5WmcYGseyE3Li7u0/N8GjD0bY61YExiqrfQYBg5 9ANflkTEn5HJROhJmkzgmUtTNeq+lJ1adVIjcGK0lg5RTau+uEmg7NmRp1eGjkTw z3sNUmz4KFMMiloP//TsHlwCRt6ktatyMt98lAuzqpnWdmS3+4z6HMtQ+hivA6il zLhHM0OXaG+WznT1EBN06hDGDvPKUiOBTHw5fYHuR1CA4iXTN1rw+yozZOeF2wON sMrLcPhRnxOT6qKvKbklqVXerc02sVyHieZG5WVmBtBQEDRwJHv4lNRuzxBGFSLZ SYu5cOoG8p/xzyTWSwr3G3bbiMD/axmXcUjAqePk/hqlN964AmpPS+goxIWnGxgA 6g2NlkYkIkI9ovNPGhVvRm+GOMBxeEYiT9THKizTNBUoZLHK4Kve86zAPLJ9XsyF tx1mCbQElzKhQUek+wzrtT3TH25tP4Y4rP+kuQiEqEhMYOTEkXNZ+fERY8N5xIQs vOFR2gkVAWImNl4FPXWN =t1te -----END PGP SIGNATURE----- --+QahgC5+KEYLbs62--