[Qemu-devel] [PATCH v2 0/7] tcg: probe_write() refactorings

[Qemu-devel] [PATCH v2 0/7] tcg: probe_write() refactorings

[David Hildenbrand]

Based on tcg-next. The last two patches from v1 are now part of
"[PATCH 0/6] exec: Cleanup watchpoints" from richard.

Fix and refactore some things around probe_write() in s390x code. Introduce
probe_write() for CONFIG_USER_ONLY.

This is the first step of some changes necessary to handle fault-safe
access accross multiple pages on s390x. The next step is making
probe_write() return an address, similar to tlb_vaddr_to_host(), and
introduing probe_read().

v1 -> v2:
- "tcg: Factor out CONFIG_USER_ONLY probe_write() from s390x code"
-- Perform only a single !guest_addr_valid(addr) check.
- "tcg: Enforce single page access in probe_write()"
-- Also add the check for CONFIG_USER_ONLY

Cc: Richard Henderson <rth@twiddle.net>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Riku Voipio <riku.voipio@iki.fi>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Aleksandar Markovic <amarkovic@wavecomp.com>
Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
Cc: Cornelia Huck <cohuck@redhat.com>

David Hildenbrand (7):
  s390x/tcg: Use guest_addr_valid() instead of h2g_valid() in
    probe_write_access()
  s390x/tcg: Fix length calculation in probe_write_access()
  tcg: Factor out CONFIG_USER_ONLY probe_write() from s390x code
  tcg: Enforce single page access in probe_write()
  mips/tcg: Call probe_write() for CONFIG_USER_ONLY as well
  hppa/tcg: Call probe_write() also for CONFIG_USER_ONLY
  s390x/tcg: Pass a size to probe_write() in do_csst()

 accel/tcg/cputlb.c        |  2 ++
 accel/tcg/user-exec.c     | 17 +++++++++++++++++
 include/exec/exec-all.h   |  4 ++--
 target/hppa/op_helper.c   |  2 --
 target/mips/op_helper.c   |  8 +++-----
 target/s390x/mem_helper.c | 13 ++-----------
 6 files changed, 26 insertions(+), 20 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH v2 1/7] s390x/tcg: Use guest_addr_valid() instead of h2g_valid() in probe_write_access()

[David Hildenbrand]

If I'm not completely wrong, we are dealing with guest addresses here
and not with host addresses. Use the right check.

Fixes: c5a7392cfb96 ("s390x/tcg: Provide probe_write_access helper")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mem_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 91ba2e03d9..7819aca15d 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -2616,7 +2616,7 @@ void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len,
                         uintptr_t ra)
 {
 #ifdef CONFIG_USER_ONLY
-    if (!h2g_valid(addr) || !h2g_valid(addr + len - 1) ||
+    if (!guest_addr_valid(addr) || !guest_addr_valid(addr + len - 1) ||
         page_check_range(addr, len, PAGE_WRITE) < 0) {
         s390_program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO, ra);
     }
-- 
2.21.0

[Qemu-devel] [PATCH v2 2/7] s390x/tcg: Fix length calculation in probe_write_access()

[David Hildenbrand]

Hm... how did that "-" slip in (-TAGRET_PAGE_SIZE would be correct). This
currently makes us exceed one page in a single probe_write() call,
essentially leaving some memory unchecked.

Fixes: c5a7392cfb96 ("s390x/tcg: Provide probe_write_access helper")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mem_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 7819aca15d..4b43440e89 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -2623,7 +2623,7 @@ void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len,
 #else
     /* test the actual access, not just any access to the page due to LAP */
     while (len) {
-        const uint64_t pagelen = -(addr | -TARGET_PAGE_MASK);
+        const uint64_t pagelen = -(addr | TARGET_PAGE_MASK);
         const uint64_t curlen = MIN(pagelen, len);
 
         probe_write(env, addr, curlen, cpu_mmu_index(env, false), ra);
-- 
2.21.0

[Qemu-devel] [PATCH v2 3/7] tcg: Factor out CONFIG_USER_ONLY probe_write() from s390x code

[David Hildenbrand]

Factor it out into common code. Similar to the !CONFIG_USER_ONLY variant,
let's not allow to cross page boundaries.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 accel/tcg/user-exec.c     | 15 +++++++++++++++
 include/exec/exec-all.h   |  4 ++--
 target/s390x/mem_helper.c |  7 -------
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 897d1571c4..68f4425cbc 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -188,6 +188,21 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info,
     g_assert_not_reached();
 }
 
+void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
+                 uintptr_t retaddr)
+{
+    CPUState *cpu = env_cpu(env);
+    CPUClass *cc;
+
+    if (!guest_addr_valid(addr) ||
+        page_check_range(addr, size, PAGE_WRITE) < 0) {
+        cc = CPU_GET_CLASS(cpu);
+        cc->tlb_fill(cpu, addr, size, MMU_DATA_STORE, MMU_USER_IDX, false,
+                     retaddr);
+        g_assert_not_reached();
+    }
+}
+
 #if defined(__i386__)
 
 #if defined(__NetBSD__)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 135aeaab0d..cbcc85add3 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -260,8 +260,6 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr,
 void tlb_set_page(CPUState *cpu, target_ulong vaddr,
                   hwaddr paddr, int prot,
                   int mmu_idx, target_ulong size);
