* [PATCH 0/2] target/ppc: Implement Dynamic Execution Control Registers
@ 2022-11-24 5:51 Nicholas Miehlbradt
2022-11-24 5:51 ` [PATCH 1/2] target/ppc: Implement the DEXCR and HDEXCR Nicholas Miehlbradt
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Nicholas Miehlbradt @ 2022-11-24 5:51 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, danielhb413, clg, david, groug, victor.colombo,
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.
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 | 9 +++++++
5 files changed, 97 insertions(+), 15 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] target/ppc: Implement the DEXCR and HDEXCR
2022-11-24 5:51 [PATCH 0/2] target/ppc: Implement Dynamic Execution Control Registers Nicholas Miehlbradt
@ 2022-11-24 5:51 ` Nicholas Miehlbradt
2022-11-28 20:33 ` Daniel Henrique Barboza
2022-11-24 5:51 ` [PATCH 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions Nicholas Miehlbradt
2022-11-25 5:48 ` [PATCH 0/2] target/ppc: Implement Dynamic Execution Control Registers Joel Stanley
2 siblings, 1 reply; 6+ messages in thread
From: Nicholas Miehlbradt @ 2022-11-24 5:51 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, danielhb413, clg, david, groug, victor.colombo,
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>
---
target/ppc/cpu.h | 19 +++++++++++++++++++
target/ppc/cpu_init.c | 25 +++++++++++++++++++++++++
target/ppc/spr_common.h | 1 +
target/ppc/translate.c | 9 +++++++++
4 files changed, 54 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..d6b950feb6 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_dexcr,
+ 0);
+
+ spr_register(env, SPR_UDEXCR, "DEXCR",
+ &spr_read_generic, SPR_NOACCESS,
+ &spr_read_generic, SPR_NOACCESS,
+ 0);
+
+ spr_register_hv(env, SPR_HDEXCR, "HDEXCR",
+ SPR_NOACCESS, SPR_NOACCESS,
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_dexcr,
+ 0);
+
+ spr_register(env, SPR_UHDEXCR, "HDEXCR",
+ &spr_read_generic, SPR_NOACCESS,
+ &spr_read_generic, 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..3cfa500250 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_write_dexcr(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..24e9e2fece 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1249,6 +1249,15 @@ 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_write_dexcr(DisasContext *ctx, int sprn, int gprn)
+{
+ TCGv t0 = tcg_temp_new();
+ spr_write_generic(ctx, sprn, gprn);
+ tcg_gen_ext32u_tl(t0, cpu_gpr[gprn]);
+ gen_store_spr(sprn - 16, 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 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions
2022-11-24 5:51 [PATCH 0/2] target/ppc: Implement Dynamic Execution Control Registers Nicholas Miehlbradt
2022-11-24 5:51 ` [PATCH 1/2] target/ppc: Implement the DEXCR and HDEXCR Nicholas Miehlbradt
@ 2022-11-24 5:51 ` Nicholas Miehlbradt
2022-11-28 20:35 ` Daniel Henrique Barboza
2022-11-25 5:48 ` [PATCH 0/2] target/ppc: Implement Dynamic Execution Control Registers Joel Stanley
2 siblings, 1 reply; 6+ messages in thread
From: Nicholas Miehlbradt @ 2022-11-24 5:51 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, danielhb413, clg, david, groug, victor.colombo,
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.
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 0/2] target/ppc: Implement Dynamic Execution Control Registers
2022-11-24 5:51 [PATCH 0/2] target/ppc: Implement Dynamic Execution Control Registers Nicholas Miehlbradt
2022-11-24 5:51 ` [PATCH 1/2] target/ppc: Implement the DEXCR and HDEXCR Nicholas Miehlbradt
2022-11-24 5:51 ` [PATCH 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions Nicholas Miehlbradt
@ 2022-11-25 5:48 ` Joel Stanley
2 siblings, 0 replies; 6+ messages in thread
From: Joel Stanley @ 2022-11-25 5:48 UTC (permalink / raw)
To: Nicholas Miehlbradt
Cc: qemu-devel, qemu-ppc, danielhb413, clg, david, groug, victor.colombo
Hi Nick,
On Thu, 24 Nov 2022 at 05:53, Nicholas Miehlbradt
<nicholas@linux.ibm.com> wrote:
>
> 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.
I had a look at these and they appear to follow the style of the
existing code. I am no expert on the target code though!
It might be worth mentioning to reviewers that these registers will be
exercised by the Linux kernel with some upcoming patches that you're
developing.
Cheers,
Joel
>
> 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 | 9 +++++++
> 5 files changed, 97 insertions(+), 15 deletions(-)
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] target/ppc: Implement the DEXCR and HDEXCR
2022-11-24 5:51 ` [PATCH 1/2] target/ppc: Implement the DEXCR and HDEXCR Nicholas Miehlbradt
@ 2022-11-28 20:33 ` Daniel Henrique Barboza
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Henrique Barboza @ 2022-11-28 20:33 UTC (permalink / raw)
To: Nicholas Miehlbradt, qemu-devel
Cc: qemu-ppc, clg, david, groug, victor.colombo
On 11/24/22 02:51, 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>
> ---
> target/ppc/cpu.h | 19 +++++++++++++++++++
> target/ppc/cpu_init.c | 25 +++++++++++++++++++++++++
> target/ppc/spr_common.h | 1 +
> target/ppc/translate.c | 9 +++++++++
> 4 files changed, 54 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..d6b950feb6 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_dexcr,
> + 0);
> +
> + spr_register(env, SPR_UDEXCR, "DEXCR",
> + &spr_read_generic, SPR_NOACCESS,
> + &spr_read_generic, SPR_NOACCESS,
> + 0);
> +
> + spr_register_hv(env, SPR_HDEXCR, "HDEXCR",
> + SPR_NOACCESS, SPR_NOACCESS,
> + SPR_NOACCESS, SPR_NOACCESS,
> + &spr_read_generic, &spr_write_dexcr,
> + 0);
> +
> + spr_register(env, SPR_UHDEXCR, "HDEXCR",
> + &spr_read_generic, SPR_NOACCESS,
> + &spr_read_generic, 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..3cfa500250 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_write_dexcr(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..24e9e2fece 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1249,6 +1249,15 @@ 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_write_dexcr(DisasContext *ctx, int sprn, int gprn)
> +{
> + TCGv t0 = tcg_temp_new();
> + spr_write_generic(ctx, sprn, gprn);
> + tcg_gen_ext32u_tl(t0, cpu_gpr[gprn]);
> + gen_store_spr(sprn - 16, t0);
> + tcg_temp_free(t0);
> +}
IIUC you're writing sprn as is, then zeroing bits 0-31 and writing it to
sprn - 16, because of this comment in PowerISA:
-------------
This register can be read and written in privileged state
using SPR 828. Bits 32:63 of the register can only be
read in problem state using SPR 812; mfspr 812
returns 0s for bits 0:31.
-------------
I am no longer familiar with Power10 internals and can't say whether
sprn 812/455 is writable in problem state. You didn't create a write
function for it, so I'll assume that problem state can't write sprn
812/455 and can read only the higher bits.
I'll note that there's another way of doing it: instead of doing two
writes and clearing 32 bits every time sprn 828/471 is written, you can
also just do a regular write and, instead, implement spr_read_dexcr_ureg
that would clear bits 0-31 when problem state reads it. Depending on
how frequent problem state reads the reg versus priv state writes it,
we'll spare a few cycles in the emulation. I did something similar in
spr_read_MMCR0_ureg if you want to take a look.
All that said, what is being done here also works. I'll just ask you to
put a comment in spr_write_dexcr() explaining why you're clearing bits
and writing another sprn-16 in it.
Thanks,
Daniel
> #endif
>
> #define GEN_HANDLER(name, opc1, opc2, opc3, inval, type) \
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions
2022-11-24 5:51 ` [PATCH 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions Nicholas Miehlbradt
@ 2022-11-28 20:35 ` Daniel Henrique Barboza
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Henrique Barboza @ 2022-11-28 20:35 UTC (permalink / raw)
To: Nicholas Miehlbradt, qemu-devel
Cc: qemu-ppc, clg, david, groug, victor.colombo
On 11/24/22 02:51, 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.
>
> Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com>
> ---
LGTM
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.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)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-11-28 20:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-24 5:51 [PATCH 0/2] target/ppc: Implement Dynamic Execution Control Registers Nicholas Miehlbradt
2022-11-24 5:51 ` [PATCH 1/2] target/ppc: Implement the DEXCR and HDEXCR Nicholas Miehlbradt
2022-11-28 20:33 ` Daniel Henrique Barboza
2022-11-24 5:51 ` [PATCH 2/2] target/ppc: Check DEXCR on hash{st, chk} instructions Nicholas Miehlbradt
2022-11-28 20:35 ` Daniel Henrique Barboza
2022-11-25 5:48 ` [PATCH 0/2] target/ppc: Implement Dynamic Execution Control Registers Joel Stanley
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.