[PATCH v2 27/54] accel/tcg: Return CPUTLBEntryFull not pointer in probe_access_internal

Richard Henderson posted 54 patches 1 week, 2 days ago
[PATCH v2 27/54] accel/tcg: Return CPUTLBEntryFull not pointer in probe_access_internal
Posted by Richard Henderson 1 week, 2 days ago
Return a copy of the structure, not a pointer.

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

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 84e7e633e3..41b2f76cc9 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1364,7 +1364,7 @@ static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
 static int probe_access_internal(CPUState *cpu, vaddr addr,
                                  int fault_size, MMUAccessType access_type,
                                  int mmu_idx, bool nonfault,
-                                 void **phost, CPUTLBEntryFull **pfull,
+                                 void **phost, CPUTLBEntryFull *pfull,
                                  uintptr_t retaddr, bool check_mem_cbs)
 {
     uintptr_t index = tlb_index(cpu, mmu_idx, addr);
@@ -1379,7 +1379,7 @@ static int probe_access_internal(CPUState *cpu, vaddr addr,
                                 0, fault_size, nonfault, retaddr)) {
                 /* Non-faulting page table read failed.  */
                 *phost = NULL;
-                *pfull = NULL;
+                memset(pfull, 0, sizeof(*pfull));
                 return TLB_INVALID_MASK;
             }
 
@@ -1398,8 +1398,9 @@ static int probe_access_internal(CPUState *cpu, vaddr addr,
     }
     flags &= tlb_addr;
 
-    *pfull = full = &cpu->neg.tlb.d[mmu_idx].fulltlb[index];
+    full = &cpu->neg.tlb.d[mmu_idx].fulltlb[index];
     flags |= full->slow_flags[access_type];
+    *pfull = *full;
 
     /*
      * Fold all "mmio-like" bits, and required plugin callbacks, to TLB_MMIO.
@@ -1423,19 +1424,17 @@ int probe_access_full(CPUArchState *env, vaddr addr, int size,
                       bool nonfault, void **phost, CPUTLBEntryFull *pfull,
                       uintptr_t retaddr)
 {
-    CPUTLBEntryFull *full;
     int flags = probe_access_internal(env_cpu(env), addr, size, access_type,
-                                      mmu_idx, nonfault, phost, &full, retaddr,
+                                      mmu_idx, nonfault, phost, pfull, retaddr,
                                       true);
 
     /* Handle clean RAM pages.  */
     if (unlikely(flags & TLB_NOTDIRTY)) {
         int dirtysize = size == 0 ? 1 : size;
-        notdirty_write(env_cpu(env), addr, dirtysize, full, retaddr);
+        notdirty_write(env_cpu(env), addr, dirtysize, pfull, retaddr);
         flags &= ~TLB_NOTDIRTY;
     }
 
-    *pfull = *full;
     return flags;
 }
 
@@ -1444,25 +1443,22 @@ int probe_access_full_mmu(CPUArchState *env, vaddr addr, int size,
                           void **phost, CPUTLBEntryFull *pfull)
 {
     void *discard_phost;
-    CPUTLBEntryFull *full;
+    CPUTLBEntryFull discard_full;
 
     /* privately handle users that don't need full results */
     phost = phost ? phost : &discard_phost;
+    pfull = pfull ? pfull : &discard_full;
 
     int flags = probe_access_internal(env_cpu(env), addr, size, access_type,
-                                      mmu_idx, true, phost, &full, 0, false);
+                                      mmu_idx, true, phost, pfull, 0, false);
 
     /* Handle clean RAM pages.  */
     if (unlikely(flags & TLB_NOTDIRTY)) {
         int dirtysize = size == 0 ? 1 : size;
-        notdirty_write(env_cpu(env), addr, dirtysize, full, 0);
+        notdirty_write(env_cpu(env), addr, dirtysize, pfull, 0);
         flags &= ~TLB_NOTDIRTY;
     }
 
-    if (pfull) {
-        *pfull = *full;
-    }
-
     return flags;
 }
 
