diff mbox series

[v2,02/28] cputlb: Use trace_mem_get_info instead of trace_mem_build_info

Message ID 20191216221158.29572-3-richard.henderson@linaro.org
State Superseded
Headers show
Series cputlb: Remove support for MMU_MODE*_SUFFIX | expand

Commit Message

Richard Henderson Dec. 16, 2019, 10:11 p.m. UTC 
In the cpu_ldst templates, we already require a MemOp, and it
is cleaner and clearer to pass that instead of 3 separate
arguments describing the memory operation.

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

---
 include/exec/cpu_ldst_template.h          | 22 +++++++++++-----------
 include/exec/cpu_ldst_useronly_template.h | 12 ++++++------
 2 files changed, 17 insertions(+), 17 deletions(-)

-- 
2.20.1

Comments

Philippe Mathieu-Daudé Dec. 17, 2019, 3:59 a.m. UTC | #1 
On 12/16/19 11:11 PM, Richard Henderson wrote:
> In the cpu_ldst templates, we already require a MemOp, and it

> is cleaner and clearer to pass that instead of 3 separate

> arguments describing the memory operation.

> 

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

> ---

>   include/exec/cpu_ldst_template.h          | 22 +++++++++++-----------

>   include/exec/cpu_ldst_useronly_template.h | 12 ++++++------

>   2 files changed, 17 insertions(+), 17 deletions(-)

> 

> diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h

> index 54b5e858ce..0ad5de3ef9 100644

> --- a/include/exec/cpu_ldst_template.h

> +++ b/include/exec/cpu_ldst_template.h

> @@ -86,9 +86,9 @@ glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>       RES_TYPE res;

>       target_ulong addr;

>       int mmu_idx = CPU_MMU_INDEX;

> -    TCGMemOpIdx oi;

> +    MemOp op = MO_TE | SHIFT;

>   #if !defined(SOFTMMU_CODE_ACCESS)

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, false, mmu_idx);

> +    uint16_t meminfo = trace_mem_get_info(op, mmu_idx, false);

>       trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>   #endif

>   

> @@ -96,9 +96,9 @@ glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>       entry = tlb_entry(env, mmu_idx, addr);

