[v4] Move rom and notdirty handling to cputlb

[PATCH v4 12/16] cputlb: Handle TLB_NOTDIRTY in probe_access

Posted by Richard Henderson 6 years, 4 months ago

We can use notdirty_write for the write and
return a valid host pointer for this case.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cputlb.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 09b0df87c6..d0bdef1eb3 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1167,16 +1167,24 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,
         return NULL;
     }
 
-    /* Handle watchpoints.  */
-    if (tlb_addr & TLB_WATCHPOINT) {
-        cpu_check_watchpoint(env_cpu(env), addr, size,
-                             env_tlb(env)->d[mmu_idx].iotlb[index].attrs,
-                             wp_access, retaddr);
-    }
+    if (unlikely(tlb_addr & TLB_FLAGS_MASK)) {
+        CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
 
-    /* Reject I/O access, or other required slow-path.  */
-    if (tlb_addr & (TLB_NOTDIRTY | TLB_MMIO | TLB_BSWAP | TLB_ROM)) {
-        return NULL;
+        /* Reject I/O access, or other required slow-path.  */
+        if (tlb_addr & (TLB_MMIO | TLB_BSWAP | TLB_ROM)) {
+            return NULL;
+        }
+
+        /* Handle watchpoints.  */
+        if (tlb_addr & TLB_WATCHPOINT) {
+            cpu_check_watchpoint(env_cpu(env), addr, size,
+                                 iotlbentry->attrs, wp_access, retaddr);
+        }
+
+        /* Handle clean RAM pages.  */
+        if (tlb_addr & TLB_NOTDIRTY) {
+            notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr);
+        }
     }
 
     return (void *)((uintptr_t)addr + entry->addend);
-- 
2.17.1

Re: [PATCH v4 12/16] cputlb: Handle TLB_NOTDIRTY in probe_access

Posted by David Hildenbrand 6 years, 4 months ago

On 24.09.19 01:00, Richard Henderson wrote:
> We can use notdirty_write for the write and
> return a valid host pointer for this case.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/cputlb.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 09b0df87c6..d0bdef1eb3 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1167,16 +1167,24 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,
>          return NULL;
>      }
>  
> -    /* Handle watchpoints.  */
> -    if (tlb_addr & TLB_WATCHPOINT) {
> -        cpu_check_watchpoint(env_cpu(env), addr, size,
> -                             env_tlb(env)->d[mmu_idx].iotlb[index].attrs,
> -                             wp_access, retaddr);
> -    }
> +    if (unlikely(tlb_addr & TLB_FLAGS_MASK)) {
> +        CPUIOTLBEntry *iotlbentry = &env_tlb(env)->d[mmu_idx].iotlb[index];
>  
> -    /* Reject I/O access, or other required slow-path.  */
> -    if (tlb_addr & (TLB_NOTDIRTY | TLB_MMIO | TLB_BSWAP | TLB_ROM)) {
> -        return NULL;
> +        /* Reject I/O access, or other required slow-path.  */
> +        if (tlb_addr & (TLB_MMIO | TLB_BSWAP | TLB_ROM)) {
> +            return NULL;
> +        }
> +
> +        /* Handle watchpoints.  */
> +        if (tlb_addr & TLB_WATCHPOINT) {
> +            cpu_check_watchpoint(env_cpu(env), addr, size,
> +                                 iotlbentry->attrs, wp_access, retaddr);
> +        }
> +
> +        /* Handle clean RAM pages.  */
> +        if (tlb_addr & TLB_NOTDIRTY) {
> +            notdirty_write(env_cpu(env), addr, size, iotlbentry, retaddr);
> +        }
>      }
>  
>      return (void *)((uintptr_t)addr + entry->addend);
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [PATCH v4 12/16] cputlb: Handle TLB_NOTDIRTY in probe_access

Posted by Alex Bennée 6 years, 4 months ago

Richard Henderson <richard.henderson@linaro.org> writes:

> We can use notdirty_write for the write and
> return a valid host pointer for this case.

nit: reflow the text

>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/cputlb.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 09b0df87c6..d0bdef1eb3 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1167,16 +1167,24 @@ void *probe_access(CPUArchState *env, target_ulong addr, int size,
>          return NULL;
>      }
>
> -    /* Handle watchpoints.  */
> -    if (tlb_addr & TLB_WATCHPOINT) {
> -        cpu_check_watchpoint(env_cpu(env), addr, size,
> -                             env_tlb(env)->d[mmu_idx].iotlb[index].attrs,
> -                             wp_access, retaddr);
> -    }
> +    if (unlikely(tlb_addr & TLB_FLAGS_MASK)) {
> +        CPUIOTLBEntry *iotlbentry =
> &env_tlb(env)->d[mmu_idx].iotlb[index];

I was going to say we compute this early but I'm assuming the compiler
can figure that out if it needs to.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


--
Alex Bennée