From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43565) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8J7U-0001lj-If for qemu-devel@nongnu.org; Wed, 01 Jun 2016 23:16:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8J7Q-0003P7-NQ for qemu-devel@nongnu.org; Wed, 01 Jun 2016 23:16:28 -0400 Date: Thu, 2 Jun 2016 13:15:09 +1000 From: David Gibson Message-ID: <20160602031509.GI15455@voom.fritz.box> References: <1464655277-14748-1-git-send-email-david@gibson.dropbear.id.au> <1464655277-14748-3-git-send-email-david@gibson.dropbear.id.au> <574F388A.3020201@ilande.co.uk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="fmvA4kSBHQVZhkR6" Content-Disposition: inline In-Reply-To: <574F388A.3020201@ilande.co.uk> Subject: Re: [Qemu-devel] [Qemu-ppc] [PULL 02/12] ppc: Use split I/D mmu modes to avoid flushes on interrupts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark Cave-Ayland Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, bharata.rao@gmail.com, pbonzini@redhat.com --fmvA4kSBHQVZhkR6 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 01, 2016 at 08:33:30PM +0100, Mark Cave-Ayland wrote: > On 31/05/16 01:41, David Gibson wrote: >=20 > > From: Benjamin Herrenschmidt > >=20 > > We rework the way the MMU indices are calculated, providing separate > > indices for I and D side based on MSR:IR and MSR:DR respectively, > > and thus no longer need to flush the TLB on context changes. This also > > adds correct support for HV as a separate address space. > >=20 > > Signed-off-by: Benjamin Herrenschmidt > > Signed-off-by: David Gibson > > --- > > target-ppc/cpu.h | 11 +++++++--- > > target-ppc/excp_helper.c | 11 ---------- > > target-ppc/helper_regs.h | 54 ++++++++++++++++++++++++++++++++++++++++= +------- > > target-ppc/machine.c | 5 ++++- > > target-ppc/translate.c | 7 ++++--- > > 5 files changed, 63 insertions(+), 25 deletions(-) > >=20 > > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h > > index 02e71ea..2c8c8c0 100644 > > --- a/target-ppc/cpu.h > > +++ b/target-ppc/cpu.h > > @@ -359,6 +359,8 @@ struct ppc_slb_t { > > #define MSR_EP 6 /* Exception prefix on 601 = */ > > #define MSR_IR 5 /* Instruction relocate = */ > > #define MSR_DR 4 /* Data relocate = */ > > +#define MSR_IS 5 /* Instruction address space (BookE) = */ > > +#define MSR_DS 4 /* Data address space (BookE) = */ > > #define MSR_PE 3 /* Protection enable on 403 = */ > > #define MSR_PX 2 /* Protection exclusive on 403 x = */ > > #define MSR_PMM 2 /* Performance monitor mark on POWER x = */ > > @@ -410,6 +412,8 @@ struct ppc_slb_t { > > #define msr_ep ((env->msr >> MSR_EP) & 1) > > #define msr_ir ((env->msr >> MSR_IR) & 1) > > #define msr_dr ((env->msr >> MSR_DR) & 1) > > +#define msr_is ((env->msr >> MSR_IS) & 1) > > +#define msr_ds ((env->msr >> MSR_DS) & 1) > > #define msr_pe ((env->msr >> MSR_PE) & 1) > > #define msr_px ((env->msr >> MSR_PX) & 1) > > #define msr_pmm ((env->msr >> MSR_PMM) & 1) > > @@ -889,7 +893,7 @@ struct ppc_segment_page_sizes { > > =20 > > /*********************************************************************= ********/ > > /* The whole PowerPC CPU context */ > > -#define NB_MMU_MODES 3 > > +#define NB_MMU_MODES 8 > > =20 > > #define PPC_CPU_OPCODES_LEN 0x40 > > #define PPC_CPU_INDIRECT_OPCODES_LEN 0x20 > > @@ -1053,7 +1057,8 @@ struct CPUPPCState { > > /* Those resources are used only in QEMU core */ > > target_ulong hflags; /* hflags is a MSR & HFLAGS_MASK = */ > > target_ulong hflags_nmsr; /* specific hflags, not coming from MSR = */ > > - int mmu_idx; /* precomputed MMU index to speed up mem acce= sses */ > > + int immu_idx; /* precomputed MMU index to speed up insn ac= cess */ > > + int dmmu_idx; /* precomputed MMU index to speed up data ac= cesses */ > > =20 > > /* Power management */ > > int (*check_pow)(CPUPPCState *env); > > @@ -1245,7 +1250,7 @@ int ppc_dcr_write (ppc_dcr_t *dcr_env, int dcrn, = uint32_t val); > > #define MMU_USER_IDX 0 > > static inline int cpu_mmu_index (CPUPPCState *env, bool ifetch) > > { > > - return env->mmu_idx; > > + return ifetch ? env->immu_idx : env->dmmu_idx; > > } > > =20 > > #include "exec/cpu-all.h" > > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > > index 288903e..ba3caec 100644 > > --- a/target-ppc/excp_helper.c > > +++ b/target-ppc/excp_helper.c > > @@ -646,9 +646,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, in= t excp_model, int excp) > > =20 > > if (env->spr[SPR_LPCR] & LPCR_AIL) { > > new_msr |=3D (1 << MSR_IR) | (1 << MSR_DR); > > - } else if (msr & ((1 << MSR_IR) | (1 << MSR_DR))) { > > - /* If we disactivated any translation, flush TLBs */ > > - tlb_flush(cs, 1); > > } > > =20 > > #ifdef TARGET_PPC64 > > @@ -721,14 +718,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, i= nt excp_model, int excp) > > /* Reset exception state */ > > cs->exception_index =3D POWERPC_EXCP_NONE; > > env->error_code =3D 0; > > - > > - if ((env->mmu_model =3D=3D POWERPC_MMU_BOOKE) || > > - (env->mmu_model =3D=3D POWERPC_MMU_BOOKE206)) { > > - /* XXX: The BookE changes address space when switching modes, > > - we should probably implement that as different MMU ind= exes, > > - but for the moment we do it the slow way and flush all= =2E */ > > - tlb_flush(cs, 1); > > - } > > } > > =20 > > void ppc_cpu_do_interrupt(CPUState *cs) > > diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h > > index 271fddf..f7edd5b 100644 > > --- a/target-ppc/helper_regs.h > > +++ b/target-ppc/helper_regs.h > > @@ -41,11 +41,50 @@ static inline void hreg_swap_gpr_tgpr(CPUPPCState *= env) > > =20 > > static inline void hreg_compute_mem_idx(CPUPPCState *env) > > { > > - /* Precompute MMU index */ > > - if (msr_pr =3D=3D 0 && msr_hv !=3D 0) { > > - env->mmu_idx =3D 2; > > + /* This is our encoding for server processors > > + * > > + * 0 =3D Guest User space virtual mode > > + * 1 =3D Guest Kernel space virtual mode > > + * 2 =3D Guest Kernel space real mode > > + * 3 =3D HV User space virtual mode > > + * 4 =3D HV Kernel space virtual mode > > + * 5 =3D HV Kernel space real mode > > + * > > + * The combination PR=3D1 IR&DR=3D0 is invalid, we will treat > > + * it as IR=3DDR=3D1 > > + * > > + * For BookE, we need 8 MMU modes as follow: > > + * > > + * 0 =3D AS 0 HV User space > > + * 1 =3D AS 0 HV Kernel space > > + * 2 =3D AS 1 HV User space > > + * 3 =3D AS 1 HV Kernel space > > + * 4 =3D AS 0 Guest User space > > + * 5 =3D AS 0 Guest Kernel space > > + * 6 =3D AS 1 Guest User space > > + * 7 =3D AS 1 Guest Kernel space > > + */ > > + if (env->mmu_model & POWERPC_MMU_BOOKE) { > > + env->immu_idx =3D env->dmmu_idx =3D msr_pr ? 0 : 1; > > + env->immu_idx +=3D msr_is ? 2 : 0; > > + env->dmmu_idx +=3D msr_ds ? 2 : 0; > > + env->immu_idx +=3D msr_gs ? 4 : 0; > > + env->dmmu_idx +=3D msr_gs ? 4 : 0; > > } else { > > - env->mmu_idx =3D 1 - msr_pr; > > + /* First calucalte a base value independent of HV */ > > + if (msr_pr !=3D 0) { > > + /* User space, ignore IR and DR */ > > + env->immu_idx =3D env->dmmu_idx =3D 0; > > + } else { > > + /* Kernel, setup a base I/D value */ > > + env->immu_idx =3D msr_ir ? 1 : 2; > > + env->dmmu_idx =3D msr_dr ? 1 : 2; > > + } > > + /* Then offset it for HV */ > > + if (msr_hv) { > > + env->immu_idx +=3D 3; > > + env->dmmu_idx +=3D 3; > > + } > > } > > } > > =20 > > @@ -82,9 +121,10 @@ static inline int hreg_store_msr(CPUPPCState *env, = target_ulong value, > > } > > if (((value >> MSR_IR) & 1) !=3D msr_ir || > > ((value >> MSR_DR) & 1) !=3D msr_dr) { > > - /* Flush all tlb when changing translation mode */ > > - tlb_flush(cs, 1); > > - excp =3D POWERPC_EXCP_NONE; > > + cs->interrupt_request |=3D CPU_INTERRUPT_EXITTB; > > + } > > + if ((env->mmu_model & POWERPC_MMU_BOOKE) && > > + ((value >> MSR_GS) & 1) !=3D msr_gs) { > > cs->interrupt_request |=3D CPU_INTERRUPT_EXITTB; > > } > > if (unlikely((env->flags & POWERPC_FLAG_TGPR) && > > diff --git a/target-ppc/machine.c b/target-ppc/machine.c > > index f6c7256..4820f22 100644 > > --- a/target-ppc/machine.c > > +++ b/target-ppc/machine.c > > @@ -97,9 +97,12 @@ static int cpu_load_old(QEMUFile *f, void *opaque, i= nt version_id) > > qemu_get_betls(f, &env->nip); > > qemu_get_betls(f, &env->hflags); > > qemu_get_betls(f, &env->hflags_nmsr); > > - qemu_get_sbe32s(f, &env->mmu_idx); > > + qemu_get_sbe32(f); /* Discard unused mmu_idx */ > > qemu_get_sbe32(f); /* Discard unused power_mode */ > > =20 > > + /* Recompute mmu indices */ > > + hreg_compute_mem_idx(env); > > + > > return 0; > > } > > =20 > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > > index f5ceae5..b757634 100644 > > --- a/target-ppc/translate.c > > +++ b/target-ppc/translate.c > > @@ -11220,8 +11220,9 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, = fprintf_function cpu_fprintf, > > env->nip, env->lr, env->ctr, cpu_read_xer(env), > > cs->cpu_index); > > cpu_fprintf(f, "MSR " TARGET_FMT_lx " HID0 " TARGET_FMT_lx " HF " > > - TARGET_FMT_lx " idx %d\n", env->msr, env->spr[SPR_HID0= ], > > - env->hflags, env->mmu_idx); > > + TARGET_FMT_lx " iidx %d didx %d\n", > > + env->msr, env->spr[SPR_HID0], > > + env->hflags, env->immu_idx, env->dmmu_idx); > > #if !defined(NO_TIMER_DUMP) > > cpu_fprintf(f, "TB %08" PRIu32 " %08" PRIu64 > > #if !defined(CONFIG_USER_ONLY) > > @@ -11428,7 +11429,7 @@ void gen_intermediate_code(CPUPPCState *env, st= ruct TranslationBlock *tb) > > ctx.spr_cb =3D env->spr_cb; > > ctx.pr =3D msr_pr; > > ctx.hv =3D !msr_pr && msr_hv; > > - ctx.mem_idx =3D env->mmu_idx; > > + ctx.mem_idx =3D env->dmmu_idx; > > ctx.insns_flags =3D env->insns_flags; > > ctx.insns_flags2 =3D env->insns_flags2; > > ctx.access_type =3D -1; >=20 > Apologies to have to report another regression, however this patch > causes a kernel panic in Darwin PPC under TCG: >=20 > ./qemu-system-ppc -cdrom darwinppc-602.iso -boot d >=20 > The panic backtrace on-screen points towards one of the SCSI modules, so > perhaps this patch doesn't handle DMA correctly when trying to access > the CDROM? Bother. It's a lot less obvious to me how this one would cause trouble. Possibly a bug in the CD-ROM device which was masked by the pervious mem access stuff. --=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 --fmvA4kSBHQVZhkR6 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXT6S9AAoJEGw4ysog2bOST60QAKzk9833zALTVMnH7LMq9xYO rohk085AUEoRZzDv2SqJLnBHReyjjXQapnBF0DSad+85cT/OBOFuMzlwWuZtlusr g+OAQw3xtCYIpQzzaHcGGWYYcdYFzyN3QX+bGHogVLP9XXMGcs0DN2RBL7KOtGGG yDiEb63KNQdHbEP2hdK9ddEFBS2i8EXbOk9olPVn89YOgm/BD1axnicqpKaLPmJQ e3MOcHlTtUJd1z/F7VUlFANoeo3TxqVmcbfS+mywGMn5Vk4vm5VrnAKQpVLNCdOM Py3GHanU2xsFeDTGA5w4S3b3yOMGcQ5VyNprNdjRXiLodLXQgKqrw6AD1YGa0QMz nll8Tgs35kQdj16X57FBFK1EGZ6M5LMZnvHvqE7q4Fe2Qlt6CstTQlpiRg/Y5Rgv IcwVZ928NfwS/XaRcUHivMcUYc/BLLwcuHlS1hZ32IKPAjLhrNn+dnjQELw1Xcxe 031z700yW9/DDu2UEHgn1htw22kFlfdva2m3z06yPQOW07QYsqmDXQqXopzwMUWO ps4/BASlaSfk7BYmRcbvBqhgNogBlss/dUm0mciQg//ewzoO33OP8bGESTqiBG/7 YLfL2YjoGTDTEs1Jnc/rE1zLBRi752eDnpGNlJ4Bej3iRAHIeBpyPIl3AKB8oSS6 SBD7/VTK6b8PzqN7sJe6 =pNBu -----END PGP SIGNATURE----- --fmvA4kSBHQVZhkR6--