qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: qemu-ppc@nongnu.org
Cc: Nicholas Piggin <npiggin@gmail.com>, qemu-devel@nongnu.org
Subject: [PATCH 3/3] target/ppc: add missing TLB flushes for memory protection key SPR updates
Date: Mon,  3 Mar 2025 21:23:14 +1000	[thread overview]
Message-ID: <20250303112315.586478-4-npiggin@gmail.com> (raw)
In-Reply-To: <20250303112315.586478-1-npiggin@gmail.com>

The IAMR and AMR registers are involved with MMU translations that
are not tagged in the TLB (i.e., with mmuidx), so the TLB needs to be
flushed when these are changed, e.g., as PIDR, LPIDR already do.

This moves AMR and IAMR write to helpers rather than use tlb_need_flush
because they can be written in problem state where tlb_need_flush is not
checked.

XXX: As far as I can tell this is needed for correct memory protection
key operation, however it seems to be causing slowdowns when booting
Linux, enough to cause failures due to timeouts, so I will not merge it
at the moment. I have been considering possible ways to speed this up
e.g., with mmu indexes, but that's not entirely trivial and needs a bit
more work.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/helper.h      |  2 ++
 target/ppc/cpu_init.c    |  4 ++--
 target/ppc/misc_helper.c | 48 +++++++++++++++++++++++++++++++++++++
 target/ppc/translate.c   | 52 ++--------------------------------------
 4 files changed, 54 insertions(+), 52 deletions(-)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 6178ebe138f..e8de4f95581 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -723,6 +723,8 @@ DEF_HELPER_FLAGS_1(load_vtb, TCG_CALL_NO_RWG, tl, env)
 #if defined(TARGET_PPC64)
 DEF_HELPER_FLAGS_1(load_purr, TCG_CALL_NO_RWG, tl, env)
 DEF_HELPER_FLAGS_2(store_purr, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_2(store_amr, void, env, tl)
+DEF_HELPER_2(store_iamr, void, env, tl)
 DEF_HELPER_2(store_hrmor, void, env, tl)
 DEF_HELPER_2(store_ptcr, void, env, tl)
 DEF_HELPER_FLAGS_1(load_dpdes, TCG_CALL_NO_RWG, tl, env)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 92316b55afd..43af471ae64 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -238,7 +238,7 @@ static void register_amr_sprs(CPUPPCState *env)
     spr_register_kvm_hv(env, SPR_AMR, "AMR",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_amr,
-                     &spr_read_generic, &spr_write_generic,
+                     &spr_read_generic, &spr_write_amr,
                      KVM_REG_PPC_AMR, 0);
     spr_register_kvm_hv(env, SPR_UAMOR, "UAMOR",
                      SPR_NOACCESS, SPR_NOACCESS,
@@ -259,7 +259,7 @@ static void register_iamr_sprs(CPUPPCState *env)
     spr_register_kvm_hv(env, SPR_IAMR, "IAMR",
                         SPR_NOACCESS, SPR_NOACCESS,
                         &spr_read_generic, &spr_write_iamr,
-                        &spr_read_generic, &spr_write_generic,
+                        &spr_read_generic, &spr_write_iamr,
                         KVM_REG_PPC_IAMR, 0);
 #endif /* !CONFIG_USER_ONLY */
 }
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index ac439e00326..dfc9d806b30 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -169,6 +169,54 @@ void helper_store_sdr1(CPUPPCState *env, target_ulong val)
 }
 
 #if defined(TARGET_PPC64)
