hw/ppc/spapr.c | 7 ++- hw/ppc/spapr_caps.c | 35 ++++++++++++++ hw/ppc/spapr_hcall.c | 50 ++++++++++++-------- include/hw/ppc/spapr.h | 6 ++ target/ppc/cpu.c | 114 +++++++++++++++++++++++++--------------------- target/ppc/cpu.h | 6 ++ target/ppc/cpu_init.c | 15 ++++++ target/ppc/excp_helper.c | 61 ++++++++++++++----------- target/ppc/helper.h | 2 + target/ppc/kvm.c | 12 +++++ target/ppc/kvm_ppc.h | 12 +++++ target/ppc/machine.c | 1 target/ppc/misc_helper.c | 20 ++++++-- target/ppc/spr_common.h | 2 + target/ppc/translate.c | 25 +++++++--- 15 files changed, 253 insertions(+), 115 deletions(-)
Extend the existing watchpoint facility from TCG DAWR0 emulation
to DAWR1 on POWER10.
As per the PAPR, bit 0 of byte 64 in pa-features property
indicates availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to find
whether kvm supports 2nd DAWR or not. If it's supported, allow user to set
the pa-feature bit in guest DT using cap-dawr1 machine capability.
Signed-off-by: Ravi Bangoria <ravi.bangoria at linux.ibm.com>
Signed-off-by: Shivaprasad G Bhat <sbhat at linux.ibm.com>
---
Changelog:
v6: https://lore.kernel.org/qemu-devel/168871963321.58984.15628382614621248470.stgit@ltcd89-lp2/
v6->v7:
- Sorry about the delay in sending out this version, I have dropped the
Reviewed-bys as suggested and converted the patch to RFC back again.
- Added the TCG support. Basically, converted the existing DAWR0 support
routines into macros for reuse by the DAWR1. Let me know if the macro
conversions should be moved to a separate independent patch.
- As the dawr1 works on TCG, the checks in cap_dawr1_apply() report a warning
now only for P9 or P9 compat modes for both KVM and TCG use cases.
- 'make test' passes for caps checks. Also, as suggested by Greg Kurz, the
'make test' after making the DAWR1 default 'on' and updating defaut cpu
to Power10, shows no failures.
v5: https://lore.kernel.org/all/20210412114433.129702-1-ravi.bangoria@linux.ibm.com/
v5->v6:
- The other patches in the original series already merged.
- Rebased to the top of the tree. So, the gen_spr_book3s_310_dbg() is renamed
to register_book3s_310_dbg_sprs() and moved to cpu_init.c accordingly.
- No functional changes.
v4: https://lore.kernel.org/r/20210406053833.282907-1-ravi.bangoria@linux.ibm.com
v3->v4:
- Make error message more proper.
v3: https://lore.kernel.org/r/20210330095350.36309-1-ravi.bangoria@linux.ibm.com
v3->v4:
- spapr_dt_pa_features(): POWER10 processor is compatible with 3.0
(PCR_COMPAT_3_00). No need to ppc_check_compat(3_10) for now as
ppc_check_compati(3_00) will also be true. ppc_check_compat(3_10)
can be added while introducing pa_features_310 in future.
- Use error_append_hint() for hints. Also add ERRP_GUARD().
- Add kvmppc_set_cap_dawr1() stub function for CONFIG_KVM=n.
v2: https://lore.kernel.org/r/20210329041906.213991-1-ravi.bangoria@linux.ibm.com
v2->v3:
- Don't introduce pa_features_310[], instead, reuse pa_features_300[]
for 3.1 guests, as there is no difference between initial values of
them atm.
- Call gen_spr_book3s_310_dbg() from init_proc_POWER10() instead of
init_proc_POWER8(). Also, Don't call gen_spr_book3s_207_dbg() from
gen_spr_book3s_310_dbg() as init_proc_POWER10() already calls it.
v1: https://lore.kernel.org/r/20200723104220.314671-1-ravi.bangoria@linux.ibm.com
v1->v2:
- Introduce machine capability cap-dawr1 to enable/disable
the feature. By default, 2nd DAWR is OFF for guests even
when host kvm supports it. User has to manually enable it
with -machine cap-dawr1=on if he wishes to use it.
- Split the header file changes into separate patch. (Sync
headers from v5.12-rc3)
hw/ppc/spapr.c | 7 ++-
hw/ppc/spapr_caps.c | 35 ++++++++++++++
hw/ppc/spapr_hcall.c | 50 ++++++++++++--------
include/hw/ppc/spapr.h | 6 ++
target/ppc/cpu.c | 114 +++++++++++++++++++++++++---------------------
target/ppc/cpu.h | 6 ++
target/ppc/cpu_init.c | 15 ++++++
target/ppc/excp_helper.c | 61 ++++++++++++++-----------
target/ppc/helper.h | 2 +
target/ppc/kvm.c | 12 +++++
target/ppc/kvm_ppc.h | 12 +++++
target/ppc/machine.c | 1
target/ppc/misc_helper.c | 20 ++++++--
target/ppc/spr_common.h | 2 +
target/ppc/translate.c | 25 +++++++---
15 files changed, 253 insertions(+), 115 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index df09aa9d6a..c1cb47464b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -262,7 +262,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
/* 54: DecFP, 56: DecI, 58: SHA */
0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
- /* 60: NM atomic, 62: RNG */
+ /* 60: NM atomic, 62: RNG, 64: DAWR1 (ISA 3.1) */
0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
};
uint8_t *pa_features = NULL;
@@ -303,6 +303,9 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
* in pa-features. So hide it from them. */
pa_features[40 + 2] &= ~0x80; /* Radix MMU */
}
+ if (spapr_get_cap(spapr, SPAPR_CAP_DAWR1)) {
+ pa_features[66] |= 0x80;
+ }
_FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
}
@@ -2138,6 +2141,7 @@ static const VMStateDescription vmstate_spapr = {
&vmstate_spapr_cap_fwnmi,
&vmstate_spapr_fwnmi,
&vmstate_spapr_cap_rpt_invalidate,
+ &vmstate_spapr_cap_dawr1,
NULL
}
};
@@ -4717,6 +4721,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
smc->default_caps.caps[SPAPR_CAP_RPT_INVALIDATE] = SPAPR_CAP_OFF;
+ smc->default_caps.caps[SPAPR_CAP_DAWR1] = SPAPR_CAP_OFF;
/*
* This cap specifies whether the AIL 3 mode for
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 5a0755d34f..e265b56711 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -655,6 +655,31 @@ static void cap_ail_mode_3_apply(SpaprMachineState *spapr,
}
}
+static void cap_dawr1_apply(SpaprMachineState *spapr, uint8_t val,
+ Error **errp)
+{
+ ERRP_GUARD();
+
+ if (!val) {
+ return; /* Disable by default */
+ }
+
+ if (!ppc_type_check_compat(MACHINE(spapr)->cpu_type, CPU_POWERPC_LOGICAL_3_10,
+ 0, spapr->max_compat_pvr)) {
+ warn_report("DAWR1 supported only on POWER10 and later CPUs");
+ }
+
+ if (kvm_enabled()) {
+ if (!kvmppc_has_cap_dawr1()) {
+ error_setg(errp, "DAWR1 not supported by KVM.");
+ error_append_hint(errp, "Try appending -machine cap-dawr1=off");
+ } else if (kvmppc_set_cap_dawr1(val) < 0) {
+ error_setg(errp, "Error enabling cap-dawr1 with KVM.");
+ error_append_hint(errp, "Try appending -machine cap-dawr1=off");
+ }
+ }
+}
+
SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
[SPAPR_CAP_HTM] = {
.name = "htm",
@@ -781,6 +806,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
.type = "bool",
.apply = cap_ail_mode_3_apply,
},
+ [SPAPR_CAP_DAWR1] = {
+ .name = "dawr1",
+ .description = "Allow 2nd Data Address Watchpoint Register (DAWR1)",
+ .index = SPAPR_CAP_DAWR1,
+ .get = spapr_cap_get_bool,
+ .set = spapr_cap_set_bool,
+ .type = "bool",
+ .apply = cap_dawr1_apply,
+ },
};
static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
@@ -923,6 +957,7 @@ SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI);
SPAPR_CAP_MIG_STATE(rpt_invalidate, SPAPR_CAP_RPT_INVALIDATE);
+SPAPR_CAP_MIG_STATE(dawr1, SPAPR_CAP_DAWR1);
void spapr_caps_init(SpaprMachineState *spapr)
{
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 522a2396c7..50bb305917 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -815,29 +815,33 @@ static target_ulong h_set_mode_resource_set_ciabr(PowerPCCPU *cpu,
return H_SUCCESS;
}
-static target_ulong h_set_mode_resource_set_dawr0(PowerPCCPU *cpu,
- SpaprMachineState *spapr,
- target_ulong mflags,
- target_ulong value1,
- target_ulong value2)
-{
- CPUPPCState *env = &cpu->env;
-
- assert(tcg_enabled()); /* KVM will have handled this */
-
- if (mflags) {
- return H_UNSUPPORTED_FLAG;
- }
- if (value2 & PPC_BIT(61)) {
- return H_P4;
- }
-
- ppc_store_dawr0(env, value1);
- ppc_store_dawrx0(env, value2);
-
- return H_SUCCESS;
+#define DEF_H_SET_MODE_RESOURCE_SET_DAWR(id) \
+static target_ulong h_set_mode_resource_set_dawr##id(PowerPCCPU *cpu, \
+ SpaprMachineState *spapr,\
+ target_ulong mflags, \
+ target_ulong value1, \
+ target_ulong value2) \
+{ \
+ CPUPPCState *env = &cpu->env; \
+ \
+ assert(tcg_enabled()); /* KVM will have handled this */ \
+ \
+ if (mflags) { \
+ return H_UNSUPPORTED_FLAG; \
+ } \
+ if (value2 & PPC_BIT(61)) { \
+ return H_P4; \
+ } \
+ \
+ ppc_store_dawr##id(env, value1); \
+ ppc_store_dawrx##id(env, value2); \
+ \
+ return H_SUCCESS; \
}
+DEF_H_SET_MODE_RESOURCE_SET_DAWR(0)
+DEF_H_SET_MODE_RESOURCE_SET_DAWR(1)
+
static target_ulong h_set_mode_resource_le(PowerPCCPU *cpu,
SpaprMachineState *spapr,
target_ulong mflags,
@@ -915,6 +919,10 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, SpaprMachineState *spapr,
ret = h_set_mode_resource_set_dawr0(cpu, spapr, args[0], args[2],
args[3]);
break;
+ case H_SET_MODE_RESOURCE_SET_DAWR1:
+ ret = h_set_mode_resource_set_dawr1(cpu, spapr, args[0], args[2],
+ args[3]);
+ break;
case H_SET_MODE_RESOURCE_LE:
ret = h_set_mode_resource_le(cpu, spapr, args[0], args[2], args[3]);
break;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index e91791a1a9..2b13c9a00e 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -80,8 +80,10 @@ typedef enum {
#define SPAPR_CAP_RPT_INVALIDATE 0x0B
/* Support for AIL modes */
#define SPAPR_CAP_AIL_MODE_3 0x0C
+/* DAWR1 */
+#define SPAPR_CAP_DAWR1 0x0D
/* Num Caps */
-#define SPAPR_CAP_NUM (SPAPR_CAP_AIL_MODE_3 + 1)
+#define SPAPR_CAP_NUM (SPAPR_CAP_DAWR1 + 1)
/*
* Capability Values
@@ -403,6 +405,7 @@ struct SpaprMachineState {
#define H_SET_MODE_RESOURCE_SET_DAWR0 2
#define H_SET_MODE_RESOURCE_ADDR_TRANS_MODE 3
#define H_SET_MODE_RESOURCE_LE 4
+#define H_SET_MODE_RESOURCE_SET_DAWR1 5
/* Flags for H_SET_MODE_RESOURCE_LE */
#define H_SET_MODE_ENDIAN_BIG 0
@@ -986,6 +989,7 @@ extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
extern const VMStateDescription vmstate_spapr_cap_fwnmi;
extern const VMStateDescription vmstate_spapr_cap_rpt_invalidate;
extern const VMStateDescription vmstate_spapr_wdt;
+extern const VMStateDescription vmstate_spapr_cap_dawr1;
static inline uint8_t spapr_get_cap(SpaprMachineState *spapr, int cap)
{
diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index e3ad8e0c27..3e2fe93074 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -130,64 +130,76 @@ void ppc_store_ciabr(CPUPPCState *env, target_ulong val)
ppc_update_ciabr(env);
}
-void ppc_update_daw0(CPUPPCState *env)
-{
- CPUState *cs = env_cpu(env);
- target_ulong deaw = env->spr[SPR_DAWR0] & PPC_BITMASK(0, 60);
- uint32_t dawrx = env->spr[SPR_DAWRX0];
- int mrd = extract32(dawrx, PPC_BIT_NR(48), 54 - 48);
- bool dw = extract32(dawrx, PPC_BIT_NR(57), 1);
- bool dr = extract32(dawrx, PPC_BIT_NR(58), 1);
- bool hv = extract32(dawrx, PPC_BIT_NR(61), 1);
- bool sv = extract32(dawrx, PPC_BIT_NR(62), 1);
- bool pr = extract32(dawrx, PPC_BIT_NR(62), 1);
- vaddr len;
- int flags;
-
- if (env->dawr0_watchpoint) {
- cpu_watchpoint_remove_by_ref(cs, env->dawr0_watchpoint);
- env->dawr0_watchpoint = NULL;
- }
-
- if (!dr && !dw) {
- return;
- }
-
- if (!hv && !sv && !pr) {
- return;
- }
-
- len = (mrd + 1) * 8;
- flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
- if (dr) {
- flags |= BP_MEM_READ;
- }
- if (dw) {
- flags |= BP_MEM_WRITE;
- }
+#define DEF_PPC_UPDATE_DAW(id) \
+void ppc_update_daw##id(CPUPPCState *env) \
+{ \
+ CPUState *cs = env_cpu(env); \
+ target_ulong deaw = env->spr[SPR_DAWR##id] & PPC_BITMASK(0, 60); \
+ uint32_t dawrx = env->spr[SPR_DAWRX##id]; \
+ int mrd = extract32(dawrx, PPC_BIT_NR(48), 54 - 48); \
+ bool dw = extract32(dawrx, PPC_BIT_NR(57), 1); \
+ bool dr = extract32(dawrx, PPC_BIT_NR(58), 1); \
+ bool hv = extract32(dawrx, PPC_BIT_NR(61), 1); \
+ bool sv = extract32(dawrx, PPC_BIT_NR(62), 1); \
+ bool pr = extract32(dawrx, PPC_BIT_NR(62), 1); \
+ vaddr len; \
+ int flags; \
+ \
+ if (env->dawr##id##_watchpoint) { \
+ cpu_watchpoint_remove_by_ref(cs, env->dawr##id##_watchpoint); \
+ env->dawr##id##_watchpoint = NULL; \
+ } \
+ \
+ if (!dr && !dw) { \
+ return; \
+ } \
+ \
+ if (!hv && !sv && !pr) { \
+ return; \
+ } \
+ \
+ len = (mrd + 1) * 8; \
+ flags = BP_CPU | BP_STOP_BEFORE_ACCESS; \
+ if (dr) { \
+ flags |= BP_MEM_READ; \
+ } \
+ if (dw) { \
+ flags |= BP_MEM_WRITE; \
+ } \
+ \
+ cpu_watchpoint_insert(cs, deaw, len, flags, &env->dawr##id##_watchpoint); \
+}
- cpu_watchpoint_insert(cs, deaw, len, flags, &env->dawr0_watchpoint);
+#define DEF_PPC_STORE_DAWR(id) \
+void ppc_store_dawr##id(CPUPPCState *env, target_ulong val) \
+{ \
+ env->spr[SPR_DAWR##id] = val; \
+ ppc_update_daw##id(env); \
}
-void ppc_store_dawr0(CPUPPCState *env, target_ulong val)
-{
- env->spr[SPR_DAWR0] = val;
- ppc_update_daw0(env);
+#define DEF_PPC_STORE_DAWRX(id) \
+void ppc_store_dawrx##id(CPUPPCState *env, uint32_t val) \
+{ \
+ int hrammc = extract32(val, PPC_BIT_NR(56), 1); \
+ \
+ if (hrammc) { \
+ /* This might be done with a 2nd watchpoint at the xor of DEAW[0] */ \
+ qemu_log_mask(LOG_UNIMP, "%s: DAWRX##id[HRAMMC] is unimplemented\n", \
+ __func__); \
+ } \
+ \
+ env->spr[SPR_DAWRX##id] = val; \
+ ppc_update_daw##id(env); \
}
-void ppc_store_dawrx0(CPUPPCState *env, uint32_t val)
-{
- int hrammc = extract32(val, PPC_BIT_NR(56), 1);
+DEF_PPC_UPDATE_DAW(0)
+DEF_PPC_STORE_DAWR(0)
+DEF_PPC_STORE_DAWRX(0)
- if (hrammc) {
- /* This might be done with a second watchpoint at the xor of DEAW[0] */
- qemu_log_mask(LOG_UNIMP, "%s: DAWRX0[HRAMMC] is unimplemented\n",
- __func__);
- }
+DEF_PPC_UPDATE_DAW(1)
+DEF_PPC_STORE_DAWR(1)
+DEF_PPC_STORE_DAWRX(1)
- env->spr[SPR_DAWRX0] = val;
- ppc_update_daw0(env);
-}
#endif
#endif
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index f8101ffa29..ab34fc7b72 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1237,6 +1237,7 @@ struct CPUArchState {
ppc_slb_t slb[MAX_SLB_ENTRIES]; /* PowerPC 64 SLB area */
struct CPUBreakpoint *ciabr_breakpoint;
struct CPUWatchpoint *dawr0_watchpoint;
+ struct CPUWatchpoint *dawr1_watchpoint;
#endif
target_ulong sr[32]; /* segment registers */
uint32_t nb_BATs; /* number of BATs */
@@ -1552,6 +1553,9 @@ void ppc_store_ciabr(CPUPPCState *env, target_ulong value);
void ppc_update_daw0(CPUPPCState *env);
void ppc_store_dawr0(CPUPPCState *env, target_ulong value);
void ppc_store_dawrx0(CPUPPCState *env, uint32_t value);
+void ppc_update_daw1(CPUPPCState *env);
+void ppc_store_dawr1(CPUPPCState *env, target_ulong value);
+void ppc_store_dawrx1(CPUPPCState *env, uint32_t value);
#endif /* !defined(CONFIG_USER_ONLY) */
void ppc_store_msr(CPUPPCState *env, target_ulong value);
@@ -1737,9 +1741,11 @@ void ppc_compat_add_property(Object *obj, const char *name,
#define SPR_PSPB (0x09F)
#define SPR_DPDES (0x0B0)
#define SPR_DAWR0 (0x0B4)
+#define SPR_DAWR1 (0x0B5)
#define SPR_RPR (0x0BA)
#define SPR_CIABR (0x0BB)
#define SPR_DAWRX0 (0x0BC)
+#define SPR_DAWRX1 (0x0BD)
#define SPR_HFSCR (0x0BE)
#define SPR_VRSAVE (0x100)
#define SPR_USPRG0 (0x100)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 40fe14a6c2..27a73bc7be 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5131,6 +5131,20 @@ static void register_book3s_207_dbg_sprs(CPUPPCState *env)
KVM_REG_PPC_CIABR, 0x00000000);
}
+static void register_book3s_310_dbg_sprs(CPUPPCState *env)
+{
+ spr_register_kvm_hv(env, SPR_DAWR1, "DAWR1",
+ SPR_NOACCESS, SPR_NOACCESS,
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_dawr1,
+ KVM_REG_PPC_DAWR1, 0x00000000);
+ spr_register_kvm_hv(env, SPR_DAWRX1, "DAWRX1",
+ SPR_NOACCESS, SPR_NOACCESS,
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_dawrx1,
+ KVM_REG_PPC_DAWRX1, 0x00000000);
+}
+
static void register_970_dbg_sprs(CPUPPCState *env)
{
/* Breakpoints */
@@ -6473,6 +6487,7 @@ static void init_proc_POWER10(CPUPPCState *env)
/* Common Registers */
init_proc_book3s_common(env);
register_book3s_207_dbg_sprs(env);
+ register_book3s_310_dbg_sprs(env);
/* Common TCG PMU */
init_tcg_pmu_power8(env);
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index a42743a3e0..484abb672d 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -3314,39 +3314,46 @@ bool ppc_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
{
#if defined(TARGET_PPC64)
CPUPPCState *env = cpu_env(cs);
+ bool wt, wti, hv, sv, pr;
+ uint32_t dawrx;
+
+ if ((env->insns_flags2 & PPC2_ISA207S) &&
+ (wp == env->dawr0_watchpoint)) {
+ dawrx = env->spr[SPR_DAWRX0];
+ } else if ((env->insns_flags2 & PPC2_ISA310) &&
+ (wp == env->dawr1_watchpoint)) {
+ dawrx = env->spr[SPR_DAWRX1];
+ } else {
+ return false;
+ }
- if (env->insns_flags2 & PPC2_ISA207S) {
- if (wp == env->dawr0_watchpoint) {
- uint32_t dawrx = env->spr[SPR_DAWRX0];
- bool wt = extract32(dawrx, PPC_BIT_NR(59), 1);
- bool wti = extract32(dawrx, PPC_BIT_NR(60), 1);
- bool hv = extract32(dawrx, PPC_BIT_NR(61), 1);
- bool sv = extract32(dawrx, PPC_BIT_NR(62), 1);
- bool pr = extract32(dawrx, PPC_BIT_NR(62), 1);
-
- if ((env->msr & ((target_ulong)1 << MSR_PR)) && !pr) {
- return false;
- } else if ((env->msr & ((target_ulong)1 << MSR_HV)) && !hv) {
- return false;
- } else if (!sv) {
+ wt = extract32(dawrx, PPC_BIT_NR(59), 1);
+ wti = extract32(dawrx, PPC_BIT_NR(60), 1);
+ hv = extract32(dawrx, PPC_BIT_NR(61), 1);
+ sv = extract32(dawrx, PPC_BIT_NR(62), 1);
+ pr = extract32(dawrx, PPC_BIT_NR(62), 1);
+
+ if ((env->msr & ((target_ulong)1 << MSR_PR)) && !pr) {
+ return false;
+ } else if ((env->msr & ((target_ulong)1 << MSR_HV)) && !hv) {
+ return false;
+ } else if (!sv) {
+ return false;
+ }
+
+ if (!wti) {
+ if (env->msr & ((target_ulong)1 << MSR_DR)) {
+ if (!wt) {
return false;
}
-
- if (!wti) {
- if (env->msr & ((target_ulong)1 << MSR_DR)) {
- if (!wt) {
- return false;
- }
- } else {
- if (wt) {
- return false;
- }
- }
+ } else {
+ if (wt) {
+ return false;
}
-
- return true;
}
}
+
+ return true;
#endif
return false;
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 86f97ee1e7..0c008bb725 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -28,6 +28,8 @@ DEF_HELPER_2(store_pcr, void, env, tl)
DEF_HELPER_2(store_ciabr, void, env, tl)
DEF_HELPER_2(store_dawr0, void, env, tl)
DEF_HELPER_2(store_dawrx0, void, env, tl)
+DEF_HELPER_2(store_dawr1, void, env, tl)
+DEF_HELPER_2(store_dawrx1, void, env, tl)
DEF_HELPER_2(store_mmcr0, void, env, tl)
DEF_HELPER_2(store_mmcr1, void, env, tl)
DEF_HELPER_3(store_pmc, void, env, i32, i64)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 9b1abe2fc4..09a8115380 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -89,6 +89,7 @@ static int cap_large_decr;
static int cap_fwnmi;
static int cap_rpt_invalidate;
static int cap_ail_mode_3;
+static int cap_dawr1;
static uint32_t debug_inst_opcode;
@@ -143,6 +144,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
cap_ppc_nested_kvm_hv = kvm_vm_check_extension(s, KVM_CAP_PPC_NESTED_HV);
cap_large_decr = kvmppc_get_dec_bits();
cap_fwnmi = kvm_vm_check_extension(s, KVM_CAP_PPC_FWNMI);
+ cap_dawr1 = kvm_vm_check_extension(s, KVM_CAP_PPC_DAWR1);
/*
* Note: setting it to false because there is not such capability
* in KVM at this moment.
@@ -2109,6 +2111,16 @@ int kvmppc_set_fwnmi(PowerPCCPU *cpu)
return kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
}
+bool kvmppc_has_cap_dawr1(void)
+{
+ return !!cap_dawr1;
+}
+
+int kvmppc_set_cap_dawr1(int enable)
+{
+ return kvm_vm_enable_cap(kvm_state, KVM_CAP_PPC_DAWR1, 0, enable);
+}
+
int kvmppc_smt_threads(void)
{
return cap_ppc_smt ? cap_ppc_smt : 1;
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 1975fb5ee6..493d6bb477 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -68,6 +68,8 @@ bool kvmppc_has_cap_htm(void);
bool kvmppc_has_cap_mmu_radix(void);
bool kvmppc_has_cap_mmu_hash_v3(void);
bool kvmppc_has_cap_xive(void);
+bool kvmppc_has_cap_dawr1(void);
+int kvmppc_set_cap_dawr1(int enable);
int kvmppc_get_cap_safe_cache(void);
int kvmppc_get_cap_safe_bounds_check(void);
int kvmppc_get_cap_safe_indirect_branch(void);
@@ -377,6 +379,16 @@ static inline bool kvmppc_has_cap_xive(void)
return false;
}
+static inline bool kvmppc_has_cap_dawr1(void)
+{
+ return false;
+}
+
+static inline int kvmppc_set_cap_dawr1(int enable)
+{
+ abort();
+}
+
static inline int kvmppc_get_cap_safe_cache(void)
{
return 0;
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 68cbdffecd..eef596dbdc 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -326,6 +326,7 @@ static int cpu_post_load(void *opaque, int version_id)
#if defined(TARGET_PPC64)
ppc_update_ciabr(env);
ppc_update_daw0(env);
+ ppc_update_daw1(env);
#endif
/*
* TCG needs to re-start the decrementer timer and/or raise the
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index a05bdf78c9..022b984e00 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -204,16 +204,24 @@ void helper_store_ciabr(CPUPPCState *env, target_ulong value)
ppc_store_ciabr(env, value);
}
-void helper_store_dawr0(CPUPPCState *env, target_ulong value)
-{
- ppc_store_dawr0(env, value);
+#define HELPER_STORE_DAWR(id) \
+void helper_store_dawr##id(CPUPPCState *env, target_ulong value) \
+{ \
+ env->spr[SPR_DAWR##id] = value; \
}
-void helper_store_dawrx0(CPUPPCState *env, target_ulong value)
-{
- ppc_store_dawrx0(env, value);
+#define HELPER_STORE_DAWRX(id) \
+void helper_store_dawrx##id(CPUPPCState *env, target_ulong value) \
+{ \
+ env->spr[SPR_DAWRX##id] = value; \
}
+HELPER_STORE_DAWR(0)
+HELPER_STORE_DAWRX(0)
+
+HELPER_STORE_DAWR(1)
+HELPER_STORE_DAWRX(1)
+
/*
* DPDES register is shared. Each bit reflects the state of the
* doorbell interrupt of a thread of the same core.
diff --git a/target/ppc/spr_common.h b/target/ppc/spr_common.h
index 8a9d6cd994..c987a50809 100644
--- a/target/ppc/spr_common.h
+++ b/target/ppc/spr_common.h
@@ -162,6 +162,8 @@ void spr_write_cfar(DisasContext *ctx, int sprn, int gprn);
void spr_write_ciabr(DisasContext *ctx, int sprn, int gprn);
void spr_write_dawr0(DisasContext *ctx, int sprn, int gprn);
void spr_write_dawrx0(DisasContext *ctx, int sprn, int gprn);
+void spr_write_dawr1(DisasContext *ctx, int sprn, int gprn);
+void spr_write_dawrx1(DisasContext *ctx, int sprn, int gprn);
void spr_write_ureg(DisasContext *ctx, int sprn, int gprn);
void spr_read_purr(DisasContext *ctx, int gprn, int sprn);
void spr_write_purr(DisasContext *ctx, int sprn, int gprn);
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 329da4d518..4d41628c04 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -582,17 +582,26 @@ void spr_write_ciabr(DisasContext *ctx, int sprn, int gprn)
}
/* Watchpoint */
-void spr_write_dawr0(DisasContext *ctx, int sprn, int gprn)
-{
- translator_io_start(&ctx->base);
- gen_helper_store_dawr0(tcg_env, cpu_gpr[gprn]);
+#define SPR_WRITE_DAWR(id) \
+void spr_write_dawr##id(DisasContext *ctx, int sprn, int gprn) \
+{ \
+ translator_io_start(&ctx->base); \
+ gen_helper_store_dawr##id(tcg_env, cpu_gpr[gprn]); \
}
-void spr_write_dawrx0(DisasContext *ctx, int sprn, int gprn)
-{
- translator_io_start(&ctx->base);
- gen_helper_store_dawrx0(tcg_env, cpu_gpr[gprn]);
+#define SPR_WRITE_DAWRX(id) \
+void spr_write_dawrx##id(DisasContext *ctx, int sprn, int gprn) \
+{ \
+ translator_io_start(&ctx->base); \
+ gen_helper_store_dawrx##id(tcg_env, cpu_gpr[gprn]); \
}
+
+SPR_WRITE_DAWR(0)
+SPR_WRITE_DAWRX(0)
+
+SPR_WRITE_DAWR(1)
+SPR_WRITE_DAWRX(1)
+
#endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */
/* CTR */On Wed Nov 22, 2023 at 5:32 PM AEST, Shivaprasad G Bhat wrote:
> Extend the existing watchpoint facility from TCG DAWR0 emulation
> to DAWR1 on POWER10.
>
> As per the PAPR, bit 0 of byte 64 in pa-features property
> indicates availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
> DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to find
> whether kvm supports 2nd DAWR or not. If it's supported, allow user to set
> the pa-feature bit in guest DT using cap-dawr1 machine capability.
Sorry for the late review.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria at linux.ibm.com>
> Signed-off-by: Shivaprasad G Bhat <sbhat at linux.ibm.com>
> ---
> Changelog:
> v6: https://lore.kernel.org/qemu-devel/168871963321.58984.15628382614621248470.stgit@ltcd89-lp2/
> v6->v7:
> - Sorry about the delay in sending out this version, I have dropped the
> Reviewed-bys as suggested and converted the patch to RFC back again.
> - Added the TCG support. Basically, converted the existing DAWR0 support
> routines into macros for reuse by the DAWR1. Let me know if the macro
> conversions should be moved to a separate independent patch.
I don't really like the macros. I have nightmares from Linux going
overboard with defining functions using spaghetti of generator macros.
Could you just make most functions accept either SPR number or number
(0, 1), or simply use if/else, to select between them?
Splitting the change in 2 would be good, first add regs + TCG, then the
spapr bits.
[snip]
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index a05bdf78c9..022b984e00 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -204,16 +204,24 @@ void helper_store_ciabr(CPUPPCState *env, target_ulong value)
> ppc_store_ciabr(env, value);
> }
>
> -void helper_store_dawr0(CPUPPCState *env, target_ulong value)
> -{
> - ppc_store_dawr0(env, value);
> +#define HELPER_STORE_DAWR(id) \
> +void helper_store_dawr##id(CPUPPCState *env, target_ulong value) \
> +{ \
> + env->spr[SPR_DAWR##id] = value; \
> }
>
> -void helper_store_dawrx0(CPUPPCState *env, target_ulong value)
> -{
> - ppc_store_dawrx0(env, value);
> +#define HELPER_STORE_DAWRX(id) \
> +void helper_store_dawrx##id(CPUPPCState *env, target_ulong value) \
> +{ \
> + env->spr[SPR_DAWRX##id] = value; \
> }
Did we lose the calls to ppc_store_dawr*? That will
break direct register access (i.e., powernv) if so.
>
> +HELPER_STORE_DAWR(0)
> +HELPER_STORE_DAWRX(0)
> +
> +HELPER_STORE_DAWR(1)
> +HELPER_STORE_DAWRX(1)
I would say open-code all these too instead of generating. If we
ever grew to >= 4 of them maybe, but as is this saves 2 lines,
and makes 'helper_store_dawrx0' more difficult to grep for.
Thanks,
Nick
Thanks for the review Nick!
On 1/23/24 17:36, Nicholas Piggin wrote:
> On Wed Nov 22, 2023 at 5:32 PM AEST, Shivaprasad G Bhat wrote:
>> Extend the existing watchpoint facility from TCG DAWR0 emulation
>> to DAWR1 on POWER10.
>>
>> As per the PAPR, bit 0 of byte 64 in pa-features property
>> indicates availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
>> DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to find
>> whether kvm supports 2nd DAWR or not. If it's supported, allow user to set
>> the pa-feature bit in guest DT using cap-dawr1 machine capability.
<snip>
> I don't really like the macros. I have nightmares from Linux going
> overboard with defining functions using spaghetti of generator macros.
>
> Could you just make most functions accept either SPR number or number
> (0, 1), or simply use if/else, to select between them?
>
> Splitting the change in 2 would be good, first add regs + TCG, then the
> spapr bits.
Sure.
> [snip]
>
>> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
>> index a05bdf78c9..022b984e00 100644
>> --- a/target/ppc/misc_helper.c
>> +++ b/target/ppc/misc_helper.c
>> @@ -204,16 +204,24 @@ void helper_store_ciabr(CPUPPCState *env, target_ulong value)
>> ppc_store_ciabr(env, value);
>> }
>>
>> -void helper_store_dawr0(CPUPPCState *env, target_ulong value)
>> -{
>> - ppc_store_dawr0(env, value);
>> +#define HELPER_STORE_DAWR(id) \
>> +void helper_store_dawr##id(CPUPPCState *env, target_ulong value) \
>> +{ \
>> + env->spr[SPR_DAWR##id] = value; \
>> }
>>
>> -void helper_store_dawrx0(CPUPPCState *env, target_ulong value)
>> -{
>> - ppc_store_dawrx0(env, value);
>> +#define HELPER_STORE_DAWRX(id) \
>> +void helper_store_dawrx##id(CPUPPCState *env, target_ulong value) \
>> +{ \
>> + env->spr[SPR_DAWRX##id] = value; \
>> }
> Did we lose the calls to ppc_store_dawr*? That will
> break direct register access (i.e., powernv) if so.
Yes. My test cases were more focussed on caps-dawr1 with pSeries
usecases, and missed this. I have taken care in the next version.
>> +HELPER_STORE_DAWR(0)
>> +HELPER_STORE_DAWRX(0)
>> +
>> +HELPER_STORE_DAWR(1)
>> +HELPER_STORE_DAWRX(1)
> I would say open-code all these too instead of generating. If we
> ever grew to >= 4 of them maybe, but as is this saves 2 lines,
> and makes 'helper_store_dawrx0' more difficult to grep for.
I open coded all of the functions with barely 12 lines more adding up
without macros.
The next version posted at
https://lore.kernel.org/qemu-devel/170679876639.188422.11634974895844092362.stgit@ltc-boston1.aus.stglabs.ibm.com/T/#t
Thanks,
Shivaprasad
© 2016 - 2024 Red Hat, Inc.