* [PATCH v2 0/2] target/ppc: Implement Dynamic Execution Control Registers
@ 2022-12-09 6:13 Nicholas Miehlbradt
2022-12-09 6:13 ` [PATCH v2 1/2] target/ppc: Implement the DEXCR and HDEXCR Nicholas Miehlbradt
2022-12-09 6:13 ` [PATCH v2 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions Nicholas Miehlbradt
0 siblings, 2 replies; 6+ messages in thread
From: Nicholas Miehlbradt @ 2022-12-09 6:13 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, danielhb413, clg, david, groug, victor.colombo, mikey,
Nicholas Miehlbradt
Implements the Dynamic Execution Control Register (DEXCR) and the
Hypervisor Dynamic Execution Control Register (HDEXCR) in TCG as
defined in Power ISA 3.1B. Only aspects 5 (Non-privileged hash instruction
enable) and 6 (Privileged hash instruction enable) have architectural
effects. Other aspects can be manipulated but have no effect on execution.
Adds checks to these registers in the hashst and hashchk instructions so
that they are executed as nops when not enabled.
There is currently an RFC for the kernel interface for the DEXCR on the
Linux PPC mailing list:
https://lore.kernel.org/linuxppc-dev/20221128024458.46121-1-bgray@linux.ibm.com/
Nicholas Miehlbradt (2):
target/ppc: Implement the DEXCR and HDEXCR
target/ppc: Check DEXCR on hash{st, chk} instructions
target/ppc/cpu.h | 19 +++++++++++++
target/ppc/cpu_init.c | 25 +++++++++++++++++
target/ppc/excp_helper.c | 58 +++++++++++++++++++++++++++++-----------
target/ppc/spr_common.h | 1 +
target/ppc/translate.c | 19 +++++++++++++
5 files changed, 107 insertions(+), 15 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] target/ppc: Implement the DEXCR and HDEXCR
2022-12-09 6:13 [PATCH v2 0/2] target/ppc: Implement Dynamic Execution Control Registers Nicholas Miehlbradt
@ 2022-12-09 6:13 ` Nicholas Miehlbradt
2022-12-11 18:28 ` Harsh Prateek Bora
2022-12-09 6:13 ` [PATCH v2 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions Nicholas Miehlbradt
1 sibling, 1 reply; 6+ messages in thread
From: Nicholas Miehlbradt @ 2022-12-09 6:13 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, danielhb413, clg, david, groug, victor.colombo, mikey,
Nicholas Miehlbradt
Define the DEXCR and HDEXCR as special purpose registers.
Each register occupies two SPR indicies, one which can be read in an
unprivileged state and one which can be modified in the appropriate
priviliged state, however both indicies refer to the same underlying
value.
Note that the ISA uses the abbreviation UDEXCR in two different
contexts: the userspace DEXCR, the SPR index which can be read from
userspace (implemented in this patch), and the ultravisor DEXCR, the
equivalent register for the ultravisor state (not implemented).
Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com>
---
v2: Clearing of upper 32 bits of DEXCR is now performed on read from
problem state rather than on write in privileged state.
---
target/ppc/cpu.h | 19 +++++++++++++++++++
target/ppc/cpu_init.c | 25 +++++++++++++++++++++++++
target/ppc/spr_common.h | 1 +
target/ppc/translate.c | 19 +++++++++++++++++++
4 files changed, 64 insertions(+)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 81d4263a07..0ed9f2ae35 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1068,6 +1068,21 @@ struct ppc_radix_page_info {
uint32_t entries[PPC_PAGE_SIZES_MAX_SZ];
};
+/*****************************************************************************/
+/* Dynamic Execution Control Register */
+
+#define DEXCR_ASPECT(name, num) \
+FIELD(DEXCR, PNH_##name, PPC_BIT_NR(num), 1) \
+FIELD(DEXCR, PRO_##name, PPC_BIT_NR(num + 32), 1) \
+FIELD(HDEXCR, HNU_##name, PPC_BIT_NR(num), 1) \
+FIELD(HDEXCR, ENF_##name, PPC_BIT_NR(num + 32), 1) \
+
+DEXCR_ASPECT(SBHE, 0)
+DEXCR_ASPECT(IDRTPB, 1)
+DEXCR_ASPECT(SRAPD, 4)
+DEXCR_ASPECT(NPHIE, 5)
+DEXCR_ASPECT(PHIE, 6)
+
/*****************************************************************************/
/* The whole PowerPC CPU context */
@@ -1674,9 +1689,11 @@ void ppc_compat_add_property(Object *obj, const char *name,
#define SPR_BOOKE_GIVOR13 (0x1BC)
#define SPR_BOOKE_GIVOR14 (0x1BD)
#define SPR_TIR (0x1BE)
+#define SPR_UHDEXCR (0x1C7)
#define SPR_PTCR (0x1D0)
#define SPR_HASHKEYR (0x1D4)
#define SPR_HASHPKEYR (0x1D5)
+#define SPR_HDEXCR (0x1D7)
#define SPR_BOOKE_SPEFSCR (0x200)
#define SPR_Exxx_BBEAR (0x201)
#define SPR_Exxx_BBTAR (0x202)
@@ -1865,8 +1882,10 @@ void ppc_compat_add_property(Object *obj, const char *name,
#define SPR_RCPU_L2U_RA2 (0x32A)
#define SPR_MPC_MD_DBRAM1 (0x32A)
#define SPR_RCPU_L2U_RA3 (0x32B)
+#define SPR_UDEXCR (0x32C)
#define SPR_TAR (0x32F)
#define SPR_ASDR (0x330)
+#define SPR_DEXCR (0x33C)
#define SPR_IC (0x350)
#define SPR_VTB (0x351)
#define SPR_MMCRC (0x353)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index cbf0081374..6433f4fdfd 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5727,6 +5727,30 @@ static void register_power10_hash_sprs(CPUPPCState *env)
hashpkeyr_initial_value);
}
+static void register_power10_dexcr_sprs(CPUPPCState *env)
+{
+ spr_register(env, SPR_DEXCR, "DEXCR",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_generic,
+ 0);
+
+ spr_register(env, SPR_UDEXCR, "DEXCR",
+ &spr_read_dexcr_ureg, SPR_NOACCESS,
+ &spr_read_dexcr_ureg, SPR_NOACCESS,
+ 0);
+
+ spr_register_hv(env, SPR_HDEXCR, "HDEXCR",
+ SPR_NOACCESS, SPR_NOACCESS,
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_generic,
+ 0);
+
+ spr_register(env, SPR_UHDEXCR, "HDEXCR",
+ &spr_read_dexcr_ureg, SPR_NOACCESS,
+ &spr_read_dexcr_ureg, SPR_NOACCESS,
+ 0);
+}
+
/*
* Initialize PMU counter overflow timers for Power8 and
* newer Power chips when using TCG.
@@ -6402,6 +6426,7 @@ static void init_proc_POWER10(CPUPPCState *env)
register_power8_rpr_sprs(env);
register_power9_mmu_sprs(env);
register_power10_hash_sprs(env);
+ register_power10_dexcr_sprs(env);
/* FIXME: Filter fields properly based on privilege level */
spr_register_kvm_hv(env, SPR_PSSCR, "PSSCR", NULL, NULL, NULL, NULL,
diff --git a/target/ppc/spr_common.h b/target/ppc/spr_common.h
index b5a5bc6895..91a74cec0f 100644
--- a/target/ppc/spr_common.h
+++ b/target/ppc/spr_common.h
@@ -195,6 +195,7 @@ void spr_read_ebb_upper32(DisasContext *ctx, int gprn, int sprn);
void spr_write_ebb_upper32(DisasContext *ctx, int sprn, int gprn);
void spr_write_hmer(DisasContext *ctx, int sprn, int gprn);
void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn);
+void spr_read_dexcr_ureg(DisasContext *ctx, int sprn, int gprn);
#endif
void register_low_BATs(CPUPPCState *env);
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 19c1d17cb0..fcb1180712 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1249,6 +1249,25 @@ void spr_write_ebb_upper32(DisasContext *ctx, int sprn, int gprn)
gen_fscr_facility_check(ctx, SPR_FSCR, FSCR_EBB, sprn, FSCR_IC_EBB);
spr_write_prev_upper32(ctx, sprn, gprn);
}
+
+void spr_read_dexcr_ureg(DisasContext *ctx, int gprn, int sprn)
+{
+ TCGv t0 = tcg_temp_new();
+
+ /*
+ * Access to the (H)DEXCR in problem state is done using seperate
+ * SPR indexes which are 16 below the SPR indexes which have full
+ * access to the (H)DEXCR in privileged state. Problem state may
+ * only read bits 32:63, bits 0:31 return 0.
+ *
+ * See section 9.3.1-9.3.2 of PowerISA v3.1B
+ */
+
+ gen_load_spr(t0, sprn + 16);
+ tcg_gen_ext32u_tl(cpu_gpr[gprn], t0);
+
+ tcg_temp_free(t0);
+}
#endif
#define GEN_HANDLER(name, opc1, opc2, opc3, inval, type) \
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions
2022-12-09 6:13 [PATCH v2 0/2] target/ppc: Implement Dynamic Execution Control Registers Nicholas Miehlbradt
2022-12-09 6:13 ` [PATCH v2 1/2] target/ppc: Implement the DEXCR and HDEXCR Nicholas Miehlbradt
@ 2022-12-09 6:13 ` Nicholas Miehlbradt
2022-12-11 18:36 ` Harsh Prateek Bora
1 sibling, 1 reply; 6+ messages in thread
From: Nicholas Miehlbradt @ 2022-12-09 6:13 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, danielhb413, clg, david, groug, victor.colombo, mikey,
Nicholas Miehlbradt
Adds checks to the hashst and hashchk instructions to only execute if
enabled by the relevant aspect in the DEXCR and HDEXCR.
This behaviour is guarded behind TARGET_PPC64 since Power10 is
currently the only implementation which has the DEXCR.
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com>
---
target/ppc/excp_helper.c | 58 +++++++++++++++++++++++++++++-----------
1 file changed, 43 insertions(+), 15 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 94adcb766b..add4d54ae7 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2902,29 +2902,57 @@ static uint64_t hash_digest(uint64_t ra, uint64_t rb, uint64_t key)
return stage1_h ^ stage1_l;
}
+static void do_hash(CPUPPCState *env, target_ulong ea, target_ulong ra,
+ target_ulong rb, uint64_t key, bool store)
+{
+ uint64_t calculated_hash = hash_digest(ra, rb, key), loaded_hash;
+
+ if (store) {
+ cpu_stq_data_ra(env, ea, calculated_hash, GETPC());
+ } else {
+ loaded_hash = cpu_ldq_data_ra(env, ea, GETPC());
+ if (loaded_hash != calculated_hash) {
+ raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
+ POWERPC_EXCP_TRAP, GETPC());
+ }
+ }
+}
+
#include "qemu/guest-random.h"
-#define HELPER_HASH(op, key, store) \
+#ifdef TARGET_PPC64
+#define HELPER_HASH(op, key, store, dexcr_aspect) \
void helper_##op(CPUPPCState *env, target_ulong ea, target_ulong ra, \
target_ulong rb) \
{ \
- uint64_t calculated_hash = hash_digest(ra, rb, key), loaded_hash; \
- \
- if (store) { \
- cpu_stq_data_ra(env, ea, calculated_hash, GETPC()); \
- } else { \
- loaded_hash = cpu_ldq_data_ra(env, ea, GETPC()); \
- if (loaded_hash != calculated_hash) { \
- raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, \
- POWERPC_EXCP_TRAP, GETPC()); \
- } \
+ if (env->msr & R_MSR_PR_MASK) { \
+ if (!(env->spr[SPR_DEXCR] & R_DEXCR_PRO_##dexcr_aspect##_MASK || \
+ env->spr[SPR_HDEXCR] & R_HDEXCR_ENF_##dexcr_aspect##_MASK)) \
+ return; \
+ } else if (!(env->msr & R_MSR_HV_MASK)) { \
+ if (!(env->spr[SPR_DEXCR] & R_DEXCR_PNH_##dexcr_aspect##_MASK || \
+ env->spr[SPR_HDEXCR] & R_HDEXCR_ENF_##dexcr_aspect##_MASK)) \
+ return; \
+ } else if (!(env->msr & R_MSR_S_MASK)) { \
+ if (!(env->spr[SPR_HDEXCR] & R_HDEXCR_HNU_##dexcr_aspect##_MASK)) \
+ return; \
} \
+ \
+ do_hash(env, ea, ra, rb, key, store); \
+}
+#else
+#define HELPER_HASH(op, key, store, dexcr_aspect) \
+void helper_##op(CPUPPCState *env, target_ulong ea, target_ulong ra, \
+ target_ulong rb) \
+{ \
+ do_hash(env, ea, ra, rb, key, store); \
}
+#endif /* TARGET_PPC64 */
-HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true)
-HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false)
-HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true)
-HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false)
+HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true, NPHIE)
+HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false, NPHIE)
+HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true, PHIE)
+HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false, PHIE)
#endif /* CONFIG_TCG */
#if !defined(CONFIG_USER_ONLY)
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] target/ppc: Implement the DEXCR and HDEXCR
2022-12-09 6:13 ` [PATCH v2 1/2] target/ppc: Implement the DEXCR and HDEXCR Nicholas Miehlbradt
@ 2022-12-11 18:28 ` Harsh Prateek Bora
0 siblings, 0 replies; 6+ messages in thread
From: Harsh Prateek Bora @ 2022-12-11 18:28 UTC (permalink / raw)
To: Nicholas Miehlbradt, qemu-devel
Cc: qemu-ppc, danielhb413, clg, david, groug, victor.colombo, mikey
On 12/9/22 11:43, Nicholas Miehlbradt wrote:
> Define the DEXCR and HDEXCR as special purpose registers.
>
> Each register occupies two SPR indicies, one which can be read in an
> unprivileged state and one which can be modified in the appropriate
> priviliged state, however both indicies refer to the same underlying
> value.
>
> Note that the ISA uses the abbreviation UDEXCR in two different
> contexts: the userspace DEXCR, the SPR index which can be read from
> userspace (implemented in this patch), and the ultravisor DEXCR, the
> equivalent register for the ultravisor state (not implemented).
>
> Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com>
> ---
> v2: Clearing of upper 32 bits of DEXCR is now performed on read from
> problem state rather than on write in privileged state.
> ---
> target/ppc/cpu.h | 19 +++++++++++++++++++
> target/ppc/cpu_init.c | 25 +++++++++++++++++++++++++
> target/ppc/spr_common.h | 1 +
> target/ppc/translate.c | 19 +++++++++++++++++++
> 4 files changed, 64 insertions(+)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 81d4263a07..0ed9f2ae35 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1068,6 +1068,21 @@ struct ppc_radix_page_info {
> uint32_t entries[PPC_PAGE_SIZES_MAX_SZ];
> };
>
> +/*****************************************************************************/
> +/* Dynamic Execution Control Register */
> +
> +#define DEXCR_ASPECT(name, num) \
> +FIELD(DEXCR, PNH_##name, PPC_BIT_NR(num), 1) \
> +FIELD(DEXCR, PRO_##name, PPC_BIT_NR(num + 32), 1) \
> +FIELD(HDEXCR, HNU_##name, PPC_BIT_NR(num), 1) \
> +FIELD(HDEXCR, ENF_##name, PPC_BIT_NR(num + 32), 1) \
> +
> +DEXCR_ASPECT(SBHE, 0)
> +DEXCR_ASPECT(IDRTPB, 1)
^^^^^^ IBRTPD ?
> +DEXCR_ASPECT(SRAPD, 4)
> +DEXCR_ASPECT(NPHIE, 5)
> +DEXCR_ASPECT(PHIE, 6)
> +
> /*****************************************************************************/
> /* The whole PowerPC CPU context */
>
> @@ -1674,9 +1689,11 @@ void ppc_compat_add_property(Object *obj, const char *name,
> #define SPR_BOOKE_GIVOR13 (0x1BC)
> #define SPR_BOOKE_GIVOR14 (0x1BD)
> #define SPR_TIR (0x1BE)
> +#define SPR_UHDEXCR (0x1C7)
> #define SPR_PTCR (0x1D0)
> #define SPR_HASHKEYR (0x1D4)
> #define SPR_HASHPKEYR (0x1D5)
> +#define SPR_HDEXCR (0x1D7)
> #define SPR_BOOKE_SPEFSCR (0x200)
> #define SPR_Exxx_BBEAR (0x201)
> #define SPR_Exxx_BBTAR (0x202)
> @@ -1865,8 +1882,10 @@ void ppc_compat_add_property(Object *obj, const char *name,
> #define SPR_RCPU_L2U_RA2 (0x32A)
> #define SPR_MPC_MD_DBRAM1 (0x32A)
> #define SPR_RCPU_L2U_RA3 (0x32B)
> +#define SPR_UDEXCR (0x32C)
> #define SPR_TAR (0x32F)
> #define SPR_ASDR (0x330)
> +#define SPR_DEXCR (0x33C)
> #define SPR_IC (0x350)
> #define SPR_VTB (0x351)
> #define SPR_MMCRC (0x353)
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index cbf0081374..6433f4fdfd 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -5727,6 +5727,30 @@ static void register_power10_hash_sprs(CPUPPCState *env)
> hashpkeyr_initial_value);
> }
>
> +static void register_power10_dexcr_sprs(CPUPPCState *env)
> +{
> + spr_register(env, SPR_DEXCR, "DEXCR",
> + SPR_NOACCESS, SPR_NOACCESS,
> + &spr_read_generic, &spr_write_generic,
> + 0);
> +
> + spr_register(env, SPR_UDEXCR, "DEXCR",
> + &spr_read_dexcr_ureg, SPR_NOACCESS,
> + &spr_read_dexcr_ureg, SPR_NOACCESS,
> + 0);
> +
> + spr_register_hv(env, SPR_HDEXCR, "HDEXCR",
> + SPR_NOACCESS, SPR_NOACCESS,
> + SPR_NOACCESS, SPR_NOACCESS,
> + &spr_read_generic, &spr_write_generic,
> + 0);
> +
> + spr_register(env, SPR_UHDEXCR, "HDEXCR",
> + &spr_read_dexcr_ureg, SPR_NOACCESS,
> + &spr_read_dexcr_ureg, SPR_NOACCESS,
> + 0);
> +}
> +
> /*
> * Initialize PMU counter overflow timers for Power8 and
> * newer Power chips when using TCG.
> @@ -6402,6 +6426,7 @@ static void init_proc_POWER10(CPUPPCState *env)
> register_power8_rpr_sprs(env);
> register_power9_mmu_sprs(env);
> register_power10_hash_sprs(env);
> + register_power10_dexcr_sprs(env);
>
> /* FIXME: Filter fields properly based on privilege level */
> spr_register_kvm_hv(env, SPR_PSSCR, "PSSCR", NULL, NULL, NULL, NULL,
> diff --git a/target/ppc/spr_common.h b/target/ppc/spr_common.h
> index b5a5bc6895..91a74cec0f 100644
> --- a/target/ppc/spr_common.h
> +++ b/target/ppc/spr_common.h
> @@ -195,6 +195,7 @@ void spr_read_ebb_upper32(DisasContext *ctx, int gprn, int sprn);
> void spr_write_ebb_upper32(DisasContext *ctx, int sprn, int gprn);
> void spr_write_hmer(DisasContext *ctx, int sprn, int gprn);
> void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn);
> +void spr_read_dexcr_ureg(DisasContext *ctx, int sprn, int gprn);
Order of sprn, gprn appears different in funcn defn below, need to
correct at either place.
> #endif
>
> void register_low_BATs(CPUPPCState *env);
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 19c1d17cb0..fcb1180712 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1249,6 +1249,25 @@ void spr_write_ebb_upper32(DisasContext *ctx, int sprn, int gprn)
> gen_fscr_facility_check(ctx, SPR_FSCR, FSCR_EBB, sprn, FSCR_IC_EBB);
> spr_write_prev_upper32(ctx, sprn, gprn);
> }
> +
> +void spr_read_dexcr_ureg(DisasContext *ctx, int gprn, int sprn)
> +{
> + TCGv t0 = tcg_temp_new();
> +
> + /*
> + * Access to the (H)DEXCR in problem state is done using seperate
> + * SPR indexes which are 16 below the SPR indexes which have full
> + * access to the (H)DEXCR in privileged state. Problem state may
s/may/can ?
> + * only read bits 32:63, bits 0:31 return 0.
> + *
> + * See section 9.3.1-9.3.2 of PowerISA v3.1B
> + */
> +
> + gen_load_spr(t0, sprn + 16);
> + tcg_gen_ext32u_tl(cpu_gpr[gprn], t0);
> +
> + tcg_temp_free(t0);
> +}
> #endif
>
> #define GEN_HANDLER(name, opc1, opc2, opc3, inval, type) \
Otherwise, looks good to me!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions
2022-12-09 6:13 ` [PATCH v2 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions Nicholas Miehlbradt
@ 2022-12-11 18:36 ` Harsh Prateek Bora
2022-12-11 21:10 ` Harsh Prateek Bora
0 siblings, 1 reply; 6+ messages in thread
From: Harsh Prateek Bora @ 2022-12-11 18:36 UTC (permalink / raw)
To: Nicholas Miehlbradt, qemu-devel
Cc: qemu-ppc, danielhb413, clg, david, groug, victor.colombo, mikey
On 12/9/22 11:43, Nicholas Miehlbradt wrote:
> Adds checks to the hashst and hashchk instructions to only execute if
> enabled by the relevant aspect in the DEXCR and HDEXCR.
>
> This behaviour is guarded behind TARGET_PPC64 since Power10 is
> currently the only implementation which has the DEXCR.
>
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com>
> ---
> target/ppc/excp_helper.c | 58 +++++++++++++++++++++++++++++-----------
> 1 file changed, 43 insertions(+), 15 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 94adcb766b..add4d54ae7 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -2902,29 +2902,57 @@ static uint64_t hash_digest(uint64_t ra, uint64_t rb, uint64_t key)
> return stage1_h ^ stage1_l;
> }
>
> +static void do_hash(CPUPPCState *env, target_ulong ea, target_ulong ra,
> + target_ulong rb, uint64_t key, bool store)
> +{
> + uint64_t calculated_hash = hash_digest(ra, rb, key), loaded_hash;
> +
> + if (store) {
> + cpu_stq_data_ra(env, ea, calculated_hash, GETPC());
> + } else {
> + loaded_hash = cpu_ldq_data_ra(env, ea, GETPC());
> + if (loaded_hash != calculated_hash) {
> + raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
> + POWERPC_EXCP_TRAP, GETPC());
> + }
> + }
> +}
> +
> #include "qemu/guest-random.h"
>
> -#define HELPER_HASH(op, key, store) \
> +#ifdef TARGET_PPC64
> +#define HELPER_HASH(op, key, store, dexcr_aspect) \
> void helper_##op(CPUPPCState *env, target_ulong ea, target_ulong ra, \
> target_ulong rb) \
> {
Conditional compilation could be contained within function, so that
duplicate lines of code in each macro block could be avoided and would
look simpler.
\
> - uint64_t calculated_hash = hash_digest(ra, rb, key), loaded_hash; \
> - \
> - if (store) { \
> - cpu_stq_data_ra(env, ea, calculated_hash, GETPC()); \
> - } else { \
> - loaded_hash = cpu_ldq_data_ra(env, ea, GETPC()); \
> - if (loaded_hash != calculated_hash) { \
> - raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, \
> - POWERPC_EXCP_TRAP, GETPC()); \
> - } \
> + if (env->msr & R_MSR_PR_MASK) { \
> + if (!(env->spr[SPR_DEXCR] & R_DEXCR_PRO_##dexcr_aspect##_MASK || \
> + env->spr[SPR_HDEXCR] & R_HDEXCR_ENF_##dexcr_aspect##_MASK)) \
> + return; \
> + } else if (!(env->msr & R_MSR_HV_MASK)) { \
> + if (!(env->spr[SPR_DEXCR] & R_DEXCR_PNH_##dexcr_aspect##_MASK || \
> + env->spr[SPR_HDEXCR] & R_HDEXCR_ENF_##dexcr_aspect##_MASK)) \
> + return; \
> + } else if (!(env->msr & R_MSR_S_MASK)) { \
> + if (!(env->spr[SPR_HDEXCR] & R_HDEXCR_HNU_##dexcr_aspect##_MASK)) \
> + return; \
> } \
> + \
> + do_hash(env, ea, ra, rb, key, store); \
> +}
> +#else
> +#define HELPER_HASH(op, key, store, dexcr_aspect) \
> +void helper_##op(CPUPPCState *env, target_ulong ea, target_ulong ra, \
> + target_ulong rb) \
> +{ \
> + do_hash(env, ea, ra, rb, key, store); \
> }
> +#endif /* TARGET_PPC64 */
>
> -HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true)
> -HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false)
> -HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true)
> -HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false)
> +HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true, NPHIE)
> +HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false, NPHIE)
> +HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true, PHIE)
> +HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false, PHIE)
> #endif /* CONFIG_TCG */
>
> #if !defined(CONFIG_USER_ONLY)
Otherwise, looks good to me!
regards,
Harsh Prateek Bora
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions
2022-12-11 18:36 ` Harsh Prateek Bora
@ 2022-12-11 21:10 ` Harsh Prateek Bora
0 siblings, 0 replies; 6+ messages in thread
From: Harsh Prateek Bora @ 2022-12-11 21:10 UTC (permalink / raw)
To: Nicholas Miehlbradt, qemu-devel
Cc: qemu-ppc, danielhb413, clg, david, groug, victor.colombo, mikey
On 12/12/22 00:06, Harsh Prateek Bora wrote:
>
>
> On 12/9/22 11:43, Nicholas Miehlbradt wrote:
>> Adds checks to the hashst and hashchk instructions to only execute if
>> enabled by the relevant aspect in the DEXCR and HDEXCR.
>>
>> This behaviour is guarded behind TARGET_PPC64 since Power10 is
>> currently the only implementation which has the DEXCR.
>>
>> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com>
>> ---
>> target/ppc/excp_helper.c | 58 +++++++++++++++++++++++++++++-----------
>> 1 file changed, 43 insertions(+), 15 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 94adcb766b..add4d54ae7 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -2902,29 +2902,57 @@ static uint64_t hash_digest(uint64_t ra,
>> uint64_t rb, uint64_t key)
>> return stage1_h ^ stage1_l;
>> }
>> +static void do_hash(CPUPPCState *env, target_ulong ea, target_ulong ra,
>> + target_ulong rb, uint64_t key, bool store)
>> +{
>> + uint64_t calculated_hash = hash_digest(ra, rb, key), loaded_hash;
>> +
>> + if (store) {
>> + cpu_stq_data_ra(env, ea, calculated_hash, GETPC());
>> + } else {
>> + loaded_hash = cpu_ldq_data_ra(env, ea, GETPC());
>> + if (loaded_hash != calculated_hash) {
>> + raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
>> + POWERPC_EXCP_TRAP, GETPC());
>> + }
>> + }
>> +}
>> +
>> #include "qemu/guest-random.h"
>> -#define HELPER_HASH(op, key,
>> store) \
>> +#ifdef TARGET_PPC64
>> +#define HELPER_HASH(op, key, store,
>> dexcr_aspect) \
>> void helper_##op(CPUPPCState *env, target_ulong ea, target_ulong
>> ra, \
>> target_ulong
>> rb) \
>> {
>
> Conditional compilation could be contained within function, so that
> duplicate lines of code in each macro block could be avoided and would
> look simpler.
Ok, I see that's not feasible within macro expansion. Please ignore.
>
> \
>> - uint64_t calculated_hash = hash_digest(ra, rb, key),
>> loaded_hash; \
>> - \
>> - if (store)
>> { \
>> - cpu_stq_data_ra(env, ea, calculated_hash,
>> GETPC()); \
>> - } else
>> { \
>> - loaded_hash = cpu_ldq_data_ra(env, ea,
>> GETPC()); \
>> - if (loaded_hash != calculated_hash)
>> { \
>> - raise_exception_err_ra(env,
>> POWERPC_EXCP_PROGRAM, \
>> - POWERPC_EXCP_TRAP,
>> GETPC()); \
>> -
>> } \
>> + if (env->msr & R_MSR_PR_MASK)
>> { \
>> + if (!(env->spr[SPR_DEXCR] & R_DEXCR_PRO_##dexcr_aspect##_MASK
>> || \
>> + env->spr[SPR_HDEXCR] &
>> R_HDEXCR_ENF_##dexcr_aspect##_MASK)) \
>> +
>> return; \
>> + } else if (!(env->msr & R_MSR_HV_MASK))
>> { \
>> + if (!(env->spr[SPR_DEXCR] & R_DEXCR_PNH_##dexcr_aspect##_MASK
>> || \
>> + env->spr[SPR_HDEXCR] &
>> R_HDEXCR_ENF_##dexcr_aspect##_MASK)) \
>> +
>> return; \
>> + } else if (!(env->msr & R_MSR_S_MASK))
>> { \
>> + if (!(env->spr[SPR_HDEXCR] &
>> R_HDEXCR_HNU_##dexcr_aspect##_MASK)) \
>> +
>> return; \
>>
>> } \
>> + \
>> + do_hash(env, ea, ra, rb, key,
>> store); \
>> +}
>> +#else
>> +#define HELPER_HASH(op, key, store,
>> dexcr_aspect) \
>> +void helper_##op(CPUPPCState *env, target_ulong ea, target_ulong
>> ra, \
>> + target_ulong
>> rb) \
>> +{ \
>> + do_hash(env, ea, ra, rb, key,
>> store); \
>> }
>> +#endif /* TARGET_PPC64 */
>> -HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true)
>> -HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false)
>> -HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true)
>> -HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false)
>> +HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true, NPHIE)
>> +HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false, NPHIE)
>> +HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true, PHIE)
>> +HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false, PHIE)
>> #endif /* CONFIG_TCG */
>> #if !defined(CONFIG_USER_ONLY)
>
> Otherwise, looks good to me!
>
> regards,
> Harsh Prateek Bora
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-11 21:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-09 6:13 [PATCH v2 0/2] target/ppc: Implement Dynamic Execution Control Registers Nicholas Miehlbradt
2022-12-09 6:13 ` [PATCH v2 1/2] target/ppc: Implement the DEXCR and HDEXCR Nicholas Miehlbradt
2022-12-11 18:28 ` Harsh Prateek Bora
2022-12-09 6:13 ` [PATCH v2 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions Nicholas Miehlbradt
2022-12-11 18:36 ` Harsh Prateek Bora
2022-12-11 21:10 ` Harsh Prateek Bora
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.