+void helper_store_amr(CPUPPCState *env, target_ulong val)
+{
+    target_ulong old, new, mask;
+
+    if (FIELD_EX64(env->msr, MSR, PR)) {
+        mask = env->spr[SPR_UAMOR];
+    } else if (FIELD_EX64(env->msr, MSR, HV)) {
+        mask = (target_ulong)-1;
+    } else {
+        mask = env->spr[SPR_AMOR];
+    }
+
+    old = env->spr[SPR_AMR];
+    /* Replace controllable bits with those in val */
+    new = (old & ~mask) | (val & mask);
+
+    if (old != new) {
+        CPUState *cs = env_cpu(env);
+        env->spr[SPR_AMR] = new;
+        /* AMR is involved in MMU translations so must flush TLB */
+        tlb_flush(cs);
+    }
+}
+
+void helper_store_iamr(CPUPPCState *env, target_ulong val)
+{
+    target_ulong old, new, mask;
+
+    if (FIELD_EX64(env->msr, MSR, PR)) {
+        g_assert_not_reached(); /* mtIAMR is privileged */
+    } else if (FIELD_EX64(env->msr, MSR, HV)) {
+        mask = (target_ulong)-1;
+    } else {
+        mask = env->spr[SPR_AMOR];
+    }
+
+    old = env->spr[SPR_IAMR];
+    /* Replace controllable bits with those in val */
+    new = (old & ~mask) | (val & mask);
+
+    if (old != new) {
+        CPUState *cs = env_cpu(env);
+        env->spr[SPR_IAMR] = new;
+        /* IAMR is involved in MMU translations so must flush TLB */
+        tlb_flush(cs);
+    }
+}
+
 void helper_store_hrmor(CPUPPCState *env, target_ulong val)
 {
     if (env->spr[SPR_HRMOR] != val) {
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index ac910151cfa..c5fe3de64e9 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1080,33 +1080,7 @@ void spr_write_excp_vector(DisasContext *ctx, int sprn, int gprn)
 #ifndef CONFIG_USER_ONLY
 void spr_write_amr(DisasContext *ctx, int sprn, int gprn)
 {
-    TCGv t0 = tcg_temp_new();
-    TCGv t1 = tcg_temp_new();
-    TCGv t2 = tcg_temp_new();
-
-    /*
-     * Note, the HV=1 PR=0 case is handled earlier by simply using
-     * spr_write_generic for HV mode in the SPR table
-     */
-
-    /* Build insertion mask into t1 based on context */
-    if (ctx->pr) {
-        gen_load_spr(t1, SPR_UAMOR);
-    } else {
-        gen_load_spr(t1, SPR_AMOR);
-    }
-
-    /* Mask new bits into t2 */
-    tcg_gen_and_tl(t2, t1, cpu_gpr[gprn]);
-
-    /* Load AMR and clear new bits in t0 */
-    gen_load_spr(t0, SPR_AMR);
-    tcg_gen_andc_tl(t0, t0, t1);
-
-    /* Or'in new bits and write it out */
-    tcg_gen_or_tl(t0, t0, t2);
-    gen_store_spr(SPR_AMR, t0);
-    spr_store_dump_spr(SPR_AMR);
+    gen_helper_store_amr(tcg_env, cpu_gpr[gprn]);
 }
 
 void spr_write_uamor(DisasContext *ctx, int sprn, int gprn)
@@ -1138,29 +1112,7 @@ void spr_write_uamor(DisasContext *ctx, int sprn, int gprn)
 
 void spr_write_iamr(DisasContext *ctx, int sprn, int gprn)
 {
-    TCGv t0 = tcg_temp_new();
-    TCGv t1 = tcg_temp_new();
-    TCGv t2 = tcg_temp_new();
-
-    /*
-     * Note, the HV=1 case is handled earlier by simply using
-     * spr_write_generic for HV mode in the SPR table
-     */
-
-    /* Build insertion mask into t1 based on context */
-    gen_load_spr(t1, SPR_AMOR);
-
-    /* Mask new bits into t2 */
-    tcg_gen_and_tl(t2, t1, cpu_gpr[gprn]);
-
-    /* Load AMR and clear new bits in t0 */
-    gen_load_spr(t0, SPR_IAMR);
-    tcg_gen_andc_tl(t0, t0, t1);
-
-    /* Or'in new bits and write it out */
-    tcg_gen_or_tl(t0, t0, t2);
-    gen_store_spr(SPR_IAMR, t0);
-    spr_store_dump_spr(SPR_IAMR);
+    gen_helper_store_iamr(tcg_env, cpu_gpr[gprn]);
 }
 #endif
 #endif
-- 
2.47.1



      parent reply	other threads:[~2025-03-03 11:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-03 11:23 [PATCH 0/3] target/ppc: Fixes for TCG TLB modeling of some MMU SPRs Nicholas Piggin
2025-03-03 11:23 ` [PATCH 1/3] target/ppc: flush TLB on HRMOR and LPCR SPR updates Nicholas Piggin
2025-03-03 11:23 ` [PATCH 2/3] target/ppc: Avoid work if MMU SPRs are written with same value Nicholas Piggin
2025-03-03 11:23 ` Nicholas Piggin [this message]

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=20250303112315.586478-4-npiggin@gmail.com \
    --to=npiggin@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).