From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48377) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9o0y-0002pk-Qa for qemu-devel@nongnu.org; Mon, 06 Jun 2016 02:27:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b9o0t-0005Nc-KU for qemu-devel@nongnu.org; Mon, 06 Jun 2016 02:27:56 -0400 Received: from 6.mo68.mail-out.ovh.net ([46.105.63.100]:40013) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9o0t-0005NN-A7 for qemu-devel@nongnu.org; Mon, 06 Jun 2016 02:27:51 -0400 Received: from player695.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo68.mail-out.ovh.net (Postfix) with ESMTP id 20C0F1001023 for ; Mon, 6 Jun 2016 08:27:50 +0200 (CEST) References: <1464955880-10176-1-git-send-email-clg@kaod.org> <57518BA1.1030400@ilande.co.uk> <5754645E.8080504@kaod.org> <5754A730.5040005@ilande.co.uk> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: <575517E2.3020105@kaod.org> Date: Mon, 6 Jun 2016 08:27:46 +0200 MIME-Version: 1.0 In-Reply-To: <5754A730.5040005@ilande.co.uk> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 0/3] ppc: complete the new HV mode List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark Cave-Ayland , David Gibson Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 06/06/2016 12:26 AM, Mark Cave-Ayland wrote: > On 05/06/16 18:41, C=C3=A9dric Le Goater wrote: >=20 >> Hello Mark, >> >> On 06/03/2016 03:52 PM, Mark Cave-Ayland wrote: >>> On 03/06/16 13:11, C=C3=A9dric Le Goater wrote: >>> >>>> This is follow up to complete the serie "ppc: preparing pnv landing >>>> (round 2)" plus a little fix on instruction privileges. >>>> >>>> Tested on a POWER8 pserie guest and on mac99. >>>> >>>> Benjamin Herrenschmidt (2): >>>> ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV >>>> ppc: Better figure out if processor has HV mode >>>> >>>> C=C3=A9dric Le Goater (1): >>>> ppc: fix hrfid, tlbia and slbia privilege >>>> >>>> target-ppc/cpu.h | 4 ++++ >>>> target-ppc/excp_helper.c | 8 ++++++-- >>>> target-ppc/helper_regs.h | 4 ++-- >>>> target-ppc/translate.c | 10 ++++++---- >>>> target-ppc/translate_init.c | 19 +++++++++++++++---- >>>> 5 files changed, 33 insertions(+), 12 deletions(-) >>> >>> Hi C=C3=A9dric, >>> >>> I can confirm that this patchset fixes starting up OpenBIOS for both >>> g3beige/mac99 in my tests here. With the escc fix also applied, the o= nly >>> outstanding issue is the removal of the tlb_flush() statements which >>> causes Darwin, MacOS X and HelenOS 0.60 to panic on boot >>> >>> My only request is if it would be possible to move patch 2 "ppc: Bett= er >>> figure out if processor has HV mode" to the front of this patchset wh= ich >>> will make the remaining patches bisectable for the Mac machines. With= that: >>> >>> Tested-by: Mark Cave-Ayland >>> >>> Does anyone know if Ben has any ideas as to why the MMU tlb_flush >>> changes patch is causing such problems? >> >> >> Here is a fix I think. Could you give it a try ?=20 >> >> Cheers, >> >> C.=20 >> >> From: C=C3=A9dric Le Goater >> Subject: [PATCH] ppc: fix tlb flushes for 32bit environment >> Date: Sun, 05 Jun 2016 18:46:19 +0200 >> MIME-Version: 1.0 >> Content-Type: text/plain; charset=3DUTF-8 >> Content-Transfer-Encoding: 8bit >> >> commit cd0c6f473532 ('ppc: Do some batching of TCG tlb flushes') >> introduced an optimisation to flush TLBs only when a context >> synchronizing event is reached (interrupt, rfi). This was done for >> ppc64 but 32bit was forgotten on the way. >> >> Tested on mac99 and g3beige with >> >> qemu-system-ppc -cdrom darwinppc-602.cdr -boot d >> >> Signed-off-by: C=C3=A9dric Le Goater >> --- >> >> I think the hunk in powerpc_excp() is needed if we don't generate a >> context synchronizing event. what is best to do ? >> >> target-ppc/cpu.h | 2 +- >> target-ppc/excp_helper.c | 10 ++++++++++ >> target-ppc/helper_regs.h | 9 ++++++++- >> target-ppc/translate.c | 2 +- >> 4 files changed, 20 insertions(+), 3 deletions(-) >> >> Index: qemu-dgibson-for-2.7.git/target-ppc/translate.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- qemu-dgibson-for-2.7.git.orig/target-ppc/translate.c >> +++ qemu-dgibson-for-2.7.git/target-ppc/translate.c >> @@ -3290,7 +3290,7 @@ static void gen_eieio(DisasContext *ctx) >> { >> } >> =20 >> -#if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64) >> +#if !defined(CONFIG_USER_ONLY) >> static inline void gen_check_tlb_flush(DisasContext *ctx) >> { >> TCGv_i32 t =3D tcg_temp_new_i32(); >> Index: qemu-dgibson-for-2.7.git/target-ppc/cpu.h >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- qemu-dgibson-for-2.7.git.orig/target-ppc/cpu.h >> +++ qemu-dgibson-for-2.7.git/target-ppc/cpu.h >> @@ -958,9 +958,9 @@ struct CPUPPCState { >> /* PowerPC 64 SLB area */ >> ppc_slb_t slb[MAX_SLB_ENTRIES]; >> int32_t slb_nr; >> +#endif >> /* tcg TLB needs flush (deferred slb inval instruction typically)= */ >> uint32_t tlb_need_flush; >> -#endif >> /* segment registers */ >> hwaddr htab_base; >> /* mask used to normalize hash value to PTEG index */ >> Index: qemu-dgibson-for-2.7.git/target-ppc/helper_regs.h >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- qemu-dgibson-for-2.7.git.orig/target-ppc/helper_regs.h >> +++ qemu-dgibson-for-2.7.git/target-ppc/helper_regs.h >> @@ -121,6 +121,13 @@ static inline int hreg_store_msr(CPUPPCS >> } >> if (((value >> MSR_IR) & 1) !=3D msr_ir || >> ((value >> MSR_DR) & 1) !=3D msr_dr) { >> + /* A change of the instruction relocation bit in the MSR can >> + * cause an implicit branch in the address space. This >> + * requires a tlb flush. >> + */ >> + if (env->mmu_model & POWERPC_MMU_32B) { >> + env->tlb_need_flush =3D 1; >> + } >> cs->interrupt_request |=3D CPU_INTERRUPT_EXITTB; >> } >> if ((env->mmu_model & POWERPC_MMU_BOOKE) && >> @@ -151,7 +158,7 @@ static inline int hreg_store_msr(CPUPPCS >> return excp; >> } >> =20 >> -#if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64) >> +#if !defined(CONFIG_USER_ONLY) >> static inline void check_tlb_flush(CPUPPCState *env) >> { >> CPUState *cs =3D CPU(ppc_env_get_cpu(env)); >> Index: qemu-dgibson-for-2.7.git/target-ppc/excp_helper.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- qemu-dgibson-for-2.7.git.orig/target-ppc/excp_helper.c >> +++ qemu-dgibson-for-2.7.git/target-ppc/excp_helper.c >> @@ -709,6 +709,16 @@ static inline void powerpc_excp(PowerPCC >> } >> } >> #endif >> + if (((new_msr >> MSR_IR) & 1) !=3D msr_ir || >> + ((new_msr >> MSR_DR) & 1) !=3D msr_dr) { >> + /* A change of the instruction relocation bit in the MSR can >> + * cause an implicit branch in the address space. This >> + * requires a tlb flush. >> + */ >> + if (env->mmu_model & POWERPC_MMU_32B) { >> + env->tlb_need_flush =3D 1; >> + } >> + } >> /* We don't use hreg_store_msr here as already have treated >> * any special case that could occur. Just store MSR and update h= flags >> * >> >> >=20 > Hi C=C3=A9dric, >=20 > I've just tried this patch on a selection of images here with both > g3beige and mac99 and as far as I can tell the crash has now gone away: >=20 > Tested-by: Mark Cave-Ayland Super. I still need to sort out the exit path of hreg_store_msr().=20 If we have a change in IR/DR, this is a case to transform mtmsr in a=20 context-synchronize instruction, for a tlb flush. This is problably=20 the reason behind the POWERPC_EXCP_NONE I believe, which is then=20 handled in powerpc_excp(). I need to look at that path closer. And, now that I have a darwin guest, I have a few questions :=20 1. How do I get X running ?=20 2. I have an old ibook G4 from which I dd'ed the disk. openbios=20 complains for some invalid state. is that supported ? Thanks, C. > Thanks for getting this sorted so quickly. >=20 >=20 > ATB, >=20 > Mark. >=20