* [PATCH 1/6] target/ppc: Remove single-step suppression inside 0x100-0xf00
2023-08-08 3:11 [PATCH for-8.2 0/6] ppc: debug facility improvements Nicholas Piggin
@ 2023-08-08 3:11 ` Nicholas Piggin
2023-08-08 3:11 ` [PATCH 2/6] target/ppc: Improve book3s branch trace interrupt for v2.07S Nicholas Piggin
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Nicholas Piggin @ 2023-08-08 3:11 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Nicholas Piggin, Cédric Le Goater, David Gibson, Greg Kurz,
Harsh Prateek Bora, Shivaprasad G Bhat, qemu-ppc, qemu-devel
Single-step interrputs are suppressed if the nip is between 0x100 and
0xf00. This has been the case for a long time and it's not clear what
the intention is. Likely either an attempt to suppress trace interrupts
for instructions that cause an interrupt on completion, or a workaround
to prevent software tripping over itself single stepping its interrupt
handlers.
BookE interrupt vectors are set by IVOR registers, and BookS has AIL
modes and new interrupt types, so there are many interrupts including
the debug interrupt which can be outside this range. So any effect it
might have had does not cover most cases (including Linux on recent
BookS CPUs).
Remove this special case.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/translate.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 74796ec7ba..06530dd782 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7410,8 +7410,7 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
}
/* Honor single stepping. */
- if (unlikely(ctx->singlestep_enabled & CPU_SINGLE_STEP)
- && (nip <= 0x100 || nip > 0xf00)) {
+ if (unlikely(ctx->singlestep_enabled & CPU_SINGLE_STEP)) {
switch (is_jmp) {
case DISAS_TOO_MANY:
case DISAS_EXIT_UPDATE:
--
2.40.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/6] target/ppc: Improve book3s branch trace interrupt for v2.07S
2023-08-08 3:11 [PATCH for-8.2 0/6] ppc: debug facility improvements Nicholas Piggin
2023-08-08 3:11 ` [PATCH 1/6] target/ppc: Remove single-step suppression inside 0x100-0xf00 Nicholas Piggin
@ 2023-08-08 3:11 ` Nicholas Piggin
2023-08-08 3:11 ` [PATCH 3/6] target/ppc: Suppress single step interrupts on rfi-type instructions Nicholas Piggin
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Nicholas Piggin @ 2023-08-08 3:11 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Nicholas Piggin, Cédric Le Goater, David Gibson, Greg Kurz,
Harsh Prateek Bora, Shivaprasad G Bhat, qemu-ppc, qemu-devel
Improve the emulation accuracy of the single step and branch trace
interrupts for v2.07S. Set SRR1[33]=1, and set SIAR to completed
instruction address.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/excp_helper.c | 16 +++++++++++++++-
target/ppc/helper.h | 1 +
target/ppc/translate.c | 21 +++++++++++----------
3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 9aa8e46566..2d6aef5e66 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1571,9 +1571,11 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
}
}
break;
+ case POWERPC_EXCP_TRACE: /* Trace exception */
+ msr |= env->error_code;
+ /* fall through */
case POWERPC_EXCP_DSEG: /* Data segment exception */
case POWERPC_EXCP_ISEG: /* Instruction segment exception */
- case POWERPC_EXCP_TRACE: /* Trace exception */
case POWERPC_EXCP_SDOOR: /* Doorbell interrupt */
case POWERPC_EXCP_PERFM: /* Performance monitor interrupt */
break;
@@ -3168,6 +3170,18 @@ void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb)
}
#endif /* TARGET_PPC64 */
+/* Single-step tracing */
+void helper_book3s_trace(CPUPPCState *env, target_ulong prev_ip)
+{
+ uint32_t error_code = 0;
+ if (env->insns_flags2 & PPC2_ISA207S) {
+ /* Load/store reporting, SRR1[35, 36] and SDAR, are not implemented. */
+ env->spr[SPR_POWER_SIAR] = prev_ip;
+ error_code = PPC_BIT(33);
+ }
+ raise_exception_err(env, POWERPC_EXCP_TRACE, error_code);
+}
+
void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
MMUAccessType access_type,
int mmu_idx, uintptr_t retaddr)
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index abec6fe341..f4db32ee1a 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -32,6 +32,7 @@ DEF_HELPER_2(read_pmc, tl, env, i32)
DEF_HELPER_2(insns_inc, void, env, i32)
DEF_HELPER_1(handle_pmc5_overflow, void, env)
#endif
+DEF_HELPER_2(book3s_trace, void, env, tl)
DEF_HELPER_1(check_tlb_flush_local, void, env)
DEF_HELPER_1(check_tlb_flush_global, void, env)
#endif
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 06530dd782..5051596670 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -338,8 +338,9 @@ static void gen_ppc_maybe_interrupt(DisasContext *ctx)
* The exception can be either POWERPC_EXCP_TRACE (on most PowerPCs) or
* POWERPC_EXCP_DEBUG (on BookE).
*/
-static uint32_t gen_prep_dbgex(DisasContext *ctx)
+static void gen_debug_exception(DisasContext *ctx)
{
+#if !defined(CONFIG_USER_ONLY)
if (ctx->flags & POWERPC_FLAG_DE) {
target_ulong dbsr = 0;
if (ctx->singlestep_enabled & CPU_SINGLE_STEP) {
@@ -352,16 +353,16 @@ static uint32_t gen_prep_dbgex(DisasContext *ctx)
gen_load_spr(t0, SPR_BOOKE_DBSR);
tcg_gen_ori_tl(t0, t0, dbsr);
gen_store_spr(SPR_BOOKE_DBSR, t0);
- return POWERPC_EXCP_DEBUG;
+ gen_helper_raise_exception(cpu_env,
+ tcg_constant_i32(POWERPC_EXCP_DEBUG));
+ ctx->base.is_jmp = DISAS_NORETURN;
} else {
- return POWERPC_EXCP_TRACE;
+ TCGv t0 = tcg_temp_new();
+ tcg_gen_movi_tl(t0, ctx->cia);
+ gen_helper_book3s_trace(cpu_env, t0);
+ ctx->base.is_jmp = DISAS_NORETURN;
}
-}
-
-static void gen_debug_exception(DisasContext *ctx)
-{
- gen_helper_raise_exception(cpu_env, tcg_constant_i32(gen_prep_dbgex(ctx)));
- ctx->base.is_jmp = DISAS_NORETURN;
+#endif
}
static inline void gen_inval_exception(DisasContext *ctx, uint32_t error)
@@ -4184,7 +4185,7 @@ static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
static void gen_lookup_and_goto_ptr(DisasContext *ctx)
{
if (unlikely(ctx->singlestep_enabled)) {
- gen_debug_exception(ctx);
+ gen_debug_exception(ctx, false);
} else {
/*
* tcg_gen_lookup_and_goto_ptr will exit the TB if
--
2.40.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/6] target/ppc: Suppress single step interrupts on rfi-type instructions
2023-08-08 3:11 [PATCH for-8.2 0/6] ppc: debug facility improvements Nicholas Piggin
2023-08-08 3:11 ` [PATCH 1/6] target/ppc: Remove single-step suppression inside 0x100-0xf00 Nicholas Piggin
2023-08-08 3:11 ` [PATCH 2/6] target/ppc: Improve book3s branch trace interrupt for v2.07S Nicholas Piggin
@ 2023-08-08 3:11 ` Nicholas Piggin
2023-08-08 3:11 ` [PATCH 4/6] target/ppc: Implement breakpoint debug facility for v2.07S Nicholas Piggin
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Nicholas Piggin @ 2023-08-08 3:11 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Nicholas Piggin, Cédric Le Goater, David Gibson, Greg Kurz,
Harsh Prateek Bora, Shivaprasad G Bhat, qemu-ppc, qemu-devel
BookS does not take single step interrupts on completion of rfi and
similar (rfid, hrfid, rfscv). This is not a completely clean way to
do it, but in general non-branch instructions that change NIP on
completion are excluded.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/translate.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 5051596670..6e8f1797ac 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -338,7 +338,7 @@ static void gen_ppc_maybe_interrupt(DisasContext *ctx)
* The exception can be either POWERPC_EXCP_TRACE (on most PowerPCs) or
* POWERPC_EXCP_DEBUG (on BookE).
*/
-static void gen_debug_exception(DisasContext *ctx)
+static void gen_debug_exception(DisasContext *ctx, bool rfi_type)
{
#if !defined(CONFIG_USER_ONLY)
if (ctx->flags & POWERPC_FLAG_DE) {
@@ -357,10 +357,12 @@ static void gen_debug_exception(DisasContext *ctx)
tcg_constant_i32(POWERPC_EXCP_DEBUG));
ctx->base.is_jmp = DISAS_NORETURN;
} else {
- TCGv t0 = tcg_temp_new();
- tcg_gen_movi_tl(t0, ctx->cia);
- gen_helper_book3s_trace(cpu_env, t0);
- ctx->base.is_jmp = DISAS_NORETURN;
+ if (!rfi_type) { /* BookS does not single step rfi type instructions */
+ TCGv t0 = tcg_temp_new();
+ tcg_gen_movi_tl(t0, ctx->cia);
+ gen_helper_book3s_trace(cpu_env, t0);
+ ctx->base.is_jmp = DISAS_NORETURN;
+ }
}
#endif
}
@@ -7412,6 +7414,8 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
/* Honor single stepping. */
if (unlikely(ctx->singlestep_enabled & CPU_SINGLE_STEP)) {
+ bool rfi_type = false;
+
switch (is_jmp) {
case DISAS_TOO_MANY:
case DISAS_EXIT_UPDATE:
@@ -7420,12 +7424,19 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
break;
case DISAS_EXIT:
case DISAS_CHAIN:
+ /*
+ * This is a heuristic, to put it kindly. The rfi class of
+ * instructions are among the few outside branches that change
+ * NIP without taking an interrupt. Single step trace interrupts
+ * do not fire on completion of these instructions.
+ */
+ rfi_type = true;
break;
default:
g_assert_not_reached();
}
- gen_debug_exception(ctx);
+ gen_debug_exception(ctx, rfi_type);
return;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/6] target/ppc: Implement breakpoint debug facility for v2.07S
2023-08-08 3:11 [PATCH for-8.2 0/6] ppc: debug facility improvements Nicholas Piggin
` (2 preceding siblings ...)
2023-08-08 3:11 ` [PATCH 3/6] target/ppc: Suppress single step interrupts on rfi-type instructions Nicholas Piggin
@ 2023-08-08 3:11 ` Nicholas Piggin
2023-08-08 3:11 ` [PATCH 5/6] target/ppc: Implement watchpoint " Nicholas Piggin
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Nicholas Piggin @ 2023-08-08 3:11 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Nicholas Piggin, Cédric Le Goater, David Gibson, Greg Kurz,
Harsh Prateek Bora, Shivaprasad G Bhat, qemu-ppc, qemu-devel
ISA v2.07S introduced the breakpoint facility based on the CIABR SPR.
Implement this in TCG.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/cpu.c | 27 ++++++++++++++++++++++++++
target/ppc/cpu.h | 3 +++
target/ppc/cpu_init.c | 5 ++++-
target/ppc/excp_helper.c | 42 ++++++++++++++++++++++++++++++++++++++++
target/ppc/helper.h | 1 +
target/ppc/internal.h | 2 ++
target/ppc/machine.c | 4 ++++
target/ppc/misc_helper.c | 5 +++++
target/ppc/spr_common.h | 1 +
target/ppc/translate.c | 10 +++++++++-
10 files changed, 98 insertions(+), 2 deletions(-)
diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index 424f2e1741..d9c665ce18 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -102,6 +102,33 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
ppc_maybe_interrupt(env);
}
+
+#if defined(TARGET_PPC64)
+void ppc_update_ciabr(CPUPPCState *env)
+{
+ CPUState *cs = env_cpu(env);
+ target_ulong ciabr = env->spr[SPR_CIABR];
+ target_ulong ciea, priv;
+
+ ciea = ciabr & PPC_BITMASK(0, 61);
+ priv = ciabr & PPC_BITMASK(62, 63);
+
+ if (env->ciabr_breakpoint) {
+ cpu_breakpoint_remove_by_ref(cs, env->ciabr_breakpoint);
+ env->ciabr_breakpoint = NULL;
+ }
+
+ if (priv) {
+ cpu_breakpoint_insert(cs, ciea, BP_CPU, &env->ciabr_breakpoint);
+ }
+}
+
+void ppc_store_ciabr(CPUPPCState *env, target_ulong val)
+{
+ env->spr[SPR_CIABR] = val;
+ ppc_update_ciabr(env);
+}
+#endif
#endif
static inline void fpscr_set_rounding_mode(CPUPPCState *env)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 25fac9577a..d97fabd8f6 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1137,6 +1137,7 @@ struct CPUArchState {
/* MMU context, only relevant for full system emulation */
#if defined(TARGET_PPC64)
ppc_slb_t slb[MAX_SLB_ENTRIES]; /* PowerPC 64 SLB area */
+ struct CPUBreakpoint *ciabr_breakpoint;
#endif
target_ulong sr[32]; /* segment registers */
uint32_t nb_BATs; /* number of BATs */
@@ -1403,6 +1404,8 @@ void ppc_translate_init(void);
#if !defined(CONFIG_USER_ONLY)
void ppc_store_sdr1(CPUPPCState *env, target_ulong value);
void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val);
+void ppc_update_ciabr(CPUPPCState *env);
+void ppc_store_ciabr(CPUPPCState *env, target_ulong value);
#endif /* !defined(CONFIG_USER_ONLY) */
void ppc_store_msr(CPUPPCState *env, target_ulong value);
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 02b7aad9b0..a2820839b3 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5127,7 +5127,7 @@ static void register_book3s_207_dbg_sprs(CPUPPCState *env)
spr_register_kvm_hv(env, SPR_CIABR, "CIABR",
SPR_NOACCESS, SPR_NOACCESS,
SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_generic, &spr_write_generic,
+ &spr_read_generic, &spr_write_ciabr,
KVM_REG_PPC_CIABR, 0x00000000);
}
@@ -7149,6 +7149,7 @@ static void ppc_cpu_reset_hold(Object *obj)
env->nip = env->hreset_vector | env->excp_prefix;
if (tcg_enabled()) {
+ cpu_breakpoint_remove_all(s, BP_CPU);
if (env->mmu_model != POWERPC_MMU_REAL) {
ppc_tlb_invalidate_all(env);
}
@@ -7336,6 +7337,8 @@ static const struct TCGCPUOps ppc_tcg_ops = {
.cpu_exec_exit = ppc_cpu_exec_exit,
.do_unaligned_access = ppc_cpu_do_unaligned_access,
.do_transaction_failed = ppc_cpu_do_transaction_failed,
+ .debug_excp_handler = ppc_cpu_debug_excp_handler,
+ .debug_check_breakpoint = ppc_cpu_debug_check_breakpoint,
#endif /* !CONFIG_USER_ONLY */
};
#endif /* CONFIG_TCG */
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 2d6aef5e66..9c9881ae19 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -3257,5 +3257,47 @@ void ppc_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
cs->exception_index = POWERPC_EXCP_MCHECK;
cpu_loop_exit_restore(cs, retaddr);
}
+
+void ppc_cpu_debug_excp_handler(CPUState *cs)
+{
+#if defined(TARGET_PPC64)
+ CPUPPCState *env = cs->env_ptr;
+
+ if (env->insns_flags2 & PPC2_ISA207S) {
+ if (cpu_breakpoint_test(cs, env->nip, BP_CPU)) {
+ raise_exception_err(env, POWERPC_EXCP_TRACE,
+ PPC_BIT(33) | PPC_BIT(43));
+ }
+ }
+#endif
+}
+
+bool ppc_cpu_debug_check_breakpoint(CPUState *cs)
+{
+#if defined(TARGET_PPC64)
+ CPUPPCState *env = cs->env_ptr;
+
+ if (env->insns_flags2 & PPC2_ISA207S) {
+ target_ulong priv;
+
+ priv = env->spr[SPR_CIABR] & PPC_BITMASK(62, 63);
+ switch (priv) {
+ case 0x1: /* problem */
+ return env->msr & ((target_ulong)1 << MSR_PR);
+ case 0x2: /* supervisor */
+ return (!(env->msr & ((target_ulong)1 << MSR_PR)) &&
+ !(env->msr & ((target_ulong)1 << MSR_HV)));
+ case 0x3: /* hypervisor */
+ return (!(env->msr & ((target_ulong)1 << MSR_PR)) &&
+ (env->msr & ((target_ulong)1 << MSR_HV)));
+ default:
+ g_assert_not_reached();
+ }
+ }
+#endif
+
+ return false;
+}
+
#endif /* CONFIG_TCG */
#endif /* !CONFIG_USER_ONLY */
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index f4db32ee1a..83d5deec07 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -25,6 +25,7 @@ DEF_HELPER_1(hrfid, void, env)
DEF_HELPER_2(rfebb, void, env, tl)
DEF_HELPER_2(store_lpcr, void, env, tl)
DEF_HELPER_2(store_pcr, void, env, tl)
+DEF_HELPER_2(store_ciabr, 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/internal.h b/target/ppc/internal.h
index 57acb3212c..16f02fd9c4 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -301,6 +301,8 @@ void ppc_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
MMUAccessType access_type,
int mmu_idx, MemTxAttrs attrs,
MemTxResult response, uintptr_t retaddr);
+void ppc_cpu_debug_excp_handler(CPUState *cs);
+bool ppc_cpu_debug_check_breakpoint(CPUState *cs);
#endif
FIELD(GER_MSK, XMSK, 0, 4)
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 134b16c625..560a875454 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -314,6 +314,10 @@ static int cpu_post_load(void *opaque, int version_id)
post_load_update_msr(env);
if (tcg_enabled()) {
+ /* Re-set breaks based on regs */
+#if defined(TARGET_PPC64)
+ ppc_update_ciabr(env);
+#endif
pmu_mmcr01_updated(env);
}
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index 692d058665..0b0f2e59a7 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -199,6 +199,11 @@ void helper_store_pcr(CPUPPCState *env, target_ulong value)
env->spr[SPR_PCR] = value & pcc->pcr_mask;
}
+void helper_store_ciabr(CPUPPCState *env, target_ulong value)
+{
+ ppc_store_ciabr(env, value);
+}
+
/*
* 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 5995070eaf..b7bedd9ef1 100644
--- a/target/ppc/spr_common.h
+++ b/target/ppc/spr_common.h
@@ -159,6 +159,7 @@ void spr_read_mas73(DisasContext *ctx, int gprn, int sprn);
#ifdef TARGET_PPC64
void spr_read_cfar(DisasContext *ctx, int gprn, int sprn);
void spr_write_cfar(DisasContext *ctx, int sprn, int gprn);
+void spr_write_ciabr(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 6e8f1797ac..69dd1ba036 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -559,8 +559,9 @@ void spr_write_lr(DisasContext *ctx, int sprn, int gprn)
tcg_gen_mov_tl(cpu_lr, cpu_gpr[gprn]);
}
-/* CFAR */
#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+/* Debug facilities */
+/* CFAR */
void spr_read_cfar(DisasContext *ctx, int gprn, int sprn)
{
tcg_gen_mov_tl(cpu_gpr[gprn], cpu_cfar);
@@ -570,6 +571,13 @@ void spr_write_cfar(DisasContext *ctx, int sprn, int gprn)
{
tcg_gen_mov_tl(cpu_cfar, cpu_gpr[gprn]);
}
+
+/* Breakpoint */
+void spr_write_ciabr(DisasContext *ctx, int sprn, int gprn)
+{
+ translator_io_start(&ctx->base);
+ gen_helper_store_ciabr(cpu_env, cpu_gpr[gprn]);
+}
#endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */
/* CTR */
--
2.40.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 5/6] target/ppc: Implement watchpoint debug facility for v2.07S
2023-08-08 3:11 [PATCH for-8.2 0/6] ppc: debug facility improvements Nicholas Piggin
` (3 preceding siblings ...)
2023-08-08 3:11 ` [PATCH 4/6] target/ppc: Implement breakpoint debug facility for v2.07S Nicholas Piggin
@ 2023-08-08 3:11 ` Nicholas Piggin
2023-08-08 3:11 ` [PATCH 6/6] spapr: implement H_SET_MODE debug facilities Nicholas Piggin
2023-08-29 16:43 ` [PATCH for-8.2 0/6] ppc: debug facility improvements Cédric Le Goater
6 siblings, 0 replies; 8+ messages in thread
From: Nicholas Piggin @ 2023-08-08 3:11 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Nicholas Piggin, Cédric Le Goater, David Gibson, Greg Kurz,
Harsh Prateek Bora, Shivaprasad G Bhat, qemu-ppc, qemu-devel
ISA v2.07S introduced the watchpoint facility based on the DAWR0
and DAWRX0 SPRs. Implement this in TCG.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/cpu.c | 59 ++++++++++++++++++++++++++++++++++++++++
target/ppc/cpu.h | 4 +++
target/ppc/cpu_init.c | 6 ++--
target/ppc/excp_helper.c | 52 ++++++++++++++++++++++++++++++++++-
target/ppc/helper.h | 2 ++
target/ppc/internal.h | 1 +
target/ppc/machine.c | 1 +
target/ppc/misc_helper.c | 10 +++++++
target/ppc/spr_common.h | 2 ++
target/ppc/translate.c | 13 +++++++++
10 files changed, 147 insertions(+), 3 deletions(-)
diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index d9c665ce18..62e1c15e3d 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -128,6 +128,65 @@ void ppc_store_ciabr(CPUPPCState *env, target_ulong val)
env->spr[SPR_CIABR] = 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;
+ }
+
+ cpu_watchpoint_insert(cs, deaw, len, flags, &env->dawr0_watchpoint);
+}
+
+void ppc_store_dawr0(CPUPPCState *env, target_ulong val)
+{
+ env->spr[SPR_DAWR0] = val;
+ ppc_update_daw0(env);
+}
+
+void ppc_store_dawrx0(CPUPPCState *env, uint32_t val)
+{
+ int hrammc = extract32(val, PPC_BIT_NR(56), 1);
+
+ 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__);
+ }
+
+ env->spr[SPR_DAWRX0] = val;
+ ppc_update_daw0(env);
+}
#endif
#endif
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index d97fabd8f6..2777ea3110 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1138,6 +1138,7 @@ struct CPUArchState {
#if defined(TARGET_PPC64)
ppc_slb_t slb[MAX_SLB_ENTRIES]; /* PowerPC 64 SLB area */
struct CPUBreakpoint *ciabr_breakpoint;
+ struct CPUWatchpoint *dawr0_watchpoint;
#endif
target_ulong sr[32]; /* segment registers */
uint32_t nb_BATs; /* number of BATs */
@@ -1406,6 +1407,9 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong value);
void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val);
void ppc_update_ciabr(CPUPPCState *env);
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);
#endif /* !defined(CONFIG_USER_ONLY) */
void ppc_store_msr(CPUPPCState *env, target_ulong value);
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index a2820839b3..9c1c045d1b 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5117,12 +5117,12 @@ static void register_book3s_207_dbg_sprs(CPUPPCState *env)
spr_register_kvm_hv(env, SPR_DAWR0, "DAWR0",
SPR_NOACCESS, SPR_NOACCESS,
SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_generic, &spr_write_generic,
+ &spr_read_generic, &spr_write_dawr0,
KVM_REG_PPC_DAWR, 0x00000000);
spr_register_kvm_hv(env, SPR_DAWRX0, "DAWRX0",
SPR_NOACCESS, SPR_NOACCESS,
SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_generic, &spr_write_generic32,
+ &spr_read_generic, &spr_write_dawrx0,
KVM_REG_PPC_DAWRX, 0x00000000);
spr_register_kvm_hv(env, SPR_CIABR, "CIABR",
SPR_NOACCESS, SPR_NOACCESS,
@@ -7150,6 +7150,7 @@ static void ppc_cpu_reset_hold(Object *obj)
if (tcg_enabled()) {
cpu_breakpoint_remove_all(s, BP_CPU);
+ cpu_watchpoint_remove_all(s, BP_CPU);
if (env->mmu_model != POWERPC_MMU_REAL) {
ppc_tlb_invalidate_all(env);
}
@@ -7339,6 +7340,7 @@ static const struct TCGCPUOps ppc_tcg_ops = {
.do_transaction_failed = ppc_cpu_do_transaction_failed,
.debug_excp_handler = ppc_cpu_debug_excp_handler,
.debug_check_breakpoint = ppc_cpu_debug_check_breakpoint,
+ .debug_check_watchpoint = ppc_cpu_debug_check_watchpoint,
#endif /* !CONFIG_USER_ONLY */
};
#endif /* CONFIG_TCG */
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 9c9881ae19..32e46e56b3 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -3264,7 +3264,15 @@ void ppc_cpu_debug_excp_handler(CPUState *cs)
CPUPPCState *env = cs->env_ptr;
if (env->insns_flags2 & PPC2_ISA207S) {
- if (cpu_breakpoint_test(cs, env->nip, BP_CPU)) {
+ if (cs->watchpoint_hit) {
+ if (cs->watchpoint_hit->flags & BP_CPU) {
+ env->spr[SPR_DAR] = cs->watchpoint_hit->hitaddr;
+ env->spr[SPR_DSISR] = PPC_BIT(41);
+ cs->watchpoint_hit = NULL;
+ raise_exception(env, POWERPC_EXCP_DSI);
+ }
+ cs->watchpoint_hit = NULL;
+ } else if (cpu_breakpoint_test(cs, env->nip, BP_CPU)) {
raise_exception_err(env, POWERPC_EXCP_TRACE,
PPC_BIT(33) | PPC_BIT(43));
}
@@ -3299,5 +3307,47 @@ bool ppc_cpu_debug_check_breakpoint(CPUState *cs)
return false;
}
+bool ppc_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
+{
+#if defined(TARGET_PPC64)
+ CPUPPCState *env = cs->env_ptr;
+
+ 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) {
+ return false;
+ }
+
+ if (!wti) {
+ if (env->msr & ((target_ulong)1 << MSR_DR)) {
+ if (!wt) {
+ return false;
+ }
+ } else {
+ if (wt) {
+ return false;
+ }
+ }
+ }
+
+ return true;
+ }
+ }
+#endif
+
+ return false;
+}
+
#endif /* CONFIG_TCG */
#endif /* !CONFIG_USER_ONLY */
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 83d5deec07..86f97ee1e7 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -26,6 +26,8 @@ DEF_HELPER_2(rfebb, void, env, tl)
DEF_HELPER_2(store_lpcr, void, env, tl)
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_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/internal.h b/target/ppc/internal.h
index 16f02fd9c4..15803bc313 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -303,6 +303,7 @@ void ppc_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
MemTxResult response, uintptr_t retaddr);
void ppc_cpu_debug_excp_handler(CPUState *cs);
bool ppc_cpu_debug_check_breakpoint(CPUState *cs);
+bool ppc_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
#endif
FIELD(GER_MSK, XMSK, 0, 4)
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 560a875454..8234e35d69 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -317,6 +317,7 @@ static int cpu_post_load(void *opaque, int version_id)
/* Re-set breaks based on regs */
#if defined(TARGET_PPC64)
ppc_update_ciabr(env);
+ ppc_update_daw0(env);
#endif
pmu_mmcr01_updated(env);
}
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index 0b0f2e59a7..a05bdf78c9 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -204,6 +204,16 @@ 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);
+}
+
+void helper_store_dawrx0(CPUPPCState *env, target_ulong value)
+{
+ ppc_store_dawrx0(env, value);
+}
+
/*
* 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 b7bedd9ef1..8a9d6cd994 100644
--- a/target/ppc/spr_common.h
+++ b/target/ppc/spr_common.h
@@ -160,6 +160,8 @@ void spr_read_mas73(DisasContext *ctx, int gprn, int sprn);
void spr_read_cfar(DisasContext *ctx, int gprn, int sprn);
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_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 69dd1ba036..b8c7f38ccd 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -578,6 +578,19 @@ void spr_write_ciabr(DisasContext *ctx, int sprn, int gprn)
translator_io_start(&ctx->base);
gen_helper_store_ciabr(cpu_env, cpu_gpr[gprn]);
}
+
+/* Watchpoint */
+void spr_write_dawr0(DisasContext *ctx, int sprn, int gprn)
+{
+ translator_io_start(&ctx->base);
+ gen_helper_store_dawr0(cpu_env, cpu_gpr[gprn]);
+}
+
+void spr_write_dawrx0(DisasContext *ctx, int sprn, int gprn)
+{
+ translator_io_start(&ctx->base);
+ gen_helper_store_dawrx0(cpu_env, cpu_gpr[gprn]);
+}
#endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */
/* CTR */
--
2.40.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 6/6] spapr: implement H_SET_MODE debug facilities
2023-08-08 3:11 [PATCH for-8.2 0/6] ppc: debug facility improvements Nicholas Piggin
` (4 preceding siblings ...)
2023-08-08 3:11 ` [PATCH 5/6] target/ppc: Implement watchpoint " Nicholas Piggin
@ 2023-08-08 3:11 ` Nicholas Piggin
2023-08-29 16:43 ` [PATCH for-8.2 0/6] ppc: debug facility improvements Cédric Le Goater
6 siblings, 0 replies; 8+ messages in thread
From: Nicholas Piggin @ 2023-08-08 3:11 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Nicholas Piggin, Cédric Le Goater, David Gibson, Greg Kurz,
Harsh Prateek Bora, Shivaprasad G Bhat, qemu-ppc, qemu-devel
Wire up the H_SET_MODE debug resources to the CIABR and DAWR0 debug
facilities in TCG.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/ppc/spapr_hcall.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 9b1f225d4a..b7dc388f2f 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -3,6 +3,7 @@
#include "qapi/error.h"
#include "sysemu/hw_accel.h"
#include "sysemu/runstate.h"
+#include "sysemu/tcg.h"
#include "qemu/log.h"
#include "qemu/main-loop.h"
#include "qemu/module.h"
@@ -789,6 +790,54 @@ static target_ulong h_logical_dcbf(PowerPCCPU *cpu, SpaprMachineState *spapr,
return H_SUCCESS;
}
+static target_ulong h_set_mode_resource_set_ciabr(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) {
+ return H_P4;
+ }
+ if ((value1 & PPC_BITMASK(62, 63)) == 0x3) {
+ return H_P3;
+ }
+
+ ppc_store_ciabr(env, value1);
+
+ 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;
+}
+
static target_ulong h_set_mode_resource_le(PowerPCCPU *cpu,
SpaprMachineState *spapr,
target_ulong mflags,
@@ -858,6 +907,14 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, SpaprMachineState *spapr,
target_ulong ret = H_P2;
switch (resource) {
+ case H_SET_MODE_RESOURCE_SET_CIABR:
+ ret = h_set_mode_resource_set_ciabr(cpu, spapr, args[0], args[2],
+ args[3]);
+ break;
+ case H_SET_MODE_RESOURCE_SET_DAWR0:
+ ret = h_set_mode_resource_set_dawr0(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;
--
2.40.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH for-8.2 0/6] ppc: debug facility improvements
2023-08-08 3:11 [PATCH for-8.2 0/6] ppc: debug facility improvements Nicholas Piggin
` (5 preceding siblings ...)
2023-08-08 3:11 ` [PATCH 6/6] spapr: implement H_SET_MODE debug facilities Nicholas Piggin
@ 2023-08-29 16:43 ` Cédric Le Goater
6 siblings, 0 replies; 8+ messages in thread
From: Cédric Le Goater @ 2023-08-29 16:43 UTC (permalink / raw)
To: Nicholas Piggin, Daniel Henrique Barboza
Cc: David Gibson, Greg Kurz, Harsh Prateek Bora, Shivaprasad G Bhat,
qemu-ppc, qemu-devel
Hello,
On 8/8/23 05:11, Nicholas Piggin wrote:
> I started out looking at this to reduce divergence of TCG and KVM
> machines with 2nd DAWR. The divergence already exists with first
> DAWR, so I don't want to tie the KVM 2nd DAWR enablement to this,
> but it would be nice to ensure the caps and such for the 2nd DAWR
> will also work for TCG.
>
> I don't know that we have great test cases for this, it does work
> with some of the Linux selftests ptrace debug tests (although those
> tests seem to have a few issues in upstream kernels), some basic
> Linux xmon and gdb tests by hand, and I've started working on some
> kvm unit tests.
>
> Thanks,
> Nick
>
> Nicholas Piggin (6):
> target/ppc: Remove single-step suppression inside 0x100-0xf00
> target/ppc: Improve book3s branch trace interrupt for v2.07S
> target/ppc: Suppress single step interrupts on rfi-type instructions
> target/ppc: Implement breakpoint debug facility for v2.07S
> target/ppc: Implement watchpoint debug facility for v2.07S
> spapr: implement H_SET_MODE debug facilities
>
> hw/ppc/spapr_hcall.c | 57 +++++++++++++++++++++
> target/ppc/cpu.c | 86 +++++++++++++++++++++++++++++++
> target/ppc/cpu.h | 7 +++
> target/ppc/cpu_init.c | 11 ++--
> target/ppc/excp_helper.c | 108 ++++++++++++++++++++++++++++++++++++++-
> target/ppc/helper.h | 4 ++
> target/ppc/internal.h | 3 ++
> target/ppc/machine.c | 5 ++
> target/ppc/misc_helper.c | 15 ++++++
> target/ppc/spr_common.h | 3 ++
> target/ppc/translate.c | 60 +++++++++++++++++-----
> 11 files changed, 341 insertions(+), 18 deletions(-)
>
I am preparing a PR with this series. It is time to take a look at it if you
haven't already !
Thanks,
C.
^ permalink raw reply [flat|nested] 8+ messages in thread