-void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
-                 uintptr_t retaddr);
 #else
 static inline void tlb_init(CPUState *cpu)
 {
@@ -312,6 +310,8 @@ static inline void tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu,
 {
 }
 #endif
+void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
+                 uintptr_t retaddr);
 
 #define CODE_GEN_ALIGN           16 /* must be >= of the size of a icache line */
 
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 4b43440e89..fdff60ce5d 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -2615,12 +2615,6 @@ uint32_t HELPER(cu42)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t m3)
 void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len,
                         uintptr_t ra)
 {
-#ifdef CONFIG_USER_ONLY
-    if (!guest_addr_valid(addr) || !guest_addr_valid(addr + len - 1) ||
-        page_check_range(addr, len, PAGE_WRITE) < 0) {
-        s390_program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO, ra);
-    }
-#else
     /* test the actual access, not just any access to the page due to LAP */
     while (len) {
         const uint64_t pagelen = -(addr | TARGET_PAGE_MASK);
@@ -2630,7 +2624,6 @@ void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len,
         addr = wrap_address(env, addr + curlen);
         len -= curlen;
     }
-#endif
 }
 
 void HELPER(probe_write_access)(CPUS390XState *env, uint64_t addr, uint64_t len)
-- 
2.21.0

[Qemu-devel] [PATCH v2 4/7] tcg: Enforce single page access in probe_write()

[David Hildenbrand]

Let's enforce the interface restriction.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 accel/tcg/cputlb.c    | 2 ++
 accel/tcg/user-exec.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 7fc7aa9482..09fe4cdcc4 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1088,6 +1088,8 @@ void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
     CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
     target_ulong tlb_addr = tlb_addr_write(entry);
 
