From: "Nicholas Piggin" <npiggin@gmail.com>
To: "Shivaprasad G Bhat" <sbhat@linux.ibm.com>,
<danielhb413@gmail.com>, <qemu-ppc@nongnu.org>,
<david@gibson.dropbear.id.au>, <harshpb@linux.ibm.com>,
<clg@kaod.org>, <groug@kaod.org>
Cc: <pbonzini@redhat.com>, <kvm@vger.kernel.org>, <qemu-devel@nongnu.org>
Subject: Re: [PATCH v8 1/2] ppc: Enable 2nd DAWR support on Power10 PowerNV machine
Date: Tue, 27 Feb 2024 22:01:07 +1000 [thread overview]
Message-ID: <CZFUFVA72347.2XPCPLI34U6MX@wheely> (raw)
In-Reply-To: <170679877410.188422.2597832350300436754.stgit@ltc-boston1.aus.stglabs.ibm.com>
On Fri Feb 2, 2024 at 12:46 AM AEST, Shivaprasad G Bhat wrote:
> Extend the existing watchpoint facility from TCG DAWR0 emulation
> to DAWR1 on POWER10.
>
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> ---
> target/ppc/cpu.c | 45 ++++++++++++++++++++++++----------
> target/ppc/cpu.h | 8 +++++-
> target/ppc/cpu_init.c | 15 +++++++++++
> target/ppc/excp_helper.c | 61 ++++++++++++++++++++++++++--------------------
> target/ppc/helper.h | 2 ++
> target/ppc/machine.c | 3 ++
> target/ppc/misc_helper.c | 10 ++++++++
> target/ppc/spr_common.h | 2 ++
> target/ppc/translate.c | 12 +++++++++
> 9 files changed, 115 insertions(+), 43 deletions(-)
>
> diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
> index e3ad8e0c27..d5ac9bb888 100644
> --- a/target/ppc/cpu.c
> +++ b/target/ppc/cpu.c
> @@ -130,11 +130,13 @@ void ppc_store_ciabr(CPUPPCState *env, target_ulong val)
> ppc_update_ciabr(env);
> }
>
> -void ppc_update_daw0(CPUPPCState *env)
> +void ppc_update_daw(CPUPPCState *env, int rid)
What's rid? Register ID?
Looks pretty good I think.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Thanks,
Nick
> {
> CPUState *cs = env_cpu(env);
> - target_ulong deaw = env->spr[SPR_DAWR0] & PPC_BITMASK(0, 60);
> - uint32_t dawrx = env->spr[SPR_DAWRX0];
> + int spr_dawr = !rid ? SPR_DAWR0 : SPR_DAWR1;
> + int spr_dawrx = !rid ? SPR_DAWRX0 : SPR_DAWRX1;
> + target_ulong deaw = env->spr[spr_dawr] & PPC_BITMASK(0, 60);
> + uint32_t dawrx = env->spr[spr_dawrx];
> 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);
> @@ -144,9 +146,9 @@ void ppc_update_daw0(CPUPPCState *env)
> vaddr len;
> int flags;
>
> - if (env->dawr0_watchpoint) {
> - cpu_watchpoint_remove_by_ref(cs, env->dawr0_watchpoint);
> - env->dawr0_watchpoint = NULL;
> + if (env->dawr_watchpoint[rid]) {
> + cpu_watchpoint_remove_by_ref(cs, env->dawr_watchpoint[rid]);
> + env->dawr_watchpoint[rid] = NULL;
> }
>
> if (!dr && !dw) {
> @@ -166,28 +168,45 @@ void ppc_update_daw0(CPUPPCState *env)
> flags |= BP_MEM_WRITE;
> }
>
> - cpu_watchpoint_insert(cs, deaw, len, flags, &env->dawr0_watchpoint);
> + cpu_watchpoint_insert(cs, deaw, len, flags, &env->dawr_watchpoint[rid]);
> }
>
> void ppc_store_dawr0(CPUPPCState *env, target_ulong val)
> {
> env->spr[SPR_DAWR0] = val;
> - ppc_update_daw0(env);
> + ppc_update_daw(env, 0);
> }
>
> -void ppc_store_dawrx0(CPUPPCState *env, uint32_t val)
> +static void ppc_store_dawrx(CPUPPCState *env, uint32_t val, int rid)
> {
> 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__);
> + qemu_log_mask(LOG_UNIMP, "%s: DAWRX%d[HRAMMC] is unimplemented\n",
> + __func__, rid);
> }
>
> - env->spr[SPR_DAWRX0] = val;
> - ppc_update_daw0(env);
> + env->spr[!rid ? SPR_DAWRX0 : SPR_DAWRX1] = val;
> + ppc_update_daw(env, rid);
> +}
> +
> +void ppc_store_dawrx0(CPUPPCState *env, uint32_t val)
> +{
> + ppc_store_dawrx(env, val, 0);
> +}
> +
> +void ppc_store_dawr1(CPUPPCState *env, target_ulong val)
> +{
> + env->spr[SPR_DAWR1] = val;
> + ppc_update_daw(env, 1);
> +}
> +
> +void ppc_store_dawrx1(CPUPPCState *env, uint32_t val)
> +{
> + ppc_store_dawrx(env, val, 1);
> }
> +
> #endif
> #endif
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index f8101ffa29..18dcc438ea 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1236,7 +1236,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;
> + struct CPUWatchpoint *dawr_watchpoint[2];
> #endif
> target_ulong sr[32]; /* segment registers */
> uint32_t nb_BATs; /* number of BATs */
> @@ -1549,9 +1549,11 @@ 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_update_daw(CPUPPCState *env, int rid);
> void ppc_store_dawr0(CPUPPCState *env, target_ulong value);
> void ppc_store_dawrx0(CPUPPCState *env, uint32_t value);
> +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 +1739,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 23eb5522b6..c901559859 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 2ec6429e36..32eba7f725 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->dawr_watchpoint[0])) {
> + dawrx = env->spr[SPR_DAWRX0];
> + } else if ((env->insns_flags2 & PPC2_ISA310) &&
> + (wp == env->dawr_watchpoint[1])) {
> + 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/machine.c b/target/ppc/machine.c
> index 203fe28e01..082712ff16 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -325,7 +325,8 @@ 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);
> + ppc_update_daw(env, 0);
> + ppc_update_daw(env, 1);
> #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 a9d41d2802..54e402b139 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -214,6 +214,16 @@ void helper_store_dawrx0(CPUPPCState *env, target_ulong value)
> ppc_store_dawrx0(env, value);
> }
>
> +void helper_store_dawr1(CPUPPCState *env, target_ulong value)
> +{
> + ppc_store_dawr1(env, value);
> +}
> +
> +void helper_store_dawrx1(CPUPPCState *env, target_ulong value)
> +{
> + ppc_store_dawrx1(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 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 049f636927..ac2a53f3b8 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -593,6 +593,18 @@ void spr_write_dawrx0(DisasContext *ctx, int sprn, int gprn)
> translator_io_start(&ctx->base);
> gen_helper_store_dawrx0(tcg_env, cpu_gpr[gprn]);
> }
> +
> +void spr_write_dawr1(DisasContext *ctx, int sprn, int gprn)
> +{
> + translator_io_start(&ctx->base);
> + gen_helper_store_dawr1(tcg_env, cpu_gpr[gprn]);
> +}
> +
> +void spr_write_dawrx1(DisasContext *ctx, int sprn, int gprn)
> +{
> + translator_io_start(&ctx->base);
> + gen_helper_store_dawrx1(tcg_env, cpu_gpr[gprn]);
> +}
> #endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */
>
> /* CTR */
next prev parent reply other threads:[~2024-02-27 12:01 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-01 14:46 [PATCH v8 0/2] ppc: Enable 2nd DAWR support on Power10 Shivaprasad G Bhat
2024-02-01 14:46 ` [PATCH v8 1/2] ppc: Enable 2nd DAWR support on Power10 PowerNV machine Shivaprasad G Bhat
2024-02-27 12:01 ` Nicholas Piggin [this message]
2024-12-19 9:48 ` Harsh Prateek Bora
2024-02-01 14:46 ` [PATCH v8 2/2] ppc: spapr: Enable 2nd DAWR on Power10 pSeries machine Shivaprasad G Bhat
2024-02-27 12:21 ` Nicholas Piggin
2024-02-27 20:52 ` David Gibson
2025-01-17 4:11 ` Shivaprasad G Bhat
2024-12-20 9:57 ` Harsh Prateek Bora
2024-12-20 10:08 ` Harsh Prateek Bora
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CZFUFVA72347.2XPCPLI34U6MX@wheely \
--to=npiggin@gmail.com \
--cc=clg@kaod.org \
--cc=danielhb413@gmail.com \
--cc=david@gibson.dropbear.id.au \
--cc=groug@kaod.org \
--cc=harshpb@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=sbhat@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.