>       if (unlikely(entry->ADDR_READ !=

>                    (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {

> -        oi = make_memop_idx(SHIFT, mmu_idx);

> +        TCGMemOpIdx oi = make_memop_idx(op, mmu_idx);

>           res = glue(glue(helper_ret_ld, URETSUFFIX), MMUSUFFIX)(env, addr,

> -                                                            oi, retaddr);

> +                                                               oi, retaddr);

>       } else {

>           uintptr_t hostaddr = addr + entry->addend;

>           res = glue(glue(ld, USUFFIX), _p)((uint8_t *)hostaddr);

> @@ -125,9 +125,9 @@ glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>       int res;

>       target_ulong addr;

>       int mmu_idx = CPU_MMU_INDEX;

> -    TCGMemOpIdx oi;

> -#if !defined(SOFTMMU_CODE_ACCESS)

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, true, MO_TE, false, mmu_idx);

> +    MemOp op = MO_TE | MO_SIGN | SHIFT;


Good, the sign-extend is easier to review this way (than 'true').

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


> +#ifndef SOFTMMU_CODE_ACCESS

> +    uint16_t meminfo = trace_mem_get_info(op, mmu_idx, false);

>       trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>   #endif

>   

> @@ -135,7 +135,7 @@ glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>       entry = tlb_entry(env, mmu_idx, addr);

>       if (unlikely(entry->ADDR_READ !=

>                    (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {

> -        oi = make_memop_idx(SHIFT, mmu_idx);

> +        TCGMemOpIdx oi = make_memop_idx(op & ~MO_SIGN, mmu_idx);

>           res = (DATA_STYPE)glue(glue(helper_ret_ld, SRETSUFFIX),

>                                  MMUSUFFIX)(env, addr, oi, retaddr);

>       } else {

> @@ -167,9 +167,9 @@ glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>       CPUTLBEntry *entry;

>       target_ulong addr;

>       int mmu_idx = CPU_MMU_INDEX;

> -    TCGMemOpIdx oi;

> +    MemOp op = MO_TE | SHIFT;

>   #if !defined(SOFTMMU_CODE_ACCESS)

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, true, mmu_idx);

> +    uint16_t meminfo = trace_mem_get_info(op, mmu_idx, true);

>       trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>   #endif

>   

> @@ -177,7 +177,7 @@ glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>       entry = tlb_entry(env, mmu_idx, addr);

>       if (unlikely(tlb_addr_write(entry) !=

>                    (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {

> -        oi = make_memop_idx(SHIFT, mmu_idx);

> +        TCGMemOpIdx oi = make_memop_idx(op, mmu_idx);

>           glue(glue(helper_ret_st, SUFFIX), MMUSUFFIX)(env, addr, v, oi,

>                                                        retaddr);

>       } else {

> diff --git a/include/exec/cpu_ldst_useronly_template.h b/include/exec/cpu_ldst_useronly_template.h

> index dbdc7a845d..e5a3d1983a 100644

> --- a/include/exec/cpu_ldst_useronly_template.h

> +++ b/include/exec/cpu_ldst_useronly_template.h

> @@ -70,8 +70,8 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr)

>       ret = glue(glue(ld, USUFFIX), _p)(g2h(ptr));

>       clear_helper_retaddr();

>   #else

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, false,

> -                                            MMU_USER_IDX);

> +    MemOp op = MO_TE | SHIFT;

> +    uint16_t meminfo = trace_mem_get_info(op, MMU_USER_IDX, false);

>       trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>       ret = glue(glue(ld, USUFFIX), _p)(g2h(ptr));

>   #endif

> @@ -102,8 +102,8 @@ glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr)

>       ret = glue(glue(lds, SUFFIX), _p)(g2h(ptr));

>       clear_helper_retaddr();

>   #else

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, true, MO_TE, false,

> -                                            MMU_USER_IDX);

> +    MemOp op = MO_TE | MO_SIGN | SHIFT;

> +    uint16_t meminfo = trace_mem_get_info(op, MMU_USER_IDX, false);

>       trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>       ret = glue(glue(lds, SUFFIX), _p)(g2h(ptr));

>       qemu_plugin_vcpu_mem_cb(env_cpu(env), ptr, meminfo);

> @@ -131,8 +131,8 @@ static inline void

>   glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr,

>                                         RES_TYPE v)

>   {

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, true,

> -                                            MMU_USER_IDX);

> +    MemOp op = MO_TE | SHIFT;

> +    uint16_t meminfo = trace_mem_get_info(op, MMU_USER_IDX, true);

>       trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>       glue(glue(st, SUFFIX), _p)(g2h(ptr), v);

>       qemu_plugin_vcpu_mem_cb(env_cpu(env), ptr, meminfo);

>
Alex Bennée Dec. 20, 2019, 3:04 p.m. UTC | #2 
Richard Henderson <richard.henderson@linaro.org> writes:

> In the cpu_ldst templates, we already require a MemOp, and it

> is cleaner and clearer to pass that instead of 3 separate

> arguments describing the memory operation.

>

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


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


> ---

>  include/exec/cpu_ldst_template.h          | 22 +++++++++++-----------

>  include/exec/cpu_ldst_useronly_template.h | 12 ++++++------

>  2 files changed, 17 insertions(+), 17 deletions(-)

>

> diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h

> index 54b5e858ce..0ad5de3ef9 100644

> --- a/include/exec/cpu_ldst_template.h

> +++ b/include/exec/cpu_ldst_template.h

> @@ -86,9 +86,9 @@ glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>      RES_TYPE res;

>      target_ulong addr;

>      int mmu_idx = CPU_MMU_INDEX;

> -    TCGMemOpIdx oi;

> +    MemOp op = MO_TE | SHIFT;

>  #if !defined(SOFTMMU_CODE_ACCESS)

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, false, mmu_idx);

> +    uint16_t meminfo = trace_mem_get_info(op, mmu_idx, false);

>      trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>  #endif

>  

> @@ -96,9 +96,9 @@ glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>      entry = tlb_entry(env, mmu_idx, addr);

>      if (unlikely(entry->ADDR_READ !=

>                   (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {

> -        oi = make_memop_idx(SHIFT, mmu_idx);

> +        TCGMemOpIdx oi = make_memop_idx(op, mmu_idx);

>          res = glue(glue(helper_ret_ld, URETSUFFIX), MMUSUFFIX)(env, addr,

> -                                                            oi, retaddr);

> +                                                               oi, retaddr);

>      } else {

>          uintptr_t hostaddr = addr + entry->addend;

>          res = glue(glue(ld, USUFFIX), _p)((uint8_t *)hostaddr);

> @@ -125,9 +125,9 @@ glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>      int res;

>      target_ulong addr;

>      int mmu_idx = CPU_MMU_INDEX;

> -    TCGMemOpIdx oi;

> -#if !defined(SOFTMMU_CODE_ACCESS)

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, true, MO_TE, false, mmu_idx);

> +    MemOp op = MO_TE | MO_SIGN | SHIFT;

> +#ifndef SOFTMMU_CODE_ACCESS

> +    uint16_t meminfo = trace_mem_get_info(op, mmu_idx, false);

>      trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>  #endif

>  

> @@ -135,7 +135,7 @@ glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>      entry = tlb_entry(env, mmu_idx, addr);

>      if (unlikely(entry->ADDR_READ !=

>                   (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {

> -        oi = make_memop_idx(SHIFT, mmu_idx);

> +        TCGMemOpIdx oi = make_memop_idx(op & ~MO_SIGN, mmu_idx);

>          res = (DATA_STYPE)glue(glue(helper_ret_ld, SRETSUFFIX),

>                                 MMUSUFFIX)(env, addr, oi, retaddr);

>      } else {

> @@ -167,9 +167,9 @@ glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>      CPUTLBEntry *entry;

>      target_ulong addr;

>      int mmu_idx = CPU_MMU_INDEX;

> -    TCGMemOpIdx oi;

> +    MemOp op = MO_TE | SHIFT;

>  #if !defined(SOFTMMU_CODE_ACCESS)

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, true, mmu_idx);

> +    uint16_t meminfo = trace_mem_get_info(op, mmu_idx, true);

>      trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>  #endif

>  

> @@ -177,7 +177,7 @@ glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,

>      entry = tlb_entry(env, mmu_idx, addr);

>      if (unlikely(tlb_addr_write(entry) !=

>                   (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {

> -        oi = make_memop_idx(SHIFT, mmu_idx);

> +        TCGMemOpIdx oi = make_memop_idx(op, mmu_idx);

>          glue(glue(helper_ret_st, SUFFIX), MMUSUFFIX)(env, addr, v, oi,

>                                                       retaddr);

>      } else {

> diff --git a/include/exec/cpu_ldst_useronly_template.h b/include/exec/cpu_ldst_useronly_template.h

> index dbdc7a845d..e5a3d1983a 100644

> --- a/include/exec/cpu_ldst_useronly_template.h

> +++ b/include/exec/cpu_ldst_useronly_template.h

> @@ -70,8 +70,8 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr)

>      ret = glue(glue(ld, USUFFIX), _p)(g2h(ptr));

>      clear_helper_retaddr();

>  #else

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, false,

> -                                            MMU_USER_IDX);

> +    MemOp op = MO_TE | SHIFT;

> +    uint16_t meminfo = trace_mem_get_info(op, MMU_USER_IDX, false);

>      trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>      ret = glue(glue(ld, USUFFIX), _p)(g2h(ptr));

>  #endif

> @@ -102,8 +102,8 @@ glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr)

>      ret = glue(glue(lds, SUFFIX), _p)(g2h(ptr));

>      clear_helper_retaddr();

>  #else

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, true, MO_TE, false,

> -                                            MMU_USER_IDX);

> +    MemOp op = MO_TE | MO_SIGN | SHIFT;

> +    uint16_t meminfo = trace_mem_get_info(op, MMU_USER_IDX, false);

>      trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>      ret = glue(glue(lds, SUFFIX), _p)(g2h(ptr));

>      qemu_plugin_vcpu_mem_cb(env_cpu(env), ptr, meminfo);

> @@ -131,8 +131,8 @@ static inline void

>  glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr,

>                                        RES_TYPE v)

>  {

> -    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, true,

> -                                            MMU_USER_IDX);

> +    MemOp op = MO_TE | SHIFT;

> +    uint16_t meminfo = trace_mem_get_info(op, MMU_USER_IDX, true);

>      trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);

>      glue(glue(st, SUFFIX), _p)(g2h(ptr), v);

>      qemu_plugin_vcpu_mem_cb(env_cpu(env), ptr, meminfo);



-- 
Alex Bennée
diff mbox series

Patch

diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h
index 54b5e858ce..0ad5de3ef9 100644
--- a/include/exec/cpu_ldst_template.h
+++ b/include/exec/cpu_ldst_template.h
@@ -86,9 +86,9 @@  glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
     RES_TYPE res;
     target_ulong addr;
     int mmu_idx = CPU_MMU_INDEX;
-    TCGMemOpIdx oi;
+    MemOp op = MO_TE | SHIFT;
 #if !defined(SOFTMMU_CODE_ACCESS)
-    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, false, mmu_idx);
+    uint16_t meminfo = trace_mem_get_info(op, mmu_idx, false);
     trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);
 #endif
 
@@ -96,9 +96,9 @@  glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
     entry = tlb_entry(env, mmu_idx, addr);
     if (unlikely(entry->ADDR_READ !=
                  (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {
-        oi = make_memop_idx(SHIFT, mmu_idx);
+        TCGMemOpIdx oi = make_memop_idx(op, mmu_idx);
         res = glue(glue(helper_ret_ld, URETSUFFIX), MMUSUFFIX)(env, addr,
-                                                            oi, retaddr);
+                                                               oi, retaddr);
     } else {
         uintptr_t hostaddr = addr + entry->addend;
         res = glue(glue(ld, USUFFIX), _p)((uint8_t *)hostaddr);
@@ -125,9 +125,9 @@  glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
     int res;
     target_ulong addr;
     int mmu_idx = CPU_MMU_INDEX;
-    TCGMemOpIdx oi;
-#if !defined(SOFTMMU_CODE_ACCESS)
-    uint16_t meminfo = trace_mem_build_info(SHIFT, true, MO_TE, false, mmu_idx);
+    MemOp op = MO_TE | MO_SIGN | SHIFT;
+#ifndef SOFTMMU_CODE_ACCESS
+    uint16_t meminfo = trace_mem_get_info(op, mmu_idx, false);
     trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);
 #endif
 
@@ -135,7 +135,7 @@  glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
     entry = tlb_entry(env, mmu_idx, addr);
     if (unlikely(entry->ADDR_READ !=
                  (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {
-        oi = make_memop_idx(SHIFT, mmu_idx);
+        TCGMemOpIdx oi = make_memop_idx(op & ~MO_SIGN, mmu_idx);
         res = (DATA_STYPE)glue(glue(helper_ret_ld, SRETSUFFIX),
                                MMUSUFFIX)(env, addr, oi, retaddr);
     } else {
@@ -167,9 +167,9 @@  glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
     CPUTLBEntry *entry;
     target_ulong addr;
     int mmu_idx = CPU_MMU_INDEX;
-    TCGMemOpIdx oi;
+    MemOp op = MO_TE | SHIFT;
 #if !defined(SOFTMMU_CODE_ACCESS)
-    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, true, mmu_idx);
+    uint16_t meminfo = trace_mem_get_info(op, mmu_idx, true);
     trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);
 #endif
 
@@ -177,7 +177,7 @@  glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env,
     entry = tlb_entry(env, mmu_idx, addr);
     if (unlikely(tlb_addr_write(entry) !=
                  (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {
-        oi = make_memop_idx(SHIFT, mmu_idx);
+        TCGMemOpIdx oi = make_memop_idx(op, mmu_idx);
         glue(glue(helper_ret_st, SUFFIX), MMUSUFFIX)(env, addr, v, oi,
                                                      retaddr);
     } else {
diff --git a/include/exec/cpu_ldst_useronly_template.h b/include/exec/cpu_ldst_useronly_template.h
index dbdc7a845d..e5a3d1983a 100644
--- a/include/exec/cpu_ldst_useronly_template.h
+++ b/include/exec/cpu_ldst_useronly_template.h
@@ -70,8 +70,8 @@  glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr)
     ret = glue(glue(ld, USUFFIX), _p)(g2h(ptr));
     clear_helper_retaddr();
 #else
-    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, false,
-                                            MMU_USER_IDX);
+    MemOp op = MO_TE | SHIFT;
+    uint16_t meminfo = trace_mem_get_info(op, MMU_USER_IDX, false);
     trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);
     ret = glue(glue(ld, USUFFIX), _p)(g2h(ptr));
 #endif
@@ -102,8 +102,8 @@  glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr)
     ret = glue(glue(lds, SUFFIX), _p)(g2h(ptr));
     clear_helper_retaddr();
 #else
-    uint16_t meminfo = trace_mem_build_info(SHIFT, true, MO_TE, false,
-                                            MMU_USER_IDX);
+    MemOp op = MO_TE | MO_SIGN | SHIFT;
+    uint16_t meminfo = trace_mem_get_info(op, MMU_USER_IDX, false);
     trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);
     ret = glue(glue(lds, SUFFIX), _p)(g2h(ptr));
     qemu_plugin_vcpu_mem_cb(env_cpu(env), ptr, meminfo);
@@ -131,8 +131,8 @@  static inline void
 glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState *env, abi_ptr ptr,
                                       RES_TYPE v)
 {
-    uint16_t meminfo = trace_mem_build_info(SHIFT, false, MO_TE, true,
-                                            MMU_USER_IDX);
+    MemOp op = MO_TE | SHIFT;
+    uint16_t meminfo = trace_mem_get_info(op, MMU_USER_IDX, true);
     trace_guest_mem_before_exec(env_cpu(env), ptr, meminfo);
     glue(glue(st, SUFFIX), _p)(g2h(ptr), v);
     qemu_plugin_vcpu_mem_cb(env_cpu(env), ptr, meminfo);