| Message ID | 20191128134700.16091-7-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/ppc: Implement KVM support under TCG (final steps) | expand |
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 <sjitindarsingh@gmail.com> > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > 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? > + 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 > #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) > {
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 <sjitindarsingh@gmail.com> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> 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 ? > >> + 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) >> { >
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 <sjitindarsingh@gmail.com> > >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > >> --- > >> 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) > >> { > > >
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); + + 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)]); #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) {
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 <sjitindarsingh@gmail.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> --- 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(+)