On Wed, Jan 08, 2020 at 04:36:50PM +0100, Cédric Le Goater wrote: > On 12/19/19 6:12 AM, David Gibson wrote: > > On Thu, Nov 28, 2019 at 02:46:59PM +0100, Cédric Le Goater wrote: > >> The privileged message send and clear instructions (msgsndp & msgclrp) > >> are privileged, but will generate a hypervisor facility unavailable > >> exception if not enabled in the HFSCR and executed in privileged > >> non-hypervisor state. > >> > >> Add checks when accessing the DPDES register and when using the > >> msgsndp and msgclrp isntructions. > >> > >> Based on previous work from Suraj Jitindar Singh. > >> > >> Cc: Suraj Jitindar Singh <[email protected]> > >> Signed-off-by: Cédric Le Goater <[email protected]> > >> --- > >> target/ppc/cpu.h | 6 ++++++ > >> target/ppc/helper.h | 1 + > >> target/ppc/excp_helper.c | 9 +++++++++ > >> target/ppc/misc_helper.c | 24 ++++++++++++++++++++++++ > >> target/ppc/translate.c | 4 ++++ > >> target/ppc/translate_init.inc.c | 18 ++++++++++++++++++ > >> 6 files changed, 62 insertions(+) > >> > >> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > >> index 8ffcfa0ea162..52608dfe6ff4 100644 > >> --- a/target/ppc/cpu.h > >> +++ b/target/ppc/cpu.h > >> @@ -397,6 +397,10 @@ typedef struct ppc_v3_pate_t { > >> #define PSSCR_ESL PPC_BIT(42) /* Enable State Loss */ > >> #define PSSCR_EC PPC_BIT(43) /* Exit Criterion */ > >> > >> +/* HFSCR bits */ > >> +#define HFSCR_MSGP PPC_BIT(53) /* Privileged Message Send Facilities > >> */ > >> +#define HFSCR_IC_MSGP 0xA > >> + > >> #define msr_sf ((env->msr >> MSR_SF) & 1) > >> #define msr_isf ((env->msr >> MSR_ISF) & 1) > >> #define msr_shv ((env->msr >> MSR_SHV) & 1) > >> @@ -1333,6 +1337,8 @@ void cpu_ppc_set_vhyp(PowerPCCPU *cpu, > >> PPCVirtualHypervisor *vhyp); > >> #endif > >> > >> void store_fpscr(CPUPPCState *env, uint64_t arg, uint32_t mask); > >> +void gen_hfscr_facility_check(DisasContext *ctx, int facility_sprn, int > >> bit, > >> + int sprn, int cause); > >> > >> static inline uint64_t ppc_dump_gpr(CPUPPCState *env, int gprn) > >> { > >> diff --git a/target/ppc/helper.h b/target/ppc/helper.h > >> index 76518a1df6f0..14c9a30a45c9 100644 > >> --- a/target/ppc/helper.h > >> +++ b/target/ppc/helper.h > >> @@ -643,6 +643,7 @@ DEF_HELPER_3(store_dcr, void, env, tl, tl) > >> > >> DEF_HELPER_2(load_dump_spr, void, env, i32) > >> DEF_HELPER_2(store_dump_spr, void, env, i32) > >> +DEF_HELPER_4(hfscr_facility_check, void, env, i32, i32, i32) > >> DEF_HELPER_4(fscr_facility_check, void, env, i32, i32, i32) > >> DEF_HELPER_4(msr_facility_check, void, env, i32, i32, i32) > >> DEF_HELPER_FLAGS_1(load_tbl, TCG_CALL_NO_RWG, tl, env) > >> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > >> index 5a247945e97f..17dad626b74e 100644 > >> --- a/target/ppc/excp_helper.c > >> +++ b/target/ppc/excp_helper.c > >> @@ -469,6 +469,15 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int > >> excp_model, int excp) > >> case POWERPC_EXCP_FU: /* Facility unavailable exception > >> */ > >> #ifdef TARGET_PPC64 > >> env->spr[SPR_FSCR] |= ((target_ulong)env->error_code << 56); > >> +#endif > >> + break; > >> + case POWERPC_EXCP_HV_FU: /* Hypervisor Facility Unavailable > >> Exception */ > >> +#ifdef TARGET_PPC64 > >> + env->spr[SPR_HFSCR] |= ((target_ulong)env->error_code << > >> FSCR_IC_POS); > >> + srr0 = SPR_HSRR0; > >> + srr1 = SPR_HSRR1; > >> + new_msr |= (target_ulong)MSR_HVB; > >> + new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > >> #endif > >> break; > >> case POWERPC_EXCP_PIT: /* Programmable interval timer interrupt > >> */ > >> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c > >> index a0e7bd9c32d3..0cd44c6edd82 100644 > >> --- a/target/ppc/misc_helper.c > >> +++ b/target/ppc/misc_helper.c > >> @@ -41,6 +41,17 @@ void helper_store_dump_spr(CPUPPCState *env, uint32_t > >> sprn) > >> } > >> > >> #ifdef TARGET_PPC64 > >> +static void raise_hv_fu_exception(CPUPPCState *env, uint32_t bit, > >> + uint32_t sprn, uint32_t cause, > >> + uintptr_t raddr) > >> +{ > >> + qemu_log("Facility SPR %d is unavailable (SPR HFSCR:%d)\n", sprn, > >> bit); > > > > That looks overly verbose. Leftover debugging? > > No. It's inherited from raise_fu_exception(). Should we remove them ? > or use mask CPU_LOG_INT ?
Ah. We sohuld definitely add a mask. CPU_LOG_INT sounds like the
right one.
> >> + env->spr[SPR_HFSCR] &= ~((target_ulong)FSCR_IC_MASK << FSCR_IC_POS);
> >> +
> >> + raise_exception_err_ra(env, POWERPC_EXCP_HV_FU, cause, raddr);
> >> +}
> >> +
> >> static void raise_fu_exception(CPUPPCState *env, uint32_t bit,
> >> uint32_t sprn, uint32_t cause,
> >> uintptr_t raddr)
> >> @@ -55,6 +66,17 @@ static void raise_fu_exception(CPUPPCState *env,
> >> uint32_t bit,
> >> }
> >> #endif
> >>
> >> +void helper_hfscr_facility_check(CPUPPCState *env, uint32_t bit,
> >> + uint32_t sprn, uint32_t cause)
> >> +{
> >> +#ifdef TARGET_PPC64
> >> + if ((env->msr_mask & MSR_HVB) && !msr_hv &&
> >> + !(env->spr[SPR_HFSCR] & (1UL <<
> >> bit))) {
> >> + raise_hv_fu_exception(env, bit, sprn, cause, GETPC());
> >> + }
> >> +#endif
> >> +}
> >> +
> >> void helper_fscr_facility_check(CPUPPCState *env, uint32_t bit,
> >> uint32_t sprn, uint32_t cause)
> >> {
> >> @@ -108,6 +130,8 @@ void helper_store_pcr(CPUPPCState *env, target_ulong
> >> value)
> >>
> >> target_ulong helper_load_dpdes(CPUPPCState *env)
> >> {
> >> + helper_hfscr_facility_check(env, HFSCR_MSGP, SPR_DPDES,
> >> + HFSCR_IC_MSGP);
> >> if (env->pending_interrupts & (1 << PPC_INTERRUPT_DOORBELL)) {
> >> return 1;
> >> }
> >> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> >> index ba759ab2bb0f..e9e70ca149fd 100644
> >> --- a/target/ppc/translate.c
> >> +++ b/target/ppc/translate.c
> >> @@ -6652,6 +6652,8 @@ static void gen_msgclrp(DisasContext *ctx)
> >> GEN_PRIV;
> >> #else
> >> CHK_SV;
> >> + gen_hfscr_facility_check(ctx, SPR_HFSCR, HFSCR_MSGP, 0,
> >> + HFSCR_IC_MSGP);
> >> gen_helper_book3s_msgclrp(cpu_env, cpu_gpr[rB(ctx->opcode)]);
> >
> > Calling a helper for the facility check, then another helper for the
> > actual instruction is a bit yucky. I'd prefer if you either call out
> > for the facility check within the instruction helper, or generate the
> > instructions necessary for the HFSCR check
>
> OK. I will take a look.
>
> C.
>
> >
> >> #endif /* defined(CONFIG_USER_ONLY) */
> >> }
> >> @@ -6662,6 +6664,8 @@ static void gen_msgsndp(DisasContext *ctx)
> >> GEN_PRIV;
> >> #else
> >> CHK_SV;
> >> + gen_hfscr_facility_check(ctx, SPR_HFSCR, HFSCR_MSGP, 0,
> >> + HFSCR_IC_MSGP);
> >> gen_helper_book3s_msgsndp(cpu_gpr[rB(ctx->opcode)]);
> >> #endif /* defined(CONFIG_USER_ONLY) */
> >> }
> >> diff --git a/target/ppc/translate_init.inc.c
> >> b/target/ppc/translate_init.inc.c
> >> index 7c74a763ba66..154e01451270 100644
> >> --- a/target/ppc/translate_init.inc.c
> >> +++ b/target/ppc/translate_init.inc.c
> >> @@ -468,11 +468,15 @@ static void spr_write_pcr(DisasContext *ctx, int
> >> sprn, int gprn)
> >> /* DPDES */
> >> static void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn)
> >> {
> >> + gen_hfscr_facility_check(ctx, SPR_HFSCR, HFSCR_MSGP, sprn,
> >> + HFSCR_IC_MSGP);
> >> gen_helper_load_dpdes(cpu_gpr[gprn], cpu_env);
> >> }
> >>
> >> static void spr_write_dpdes(DisasContext *ctx, int sprn, int gprn)
> >> {
> >> + gen_hfscr_facility_check(ctx, SPR_HFSCR, HFSCR_MSGP, sprn,
> >> + HFSCR_IC_MSGP);
> >> gen_helper_store_dpdes(cpu_env, cpu_gpr[gprn]);
> >> }
> >> #endif
> >> @@ -7523,6 +7527,20 @@ POWERPC_FAMILY(e600)(ObjectClass *oc, void *data)
> >> #define POWERPC970_HID5_INIT 0x00000000
> >> #endif
> >>
> >> +void gen_hfscr_facility_check(DisasContext *ctx, int facility_sprn, int
> >> bit,
> >> + int sprn, int cause)
> >> +{
> >> + TCGv_i32 t1 = tcg_const_i32(bit);
> >> + TCGv_i32 t2 = tcg_const_i32(sprn);
> >> + TCGv_i32 t3 = tcg_const_i32(cause);
> >> +
> >> + gen_helper_hfscr_facility_check(cpu_env, t1, t2, t3);
> >> +
> >> + tcg_temp_free_i32(t3);
> >> + tcg_temp_free_i32(t2);
> >> + tcg_temp_free_i32(t1);
> >> +}
> >> +
> >> static void gen_fscr_facility_check(DisasContext *ctx, int facility_sprn,
> >> int bit, int sprn, int cause)
> >> {
> >
>
--
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
