From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43067) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDmju-0005ry-BS for qemu-devel@nongnu.org; Fri, 17 Jun 2016 01:54:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bDmjo-0007JL-D8 for qemu-devel@nongnu.org; Fri, 17 Jun 2016 01:54:45 -0400 Received: from 4.mo53.mail-out.ovh.net ([188.165.36.167]:58826) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDmjo-0007J7-3w for qemu-devel@nongnu.org; Fri, 17 Jun 2016 01:54:40 -0400 Received: from player158.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo53.mail-out.ovh.net (Postfix) with ESMTP id 6CDDBFF92B7 for ; Fri, 17 Jun 2016 07:54:33 +0200 (CEST) References: <1465795496-15071-1-git-send-email-clg@kaod.org> <1465795496-15071-2-git-send-email-clg@kaod.org> <20160616010702.GI28087@voom.fritz.box> <20160617022731.GA19581@voom.fritz.box> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: <57639095.5010305@kaod.org> Date: Fri, 17 Jun 2016 07:54:29 +0200 MIME-Version: 1.0 In-Reply-To: <20160617022731.GA19581@voom.fritz.box> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable 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: David Gibson Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 06/17/2016 04:27 AM, David Gibson wrote: > 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 >>> >>> 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. >>> >>> This also fixes some problems when running 32-bit userspace under >>> a 64-bit kernel. >>> >>> Signed-off-by: Benjamin Herrenschmidt >>> Reviewed-by: David Gibson >> >> I've merged this patch to ppc-for-2.7. >=20 > ..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. Just this patch ? I booted a macosx image with it. but maybe just a mac99= . I will check today. C.=20 >> The reset of the series I'd like to see a respin for, even if it's >> just to clean up the commit messages. >> >>> --- >>> target-ppc/excp_helper.c | 51 +++++++++++++++++++-------------------= ---------- >>> target-ppc/translate.c | 7 +++++++ >>> 2 files changed, 27 insertions(+), 31 deletions(-) >>> >>> 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_= ulong val) >>> } >>> } >>> =20 >>> -static inline void do_rfi(CPUPPCState *env, target_ulong nip, target= _ulong msr, >>> - target_ulong msrm, int keep_msrh) >>> +static inline void do_rfi(CPUPPCState *env, target_ulong nip, target= _ulong 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, tar= get_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] & 0xffffffffu= l); >>> } >>> =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 filt= er >>> + * 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_MCSRR= 1], >>> - ~((target_ulong)0), 0); >>> + do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR= 1]); >>> } >>> #endif >>> =20 >>> @@ -1045,7 +1034,7 @@ void helper_td(CPUPPCState *env, target_ulong a= rg1, 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 >=20