On Tue, Nov 24, 2015 at 11:51:44AM +1100, Benjamin Herrenschmidt wrote: > On Fri, 2015-11-20 at 18:45 +1100, David Gibson wrote: > > snip] > > > /* tlbiel */ > > > static void gen_tlbiel(DisasContext *ctx) > > > { > > > #if defined(CONFIG_USER_ONLY) > > > - gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > + GEN_PRIV; > > > #else > > > - if (unlikely(ctx->pr || !ctx->hv)) { > > > - gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > - return; > > > - } > > > + CHK_SV; > > > > You have CHK_SV here, but the original code checks for HV, as does > > your new code for tlbia and tlbiel, is that right? > > Yes. tlbiel is supervisor accessible (for weird reasons). > > > [snip] > > > /* tlbsync */ > > > static void gen_tlbsync(DisasContext *ctx) > > > { > > > #if defined(CONFIG_USER_ONLY) > > > - gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > -#else > > > - if (unlikely(ctx->pr)) { > > > - gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > - return; > > > - } > > > + GEN_PRIV; > > > +#else > > > + CHK_HV; > > > + > > > > Old code didn't check for HV, mode, but AFAICT it should have, so > > this looks correct. > > Yes, this is a hypervisor instruction. > > > [snip] > > > @@ -5941,18 +5921,16 @@ static void gen_mfapidi(DisasContext *ctx) > > > static void gen_tlbiva(DisasContext *ctx) > > > { > > > #if defined(CONFIG_USER_ONLY) > > > - gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > + GEN_PRIV; > > > #else > > > TCGv t0; > > > - if (unlikely(ctx->pr)) { > > > - gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > - return; > > > - } > > > + > > > + CHK_SV; > > > > Is the same thing as tlbivax, or some ancient instruction? AFAICT > > the ISA says tlbivax is hypervisor privileged. > > "tlbiva" is the 4xx variant, there is no hypervisor mode on these > things. > > > > t0 = tcg_temp_new(); > > > gen_addr_reg_index(ctx, t0); > > > gen_helper_tlbie(cpu_env, cpu_gpr[rB(ctx->opcode)]); > > > tcg_temp_free(t0); > > > -#endif > > > +#endif /* defined(CONFIG_USER_ONLY) */ > > > } > > > > [snip] > > > static void gen_tlbivax_booke206(DisasContext *ctx) > > > { > > > #if defined(CONFIG_USER_ONLY) > > > - gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > + GEN_PRIV; > > > #else > > > TCGv t0; > > > - if (unlikely(ctx->pr)) { > > > - gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > - return; > > > - } > > > > > > + CHK_SV; > > > > ISA says tlbivax is hypervisor privileged when the CPU has a > > hypervisor mode, which I guess booke206 probably doesn't? > > Right so here, the "problem" is that afaik, TCG doesn't implement > the BookE hypervisor mode. So with my limited BookE testing > ability I prefer sticking to a mechanical replacement that matches > the existing code. It can be fixed later if necessary.
Fair enough. > > > t0 = tcg_temp_new(); > > > gen_addr_reg_index(ctx, t0); > > > - > > > gen_helper_booke206_tlbivax(cpu_env, t0); > > > tcg_temp_free(t0); > > > -#endif > > > +#endif /* defined(CONFIG_USER_ONLY) */ > > > } > > > > > > static void gen_tlbilx_booke206(DisasContext *ctx) > > > { > > > #if defined(CONFIG_USER_ONLY) > > > - gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > + GEN_PRIV; > > > #else > > > TCGv t0; > > > - if (unlikely(ctx->pr)) { > > > - gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); > > > - return; > > > - } > > > > > > + CHK_SV; > > > > And apparently hv vs. sv privilege of tlbilx depends on the EPCR > > register. Again, may not be relevant for 2.06. > > Well, here too, I basically preserve existing BookE TCG behaviour, > whether it's correct or not. That can be fixed separately if somebody > cares about BookE HV mode. > > > > t0 = tcg_temp_new(); > > > gen_addr_reg_index(ctx, t0); > > > > > > @@ -6672,7 +6574,7 @@ static void gen_tlbilx_booke206(DisasContext > > *ctx) > > > } > > > > > > tcg_temp_free(t0); > > > -#endif > > > +#endif /* defined(CONFIG_USER_ONLY) */ > > > } > > > -- 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
signature.asc
Description: PGP signature