qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: qemu-devel@nongnu.org
Cc: david@redhat.com
Subject: [PATCH 5/7] target/s390x: Remove TLB_NOTDIRTY workarounds
Date: Mon,  9 Jan 2023 12:18:54 -0800	[thread overview]
Message-ID: <20230109201856.3916639-6-richard.henderson@linaro.org> (raw)
In-Reply-To: <20230109201856.3916639-1-richard.henderson@linaro.org>

When this code was written, it was using tlb_vaddr_to_host,
which does not handle TLB_DIRTY.  Since then, it has been
converted to probe_access_flags, which does.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/tcg/mem_helper.c | 74 +++++++++++------------------------
 1 file changed, 22 insertions(+), 52 deletions(-)

diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 59237fe7de..f7dc710814 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -122,11 +122,7 @@ typedef struct S390Access {
      * If we can't access the host page directly, we'll have to do I/O access
      * via ld/st helpers. These are internal details, so we store the
      * mmu idx to do the access here instead of passing it around in the
-     * helpers. Maybe, one day we can get rid of ld/st access - once we can
-     * handle TLB_NOTDIRTY differently. We don't expect these special accesses
-     * to trigger exceptions - only if we would have TLB_NOTDIRTY on LAP
-     * pages, we might trigger a new MMU translation - very unlikely that
-     * the mapping changes in between and we would trigger a fault.
+     * helpers.
      */
     int mmu_idx;
 } S390Access;
