From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48880) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9o3Z-00051W-N9 for qemu-devel@nongnu.org; Mon, 06 Jun 2016 02:30:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b9o3V-00069R-DN for qemu-devel@nongnu.org; Mon, 06 Jun 2016 02:30:37 -0400 References: <1464955880-10176-1-git-send-email-clg@kaod.org> <57518BA1.1030400@ilande.co.uk> <5754645E.8080504@kaod.org> <5754A730.5040005@ilande.co.uk> <575517E2.3020105@kaod.org> From: Cedric Le Goater Message-ID: <5755187E.6030606@kaod.org> Date: Mon, 6 Jun 2016 08:30:22 +0200 MIME-Version: 1.0 In-Reply-To: <575517E2.3020105@kaod.org> 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 08:27 AM, C=C3=A9dric Le Goater wrote: > On 06/06/2016 12:26 AM, Mark Cave-Ayland wrote: >> On 05/06/16 18:41, C=C3=A9dric Le Goater wrote: >> >>> 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 = only >>>> 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: Bet= ter >>>> figure out if processor has HV mode" to the front of this patchset w= hich >>>> will make the remaining patches bisectable for the Mac machines. Wit= h 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 = hflags >>> * >>> >>> >> >> Hi C=C3=A9dric, >> >> 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= : >> >> Tested-by: Mark Cave-Ayland >=20 > Super. I still need to sort out the exit path of hreg_store_msr().=20 >=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. Forget the above as Ben just passed by :) I still interested by what is below : > And, now that I have a darwin guest, I have a few questions :=20 >=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 ? >=20 > Thanks, >=20 > C. >=20 >=20 >=20 >> Thanks for getting this sorted so quickly. >> >> >> ATB, >> >> Mark. >> >=20