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);
+
+ 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)
{
--
2.21.0
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)
> {
--
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
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)
> >> {
> >
>
--
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
© 2016 - 2026 Red Hat, Inc.