@@ -224,28 +220,14 @@ static void do_access_memset(CPUS390XState *env, vaddr vaddr, char *haddr,
                              uintptr_t ra)
 {
 #ifdef CONFIG_USER_ONLY
-    g_assert(haddr);
     memset(haddr, byte, size);
 #else
-    MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
-    int i;
-
     if (likely(haddr)) {
         memset(haddr, byte, size);
     } else {
-        /*
-         * Do a single access and test if we can then get access to the
-         * page. This is especially relevant to speed up TLB_NOTDIRTY.
-         */
-        g_assert(size > 0);
-        cpu_stb_mmu(env, vaddr, byte, oi, ra);
-        haddr = tlb_vaddr_to_host(env, vaddr, MMU_DATA_STORE, mmu_idx);
-        if (likely(haddr)) {
-            memset(haddr + 1, byte, size - 1);
-        } else {
-            for (i = 1; i < size; i++) {
-                cpu_stb_mmu(env, vaddr + i, byte, oi, ra);
-            }
+        MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
+        for (int i = 0; i < size; i++) {
+            cpu_stb_mmu(env, vaddr + i, byte, oi, ra);
         }
     }
 #endif
@@ -265,25 +247,18 @@ static void access_memset(CPUS390XState *env, S390Access *desta,
 }
 
 static uint8_t do_access_get_byte(CPUS390XState *env, vaddr vaddr,
-                                  void **haddr, int offset,
+                                  void *haddr, int offset,
                                   int mmu_idx, uintptr_t ra)
 {
 #ifdef CONFIG_USER_ONLY
-    return ldub_p(*haddr + offset);
+    return ldub_p(haddr + offset);
 #else
-    MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
-    uint8_t byte;
-
-    if (likely(*haddr)) {
-        return ldub_p(*haddr + offset);
+    if (likely(haddr)) {
+        return ldub_p(haddr + offset);
+    } else {
+        MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
+        return cpu_ldb_mmu(env, vaddr + offset, oi, ra);
     }
-    /*
-     * Do a single access and test if we can then get access to the
-     * page. This is especially relevant to speed up TLB_NOTDIRTY.
-     */
-    byte = cpu_ldb_mmu(env, vaddr + offset, oi, ra);
-    *haddr = tlb_vaddr_to_host(env, vaddr, MMU_DATA_LOAD, mmu_idx);
-    return byte;
 #endif
 }
 
@@ -291,32 +266,27 @@ static uint8_t access_get_byte(CPUS390XState *env, S390Access *access,
                                int offset, uintptr_t ra)
 {
     if (offset < access->size1) {
-        return do_access_get_byte(env, access->vaddr1, &access->haddr1,
+        return do_access_get_byte(env, access->vaddr1, access->haddr1,
                                   offset, access->mmu_idx, ra);
     }
-    return do_access_get_byte(env, access->vaddr2, &access->haddr2,
+    return do_access_get_byte(env, access->vaddr2, access->haddr2,
                               offset - access->size1, access->mmu_idx, ra);
 }
 
-static void do_access_set_byte(CPUS390XState *env, vaddr vaddr, void **haddr,
+static void do_access_set_byte(CPUS390XState *env, vaddr vaddr, void *haddr,
                                int offset, uint8_t byte, int mmu_idx,
                                uintptr_t ra)
 {
 #ifdef CONFIG_USER_ONLY
-    stb_p(*haddr + offset, byte);
+    stb_p(haddr + offset, byte);
 #else
-    MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
 
-    if (likely(*haddr)) {
-        stb_p(*haddr + offset, byte);
-        return;
+    if (likely(haddr)) {
+        stb_p(haddr + offset, byte);
+    } else {
+        MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
+        cpu_stb_mmu(env, vaddr + offset, byte, oi, ra);
     }
-    /*
-     * Do a single access and test if we can then get access to the
-     * page. This is especially relevant to speed up TLB_NOTDIRTY.
-     */
-    cpu_stb_mmu(env, vaddr + offset, byte, oi, ra);
-    *haddr = tlb_vaddr_to_host(env, vaddr, MMU_DATA_STORE, mmu_idx);
 #endif
 }
 
@@ -324,10 +294,10 @@ static void access_set_byte(CPUS390XState *env, S390Access *access,
                             int offset, uint8_t byte, uintptr_t ra)
 {
     if (offset < access->size1) {
-        do_access_set_byte(env, access->vaddr1, &access->haddr1, offset, byte,
+        do_access_set_byte(env, access->vaddr1, access->haddr1, offset, byte,
                            access->mmu_idx, ra);
     } else {
-        do_access_set_byte(env, access->vaddr2, &access->haddr2,
+        do_access_set_byte(env, access->vaddr2, access->haddr2,
                            offset - access->size1, byte, access->mmu_idx, ra);
     }
 }
-- 
2.34.1



  parent reply	other threads:[~2023-01-09 20:26 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-09 20:18 [PATCH 0/7] target/s390x: mem_helper.c cleanups Richard Henderson
2023-01-09 20:18 ` [PATCH 1/7] target/s390x: Fix s390_probe_access for user-only Richard Henderson
2023-01-11 10:12   ` David Hildenbrand
2023-03-15 15:30   ` Thomas Huth
2023-03-16 14:29     ` Richard Henderson
2023-01-09 20:18 ` [PATCH 2/7] target/s390x: Pass S390Access pointer into access_prepare Richard Henderson
2023-01-11 10:13   ` David Hildenbrand
2023-01-09 20:18 ` [PATCH 3/7] target/s390x: Use void* for haddr in S390Access Richard Henderson
2023-01-11 10:15   ` David Hildenbrand
2023-01-09 20:18 ` [PATCH 4/7] target/s390x: Tidy access_prepare_nf Richard Henderson
2023-01-11 10:15   ` David Hildenbrand
2023-01-09 20:18 ` Richard Henderson [this message]
2023-01-11 10:19   ` [PATCH 5/7] target/s390x: Remove TLB_NOTDIRTY workarounds David Hildenbrand
2023-01-09 20:18 ` [PATCH 6/7] target/s390x: Inline do_access_{get,set}_byte Richard Henderson
2023-01-11 10:20   ` David Hildenbrand
2023-01-09 20:18 ` [PATCH 7/7] target/s390x: Hoist some computation in access_memmove Richard Henderson
2023-01-11 10:22   ` David Hildenbrand
2023-02-20 17:53 ` [PATCH 0/7] target/s390x: mem_helper.c cleanups Thomas Huth

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=20230109201856.3916639-6-richard.henderson@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=david@redhat.com \
    --cc=qemu-devel@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).