@@ -1470,7 +1466,7 @@ int probe_access_flags(CPUArchState *env, vaddr addr, int size,
                        MMUAccessType access_type, int mmu_idx,
                        bool nonfault, void **phost, uintptr_t retaddr)
 {
-    CPUTLBEntryFull *full;
+    CPUTLBEntryFull full;
     int flags;
 
     g_assert(-(addr | TARGET_PAGE_MASK) >= size);
@@ -1482,7 +1478,7 @@ int probe_access_flags(CPUArchState *env, vaddr addr, int size,
     /* Handle clean RAM pages. */
     if (unlikely(flags & TLB_NOTDIRTY)) {
         int dirtysize = size == 0 ? 1 : size;
-        notdirty_write(env_cpu(env), addr, dirtysize, full, retaddr);
+        notdirty_write(env_cpu(env), addr, dirtysize, &full, retaddr);
         flags &= ~TLB_NOTDIRTY;
     }
 
@@ -1492,7 +1488,7 @@ int probe_access_flags(CPUArchState *env, vaddr addr, int size,
 void *probe_access(CPUArchState *env, vaddr addr, int size,
                    MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
 {
-    CPUTLBEntryFull *full;
+    CPUTLBEntryFull full;
     void *host;
     int flags;
 
@@ -1513,12 +1509,12 @@ void *probe_access(CPUArchState *env, vaddr addr, int size,
             int wp_access = (access_type == MMU_DATA_STORE
                              ? BP_MEM_WRITE : BP_MEM_READ);
             cpu_check_watchpoint(env_cpu(env), addr, size,
-                                 full->attrs, wp_access, retaddr);
+                                 full.attrs, wp_access, retaddr);
         }
 
         /* Handle clean RAM pages.  */
         if (flags & TLB_NOTDIRTY) {
-            notdirty_write(env_cpu(env), addr, size, full, retaddr);
+            notdirty_write(env_cpu(env), addr, size, &full, retaddr);
         }
     }
 
@@ -1528,7 +1524,7 @@ void *probe_access(CPUArchState *env, vaddr addr, int size,
 void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
                         MMUAccessType access_type, int mmu_idx)
 {
-    CPUTLBEntryFull *full;
+    CPUTLBEntryFull full;
     void *host;
     int flags;
 
@@ -1552,7 +1548,7 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
 tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr,
                                         void **hostp)
 {
-    CPUTLBEntryFull *full;
+    CPUTLBEntryFull full;
     void *p;
 
     (void)probe_access_internal(env_cpu(env), addr, 1, MMU_INST_FETCH,
@@ -1562,7 +1558,7 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr,
         return -1;
     }
 
-    if (full->lg_page_size < TARGET_PAGE_BITS) {
+    if (full.lg_page_size < TARGET_PAGE_BITS) {
         return -1;
     }
 
-- 
2.43.0
Re: [PATCH v2 27/54] accel/tcg: Return CPUTLBEntryFull not pointer in probe_access_internal
Posted by Pierrick Bouvier 1 week, 1 day ago
On 11/14/24 08:01, Richard Henderson wrote:
> Return a copy of the structure, not a pointer.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/cputlb.c | 40 ++++++++++++++++++----------------------
>   1 file changed, 18 insertions(+), 22 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 84e7e633e3..41b2f76cc9 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1364,7 +1364,7 @@ static void notdirty_write(CPUState *cpu, vaddr mem_vaddr, unsigned size,
>   static int probe_access_internal(CPUState *cpu, vaddr addr,
>                                    int fault_size, MMUAccessType access_type,
>                                    int mmu_idx, bool nonfault,
> -                                 void **phost, CPUTLBEntryFull **pfull,
> +                                 void **phost, CPUTLBEntryFull *pfull,
>                                    uintptr_t retaddr, bool check_mem_cbs)
>   {
>       uintptr_t index = tlb_index(cpu, mmu_idx, addr);
> @@ -1379,7 +1379,7 @@ static int probe_access_internal(CPUState *cpu, vaddr addr,
>                                   0, fault_size, nonfault, retaddr)) {
>                   /* Non-faulting page table read failed.  */
>                   *phost = NULL;
> -                *pfull = NULL;
> +                memset(pfull, 0, sizeof(*pfull));
>                   return TLB_INVALID_MASK;
>               }
>   
> @@ -1398,8 +1398,9 @@ static int probe_access_internal(CPUState *cpu, vaddr addr,
>       }
>       flags &= tlb_addr;
>   
> -    *pfull = full = &cpu->neg.tlb.d[mmu_idx].fulltlb[index];
> +    full = &cpu->neg.tlb.d[mmu_idx].fulltlb[index];
>       flags |= full->slow_flags[access_type];
> +    *pfull = *full;
>   
>       /*
>        * Fold all "mmio-like" bits, and required plugin callbacks, to TLB_MMIO.
> @@ -1423,19 +1424,17 @@ int probe_access_full(CPUArchState *env, vaddr addr, int size,
>                         bool nonfault, void **phost, CPUTLBEntryFull *pfull,
>                         uintptr_t retaddr)
>   {
> -    CPUTLBEntryFull *full;
>       int flags = probe_access_internal(env_cpu(env), addr, size, access_type,
> -                                      mmu_idx, nonfault, phost, &full, retaddr,
> +                                      mmu_idx, nonfault, phost, pfull, retaddr,
>                                         true);
>   
>       /* Handle clean RAM pages.  */
>       if (unlikely(flags & TLB_NOTDIRTY)) {
>           int dirtysize = size == 0 ? 1 : size;
> -        notdirty_write(env_cpu(env), addr, dirtysize, full, retaddr);
> +        notdirty_write(env_cpu(env), addr, dirtysize, pfull, retaddr);
>           flags &= ~TLB_NOTDIRTY;
>       }
>   
> -    *pfull = *full;
>       return flags;
>   }
>   
> @@ -1444,25 +1443,22 @@ int probe_access_full_mmu(CPUArchState *env, vaddr addr, int size,
>                             void **phost, CPUTLBEntryFull *pfull)
>   {
>       void *discard_phost;
> -    CPUTLBEntryFull *full;
> +    CPUTLBEntryFull discard_full;
>   
>       /* privately handle users that don't need full results */
>       phost = phost ? phost : &discard_phost;
> +    pfull = pfull ? pfull : &discard_full;
>   
>       int flags = probe_access_internal(env_cpu(env), addr, size, access_type,
> -                                      mmu_idx, true, phost, &full, 0, false);
> +                                      mmu_idx, true, phost, pfull, 0, false);
>   
>       /* Handle clean RAM pages.  */
>       if (unlikely(flags & TLB_NOTDIRTY)) {
>           int dirtysize = size == 0 ? 1 : size;
> -        notdirty_write(env_cpu(env), addr, dirtysize, full, 0);
> +        notdirty_write(env_cpu(env), addr, dirtysize, pfull, 0);
>           flags &= ~TLB_NOTDIRTY;
>       }
>   
> -    if (pfull) {
> -        *pfull = *full;
> -    }
> -
>       return flags;
>   }
>   
> @@ -1470,7 +1466,7 @@ int probe_access_flags(CPUArchState *env, vaddr addr, int size,
>                          MMUAccessType access_type, int mmu_idx,
>                          bool nonfault, void **phost, uintptr_t retaddr)
>   {
> -    CPUTLBEntryFull *full;
> +    CPUTLBEntryFull full;
>       int flags;
>   
>       g_assert(-(addr | TARGET_PAGE_MASK) >= size);
> @@ -1482,7 +1478,7 @@ int probe_access_flags(CPUArchState *env, vaddr addr, int size,
>       /* Handle clean RAM pages. */
>       if (unlikely(flags & TLB_NOTDIRTY)) {
>           int dirtysize = size == 0 ? 1 : size;
> -        notdirty_write(env_cpu(env), addr, dirtysize, full, retaddr);
> +        notdirty_write(env_cpu(env), addr, dirtysize, &full, retaddr);
>           flags &= ~TLB_NOTDIRTY;
>       }
>   
> @@ -1492,7 +1488,7 @@ int probe_access_flags(CPUArchState *env, vaddr addr, int size,
>   void *probe_access(CPUArchState *env, vaddr addr, int size,
>                      MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
>   {
> -    CPUTLBEntryFull *full;
> +    CPUTLBEntryFull full;
>       void *host;
>       int flags;
>   
> @@ -1513,12 +1509,12 @@ void *probe_access(CPUArchState *env, vaddr addr, int size,
>               int wp_access = (access_type == MMU_DATA_STORE
>                                ? BP_MEM_WRITE : BP_MEM_READ);
>               cpu_check_watchpoint(env_cpu(env), addr, size,
> -                                 full->attrs, wp_access, retaddr);
> +                                 full.attrs, wp_access, retaddr);
>           }
>   
>           /* Handle clean RAM pages.  */
>           if (flags & TLB_NOTDIRTY) {
> -            notdirty_write(env_cpu(env), addr, size, full, retaddr);
> +            notdirty_write(env_cpu(env), addr, size, &full, retaddr);
>           }
>       }
>   
> @@ -1528,7 +1524,7 @@ void *probe_access(CPUArchState *env, vaddr addr, int size,
>   void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
>                           MMUAccessType access_type, int mmu_idx)
>   {
> -    CPUTLBEntryFull *full;
> +    CPUTLBEntryFull full;
>       void *host;
>       int flags;
>   
> @@ -1552,7 +1548,7 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
>   tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr,
>                                           void **hostp)
>   {
> -    CPUTLBEntryFull *full;
> +    CPUTLBEntryFull full;
>       void *p;
>   
>       (void)probe_access_internal(env_cpu(env), addr, 1, MMU_INST_FETCH,
> @@ -1562,7 +1558,7 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr,
>           return -1;
>       }
>   
> -    if (full->lg_page_size < TARGET_PAGE_BITS) {
> +    if (full.lg_page_size < TARGET_PAGE_BITS) {
>           return -1;
>       }
>   

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>