+    g_assert(-(addr | TARGET_PAGE_MASK) >= size);
+
     if (unlikely(!tlb_hit(tlb_addr, addr))) {
         if (!VICTIM_TLB_HIT(addr_write, addr)) {
             tlb_fill(env_cpu(env), addr, size, MMU_DATA_STORE,
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 68f4425cbc..b25a342eaa 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -194,6 +194,8 @@ void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
     CPUState *cpu = env_cpu(env);
     CPUClass *cc;
 
+    g_assert(-(addr | TARGET_PAGE_MASK) >= size);
+
     if (!guest_addr_valid(addr) ||
         page_check_range(addr, size, PAGE_WRITE) < 0) {
         cc = CPU_GET_CLASS(cpu);
-- 
2.21.0

[Qemu-devel] [PATCH v2 5/7] mips/tcg: Call probe_write() for CONFIG_USER_ONLY as well

[David Hildenbrand]

Let's call it also for CONFIG_USER_ONLY. While at it, add a FIXME and get
rid of one local variable.

MIPS code probably needs a bigger refactoring in regards of
ensure_writable_pages(), similar to s390x, so for example, watchpoints
can be handled reliably later. The actually accessed addresses should
be probed only, not full pages.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/mips/op_helper.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 34bcc8d884..08d9a4f9f1 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -4537,16 +4537,14 @@ static inline void ensure_writable_pages(CPUMIPSState *env,
                                          int mmu_idx,
                                          uintptr_t retaddr)
 {
-#if !defined(CONFIG_USER_ONLY)
-    target_ulong page_addr;
+    /* FIXME: Probe the actual accesses (pass and use a size) */
     if (unlikely(MSA_PAGESPAN(addr))) {
         /* first page */
         probe_write(env, addr, 0, mmu_idx, retaddr);
         /* second page */
-        page_addr = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
-        probe_write(env, page_addr, 0, mmu_idx, retaddr);
+        addr = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+        probe_write(env, addr, 0, mmu_idx, retaddr);
     }
-#endif
 }
 
 void helper_msa_st_b(CPUMIPSState *env, uint32_t wd,
-- 
2.21.0

[Qemu-devel] [PATCH v2 6/7] hppa/tcg: Call probe_write() also for CONFIG_USER_ONLY

[David Hildenbrand]

We now have a variant for CONFIG_USER_ONLY as well.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/hppa/op_helper.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target/hppa/op_helper.c b/target/hppa/op_helper.c
index df0f1361ef..f0516e81f1 100644
--- a/target/hppa/op_helper.c
+++ b/target/hppa/op_helper.c
@@ -137,9 +137,7 @@ static void do_stby_e(CPUHPPAState *env, target_ulong addr, target_ureg val,
     default:
         /* Nothing is stored, but protection is checked and the
            cacheline is marked dirty.  */
-#ifndef CONFIG_USER_ONLY
         probe_write(env, addr, 0, cpu_mmu_index(env, 0), ra);
-#endif
         break;
     }
 }
-- 
2.21.0

[Qemu-devel] [PATCH v2 7/7] s390x/tcg: Pass a size to probe_write() in do_csst()

[David Hildenbrand]

... and also call it for CONFIG_USER_ONLY. This function probably will
also need some refactoring in regards to probing, however, we'll have to
come back to that later, once cleaning up the other mem helpers.

The alignment check always makes sure that the write access falls into a
single page.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mem_helper.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index fdff60ce5d..29fcce426e 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1443,9 +1443,7 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t r3, uint64_t a1,
     }
 
     /* Sanity check writability of the store address.  */
-#ifndef CONFIG_USER_ONLY
-    probe_write(env, a2, 0, mem_idx, ra);
-#endif
+    probe_write(env, a2, 1 << sc, mem_idx, ra);
 
     /*
      * Note that the compare-and-swap is atomic, and the store is atomic,
-- 
2.21.0

Re: [Qemu-devel] [PATCH v2 3/7] tcg: Factor out CONFIG_USER_ONLY probe_write() from s390x code

[Richard Henderson]

On 8/26/19 12:51 AM, David Hildenbrand wrote:
> Factor it out into common code. Similar to the !CONFIG_USER_ONLY variant,
> let's not allow to cross page boundaries.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  accel/tcg/user-exec.c     | 15 +++++++++++++++
>  include/exec/exec-all.h   |  4 ++--
>  target/s390x/mem_helper.c |  7 -------
>  3 files changed, 17 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v2 4/7] tcg: Enforce single page access in probe_write()

[Richard Henderson]

On 8/26/19 12:51 AM, David Hildenbrand wrote:
> Let's enforce the interface restriction.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  accel/tcg/cputlb.c    | 2 ++
>  accel/tcg/user-exec.c | 2 ++
>  2 files changed, 4 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [qemu-s390x] [PATCH v2 0/7] tcg: probe_write() refactorings

[David Hildenbrand]

On 26.08.19 09:51, David Hildenbrand wrote:
> Based on tcg-next. The last two patches from v1 are now part of
> "[PATCH 0/6] exec: Cleanup watchpoints" from richard.
> 
> Fix and refactore some things around probe_write() in s390x code. Introduce
> probe_write() for CONFIG_USER_ONLY.
> 
> This is the first step of some changes necessary to handle fault-safe
> access accross multiple pages on s390x. The next step is making
> probe_write() return an address, similar to tlb_vaddr_to_host(), and
> introduing probe_read().
> 
> v1 -> v2:
> - "tcg: Factor out CONFIG_USER_ONLY probe_write() from s390x code"
> -- Perform only a single !guest_addr_valid(addr) check.
> - "tcg: Enforce single page access in probe_write()"
> -- Also add the check for CONFIG_USER_ONLY
> 
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Riku Voipio <riku.voipio@iki.fi>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Aleksandar Markovic <amarkovic@wavecomp.com>
> Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> 
> David Hildenbrand (7):
>   s390x/tcg: Use guest_addr_valid() instead of h2g_valid() in
>     probe_write_access()
>   s390x/tcg: Fix length calculation in probe_write_access()
>   tcg: Factor out CONFIG_USER_ONLY probe_write() from s390x code
>   tcg: Enforce single page access in probe_write()
>   mips/tcg: Call probe_write() for CONFIG_USER_ONLY as well
>   hppa/tcg: Call probe_write() also for CONFIG_USER_ONLY
>   s390x/tcg: Pass a size to probe_write() in do_csst()
> 
>  accel/tcg/cputlb.c        |  2 ++
>  accel/tcg/user-exec.c     | 17 +++++++++++++++++
>  include/exec/exec-all.h   |  4 ++--
>  target/hppa/op_helper.c   |  2 --
>  target/mips/op_helper.c   |  8 +++-----
>  target/s390x/mem_helper.c | 13 ++-----------
>  6 files changed, 26 insertions(+), 20 deletions(-)
> 

Richard, in case there is no more feedback, will you take these patches
via your tree?

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [qemu-s390x] [PATCH v2 0/7] tcg: probe_write() refactorings

[Richard Henderson]

On 8/27/19 9:09 AM, David Hildenbrand wrote:
>> David Hildenbrand (7):
>>   s390x/tcg: Use guest_addr_valid() instead of h2g_valid() in
>>     probe_write_access()
>>   s390x/tcg: Fix length calculation in probe_write_access()
>>   tcg: Factor out CONFIG_USER_ONLY probe_write() from s390x code
>>   tcg: Enforce single page access in probe_write()
>>   mips/tcg: Call probe_write() for CONFIG_USER_ONLY as well
>>   hppa/tcg: Call probe_write() also for CONFIG_USER_ONLY
>>   s390x/tcg: Pass a size to probe_write() in do_csst()
>>
>>  accel/tcg/cputlb.c        |  2 ++
>>  accel/tcg/user-exec.c     | 17 +++++++++++++++++
>>  include/exec/exec-all.h   |  4 ++--
>>  target/hppa/op_helper.c   |  2 --
>>  target/mips/op_helper.c   |  8 +++-----
>>  target/s390x/mem_helper.c | 13 ++-----------
>>  6 files changed, 26 insertions(+), 20 deletions(-)
>>
> 
> Richard, in case there is no more feedback, will you take these patches
> via your tree?

I can do.


r~

Re: [Qemu-devel] [PATCH v2 1/7] s390x/tcg: Use guest_addr_valid() instead of h2g_valid() in probe_write_access()

[Cornelia Huck]

On Mon, 26 Aug 2019 09:51:06 +0200
David Hildenbrand <david@redhat.com> wrote:

> If I'm not completely wrong, we are dealing with guest addresses here
> and not with host addresses. Use the right check.
> 
> Fixes: c5a7392cfb96 ("s390x/tcg: Provide probe_write_access helper")
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/mem_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH v2 2/7] s390x/tcg: Fix length calculation in probe_write_access()

[Cornelia Huck]

On Mon, 26 Aug 2019 09:51:07 +0200
David Hildenbrand <david@redhat.com> wrote:

> Hm... how did that "-" slip in (-TAGRET_PAGE_SIZE would be correct). This

s/TAGRET/TARGET/

> currently makes us exceed one page in a single probe_write() call,
> essentially leaving some memory unchecked.
> 
> Fixes: c5a7392cfb96 ("s390x/tcg: Provide probe_write_access helper")
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/mem_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH v2 5/7] mips/tcg: Call probe_write() for CONFIG_USER_ONLY as well

[Richard Henderson]

Pinging the mips maintainers.

r~

On 8/26/19 12:51 AM, David Hildenbrand wrote:
> Let's call it also for CONFIG_USER_ONLY. While at it, add a FIXME and get
> rid of one local variable.
> 
> MIPS code probably needs a bigger refactoring in regards of
> ensure_writable_pages(), similar to s390x, so for example, watchpoints
> can be handled reliably later. The actually accessed addresses should
> be probed only, not full pages.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/mips/op_helper.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index 34bcc8d884..08d9a4f9f1 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -4537,16 +4537,14 @@ static inline void ensure_writable_pages(CPUMIPSState *env,
>                                           int mmu_idx,
>                                           uintptr_t retaddr)
>  {
> -#if !defined(CONFIG_USER_ONLY)
> -    target_ulong page_addr;
> +    /* FIXME: Probe the actual accesses (pass and use a size) */
>      if (unlikely(MSA_PAGESPAN(addr))) {
>          /* first page */
>          probe_write(env, addr, 0, mmu_idx, retaddr);
>          /* second page */
> -        page_addr = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> -        probe_write(env, page_addr, 0, mmu_idx, retaddr);
> +        addr = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> +        probe_write(env, addr, 0, mmu_idx, retaddr);
>      }
> -#endif
>  }
>  
>  void helper_msa_st_b(CPUMIPSState *env, uint32_t wd,
> 

Re: [Qemu-devel] [PATCH v2 5/7] mips/tcg: Call probe_write() for CONFIG_USER_ONLY as well

[Aleksandar Markovic]

26.08.2019. 09.52, "David Hildenbrand" <david@redhat.com> је написао/ла:
>
> Let's call it also for CONFIG_USER_ONLY. While at it, add a FIXME and get
> rid of one local variable.
>
> MIPS code probably needs a bigger refactoring in regards of
> ensure_writable_pages(), similar to s390x, so for example, watchpoints
> can be handled reliably later. The actually accessed addresses should
> be probed only, not full pages.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

>  target/mips/op_helper.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index 34bcc8d884..08d9a4f9f1 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -4537,16 +4537,14 @@ static inline void
ensure_writable_pages(CPUMIPSState *env,
>                                           int mmu_idx,
>                                           uintptr_t retaddr)
>  {
> -#if !defined(CONFIG_USER_ONLY)
> -    target_ulong page_addr;
> +    /* FIXME: Probe the actual accesses (pass and use a size) */
>      if (unlikely(MSA_PAGESPAN(addr))) {
>          /* first page */
>          probe_write(env, addr, 0, mmu_idx, retaddr);
>          /* second page */
> -        page_addr = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> -        probe_write(env, page_addr, 0, mmu_idx, retaddr);
> +        addr = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
> +        probe_write(env, addr, 0, mmu_idx, retaddr);
>      }
> -#endif
>  }
>
>  void helper_msa_st_b(CPUMIPSState *env, uint32_t wd,
> --
> 2.21.0
>
>

Re: [Qemu-devel] [qemu-s390x] [PATCH v2 0/7] tcg: probe_write() refactorings

[Richard Henderson]

> On 8/27/19 9:09 AM, David Hildenbrand wrote:
>> Richard, in case there is no more feedback, will you take these patches
>> via your tree?

Queued to tcg-next.


r~