From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44297) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDmsV-0001PN-8x for qemu-devel@nongnu.org; Fri, 17 Jun 2016 02:03:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bDmsQ-0001T2-0S for qemu-devel@nongnu.org; Fri, 17 Jun 2016 02:03:39 -0400 Received: from 7.mo53.mail-out.ovh.net ([46.105.61.78]:39587) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDmsP-0001Rn-Mg for qemu-devel@nongnu.org; Fri, 17 Jun 2016 02:03:33 -0400 Received: from player158.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo53.mail-out.ovh.net (Postfix) with ESMTP id B38BCFF91BE for ; Fri, 17 Jun 2016 08:03:32 +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> <57639095.5010305@kaod.org> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: <576392B1.6030204@kaod.org> Date: Fri, 17 Jun 2016 08:03:29 +0200 MIME-Version: 1.0 In-Reply-To: <57639095.5010305@kaod.org> 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 07:54 AM, C=E9dric Le Goater wrote: > 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. >> >> ..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. >=20 > Just this patch ? I booted a macosx image with it. but maybe just a mac= 99. > I will check today. With your branch ppc-for-2.7 (at commit aba2e6258d86) + the=20 "ppc: Fix rfi/rfid/hrfi/... emulation" patch, these guests :=20 qemu-system-ppc -cdrom ./darwinppc-602.cdr -boot d=20 qemu-system-ppc -M mac99 -cdrom ./darwinppc-602.cdr -boot d=20 reach the installer macosx installer. C. >>> 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, targe= t_ulong msr, >>>> - target_ulong msrm, int keep_msrh) >>>> +static inline void do_rfi(CPUPPCState *env, target_ulong nip, targe= t_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, ta= rget_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] & 0xffffffff= ul); >>>> } >>>> =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 fil= ter >>>> + * 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_MCSR= R1], >>>> - ~((target_ulong)0), 0); >>>> + do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSR= R1]); >>>> } >>>> #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..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