Patchwork [RFC,v2,1/4] tcg: add declarations and templates of extended MMU helpers

login
register
mail settings
Submitter YeongKyoon Lee
Date July 5, 2012, 1:23 p.m.
Message ID <1341494619-4714-2-git-send-email-yeongkyoon.lee@samsung.com>
Download mbox | patch
Permalink /patch/169165/
State New
Headers show

Comments

YeongKyoon Lee - July 5, 2012, 1:23 p.m.
Add declarations and templates of extended MMU helpers which can take return address argument to what helper functions return. These extended helper functions are called only by generated code.

Signed-off-by: Yeongkyoon Lee <yeongkyoon.lee@samsung.com>
---
 softmmu_defs.h     |   13 +++++++++++++
 softmmu_template.h |   51 +++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 54 insertions(+), 10 deletions(-)
Peter Maydell - July 5, 2012, 1:40 p.m.
On 5 July 2012 14:23, Yeongkyoon Lee <yeongkyoon.lee@samsung.com> wrote:
> Add declarations and templates of extended MMU helpers which can take return address argument to what helper functions return. These extended helper functions are called only by generated code.

It's not entirely clear from this description what the
return address argument actually is.

Also, please line wrap your commit messages.

>
> Signed-off-by: Yeongkyoon Lee <yeongkyoon.lee@samsung.com>
> ---
>  softmmu_defs.h     |   13 +++++++++++++
>  softmmu_template.h |   51 +++++++++++++++++++++++++++++++++++++++++----------
>  2 files changed, 54 insertions(+), 10 deletions(-)
>
> diff --git a/softmmu_defs.h b/softmmu_defs.h
> index 8d59f9d..728c51b 100644
> --- a/softmmu_defs.h
> +++ b/softmmu_defs.h
> @@ -19,6 +19,19 @@ void __stl_mmu(target_ulong addr, uint32_t val, int mmu_idx);
>  uint64_t __ldq_mmu(target_ulong addr, int mmu_idx);
>  void __stq_mmu(target_ulong addr, uint64_t val, int mmu_idx);
>
> +#if defined(CONFIG_QEMU_LDST_OPTIMIZATION)
> +/* Extended versions of MMU helpers for qemu_ld/st optimization.
> +   They get return address arguments because the caller PCs are not where helpers return to. */

This is >80 characters ; please use checkpatch.pl.

> +uint8_t __ext_ldb_mmu(target_ulong addr, int mmu_idx, uintptr_t ra);

'__' is a prefix reserved for the system. I know we have existing usage
of it, but I think it would be better to avoid adding new uses.

> +void __ext_stb_mmu(target_ulong addr, uint8_t val, int mmu_idx, uintptr_t ra);
> +uint16_t __ext_ldw_mmu(target_ulong addr, int mmu_idx, uintptr_t ra);
> +void __ext_stw_mmu(target_ulong addr, uint16_t val, int mmu_idx, uintptr_t ra);
> +uint32_t __ext_ldl_mmu(target_ulong addr, int mmu_idx, uintptr_t ra);
> +void __ext_stl_mmu(target_ulong addr, uint32_t val, int mmu_idx, uintptr_t ra);
> +uint64_t __ext_ldq_mmu(target_ulong addr, int mmu_idx, uintptr_t ra);
> +void __ext_stq_mmu(target_ulong addr, uint64_t val, int mmu_idx, uintptr_t ra);
> +#endif  /* CONFIG_QEMU_LDST_OPTIMIZATION */
> +
>  uint8_t __ldb_cmmu(target_ulong addr, int mmu_idx);
>  void __stb_cmmu(target_ulong addr, uint8_t val, int mmu_idx);
>  uint16_t __ldw_cmmu(target_ulong addr, int mmu_idx);
> diff --git a/softmmu_template.h b/softmmu_template.h
> index b8bd700..d549800 100644
> --- a/softmmu_template.h
> +++ b/softmmu_template.h
> @@ -66,6 +66,25 @@
>  #define HELPER_PREFIX helper_
>  #endif
>
> +#ifdef USE_EXTENDED_HELPER
> +/* Exteneded helper funtions have one more argument of address
> +   to which pc is returned after setting TLB entry */

"Extended". "functions".

> +#if !defined(CONFIG_QEMU_LDST_OPTIMIZATION) || defined(CONFIG_TCG_PASS_AREG0)
> +#error You need CONFIG_QEMU_LDST_OPTIMIZATION or to disable CONFIG_TCG_PASS_AREG0!
> +#endif
> +#undef HELPER_PREFIX
> +#define HELPER_PREFIX __ext_
> +#define RET_PARAM , uintptr_t raddr
> +#define RET_VAR raddr
> +#define GET_RET_ADDR() RET_VAR
> +#else
> +#define RET_PARAM
> +#define RET_VAR
> +#define GET_RET_ADDR() GETPC()
> +#endif  /* USE_EXTENDED_HELPER */
> +
> +
> +#ifndef USE_EXTENDED_HELPER
>  static DATA_TYPE glue(glue(slow_ld, SUFFIX), MMUSUFFIX)(ENV_PARAM
>                                                          target_ulong addr,
>                                                          int mmu_idx,
> @@ -101,12 +120,14 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(ENV_PARAM
>  #endif /* SHIFT > 2 */
>      return res;
>  }
> +#endif  /* !USE_EXTENDED_HELPER */
>
>  /* handle all cases except unaligned access which span two pages */
>  DATA_TYPE
>  glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM
>                                                         target_ulong addr,
> -                                                       int mmu_idx)
> +                                                       int mmu_idx
> +                                                       RET_PARAM)
>  {
>      DATA_TYPE res;
>      int index;
> @@ -124,13 +145,13 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM
>              /* IO access */
>              if ((addr & (DATA_SIZE - 1)) != 0)
>                  goto do_unaligned_access;
> -            retaddr = GETPC();
> +            retaddr = GET_RET_ADDR();
>              ioaddr = env->iotlb[mmu_idx][index];
>              res = glue(io_read, SUFFIX)(ENV_VAR ioaddr, addr, retaddr);
>          } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
>              /* slow unaligned access (it spans two pages or IO) */
>          do_unaligned_access:
> -            retaddr = GETPC();
> +            retaddr = GET_RET_ADDR();
>  #ifdef ALIGNED_ONLY
>              do_unaligned_access(ENV_VAR addr, READ_ACCESS_TYPE, mmu_idx, retaddr);
>  #endif
> @@ -141,7 +162,7 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM
>              uintptr_t addend;
>  #ifdef ALIGNED_ONLY
>              if ((addr & (DATA_SIZE - 1)) != 0) {
> -                retaddr = GETPC();
> +                retaddr = GET_RET_ADDR();
>                  do_unaligned_access(ENV_VAR addr, READ_ACCESS_TYPE, mmu_idx, retaddr);
>              }
>  #endif
> @@ -151,7 +172,7 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM
>          }
>      } else {
>          /* the page is not in the TLB : fill it */
> -        retaddr = GETPC();
> +        retaddr = GET_RET_ADDR();
>  #ifdef ALIGNED_ONLY
>          if ((addr & (DATA_SIZE - 1)) != 0)
>              do_unaligned_access(ENV_VAR addr, READ_ACCESS_TYPE, mmu_idx, retaddr);
> @@ -162,6 +183,7 @@ glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM
>      return res;
>  }
>
> +#ifndef USE_EXTENDED_HELPER
>  /* handle all unaligned cases */
>  static DATA_TYPE
>  glue(glue(slow_ld, SUFFIX), MMUSUFFIX)(ENV_PARAM
> @@ -213,9 +235,11 @@ glue(glue(slow_ld, SUFFIX), MMUSUFFIX)(ENV_PARAM
>      }
>      return res;
>  }
> +#endif  /* !USE_EXTENDED_HELPER */
>
>  #ifndef SOFTMMU_CODE_ACCESS
>
> +#ifndef USE_EXTENDED_HELPER
>  static void glue(glue(slow_st, SUFFIX), MMUSUFFIX)(ENV_PARAM
>                                                     target_ulong addr,
>                                                     DATA_TYPE val,
> @@ -252,11 +276,13 @@ static inline void glue(io_write, SUFFIX)(ENV_PARAM
>  #endif
>  #endif /* SHIFT > 2 */
>  }
> +#endif  /* !USE_EXTENDED_HELPER */
>
>  void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM
>                                                              target_ulong addr,
>                                                              DATA_TYPE val,
> -                                                            int mmu_idx)
> +                                                            int mmu_idx
> +                                                            RET_PARAM)
>  {
>      target_phys_addr_t ioaddr;
>      target_ulong tlb_addr;
> @@ -271,12 +297,12 @@ void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM
>              /* IO access */
>              if ((addr & (DATA_SIZE - 1)) != 0)
>                  goto do_unaligned_access;
> -            retaddr = GETPC();
> +            retaddr = GET_RET_ADDR();
>              ioaddr = env->iotlb[mmu_idx][index];
>              glue(io_write, SUFFIX)(ENV_VAR ioaddr, val, addr, retaddr);
>          } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
>          do_unaligned_access:
> -            retaddr = GETPC();
> +            retaddr = GET_RET_ADDR();
>  #ifdef ALIGNED_ONLY
>              do_unaligned_access(ENV_VAR addr, 1, mmu_idx, retaddr);
>  #endif
> @@ -287,7 +313,7 @@ void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM
>              uintptr_t addend;
>  #ifdef ALIGNED_ONLY
>              if ((addr & (DATA_SIZE - 1)) != 0) {
> -                retaddr = GETPC();
> +                retaddr = GET_RET_ADDR();
>                  do_unaligned_access(ENV_VAR addr, 1, mmu_idx, retaddr);
>              }
>  #endif
> @@ -297,7 +323,7 @@ void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM
>          }
>      } else {
>          /* the page is not in the TLB : fill it */
> -        retaddr = GETPC();
> +        retaddr = GET_RET_ADDR();
>  #ifdef ALIGNED_ONLY
>          if ((addr & (DATA_SIZE - 1)) != 0)
>              do_unaligned_access(ENV_VAR addr, 1, mmu_idx, retaddr);
> @@ -307,6 +333,7 @@ void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM
>      }
>  }
>
> +#ifndef USE_EXTENDED_HELPER
>  /* handles all unaligned cases */
>  static void glue(glue(slow_st, SUFFIX), MMUSUFFIX)(ENV_PARAM
>                                                     target_ulong addr,
> @@ -356,6 +383,7 @@ static void glue(glue(slow_st, SUFFIX), MMUSUFFIX)(ENV_PARAM
>          goto redo;
>      }
>  }
> +#endif  /* !USE_EXTENDED_HELPER */
>
>  #endif /* !defined(SOFTMMU_CODE_ACCESS) */
>
> @@ -370,3 +398,6 @@ static void glue(glue(slow_st, SUFFIX), MMUSUFFIX)(ENV_PARAM
>  #undef ENV_VAR
>  #undef CPU_PREFIX
>  #undef HELPER_PREFIX
> +#undef RET_PARAM
> +#undef RET_VAR
> +#undef GET_RET_ADDR
> --
> 1.7.4.1
>

-- PMM
YeongKyoon Lee - July 6, 2012, 10:30 a.m.
>> Add declarations and templates of extended MMU helpers which can take return address argument to what helper functions return. These extended helper functions are called only by generated code.
> It's not entirely clear from this description what the
> return address argument actually is.
My commit message might give confusion. The return address is originally 
expressed as "retaddr" in softmmu_template.h. It means the runtime host 
pc which access guest memory. In previous standard MMU helpers, the 
address is the caller's pc of MMU helper calculated from GETPC(), 
however, in new optimized MMU helpers, the address is different from the 
caller's pc because the call site is located end of TB.
> Also, please line wrap your commit messages.
I didn't know the line wrap rule of commit message. Is the rule included 
in checkpatch.pl? Let me check it.
>> +#if defined(CONFIG_QEMU_LDST_OPTIMIZATION)
>> +/* Extended versions of MMU helpers for qemu_ld/st optimization.
>> +   They get return address arguments because the caller PCs are not where helpers return to. */
> This is >80 characters ; please use checkpatch.pl.
Ok.
>> +uint8_t __ext_ldb_mmu(target_ulong addr, int mmu_idx, uintptr_t ra);
> '__' is a prefix reserved for the system. I know we have existing usage
> of it, but I think it would be better to avoid adding new uses.
Ok, I'll fix it.
>> +#ifdef USE_EXTENDED_HELPER
>> +/* Exteneded helper funtions have one more argument of address
>> +   to which pc is returned after setting TLB entry */
> "Extended". "functions".
Ok.
陳韋任 - July 6, 2012, 10:35 a.m.
> > Also, please line wrap your commit messages.
> I didn't know the line wrap rule of commit message. Is the rule included 
> in checkpatch.pl? Let me check it.

  I guess it's 80 char length rule?

Regards,
chenwj

Patch

diff --git a/softmmu_defs.h b/softmmu_defs.h
index 8d59f9d..728c51b 100644
--- a/softmmu_defs.h
+++ b/softmmu_defs.h
@@ -19,6 +19,19 @@  void __stl_mmu(target_ulong addr, uint32_t val, int mmu_idx);
 uint64_t __ldq_mmu(target_ulong addr, int mmu_idx);
 void __stq_mmu(target_ulong addr, uint64_t val, int mmu_idx);
 
+#if defined(CONFIG_QEMU_LDST_OPTIMIZATION)
+/* Extended versions of MMU helpers for qemu_ld/st optimization.
+   They get return address arguments because the caller PCs are not where helpers return to. */
+uint8_t __ext_ldb_mmu(target_ulong addr, int mmu_idx, uintptr_t ra);
+void __ext_stb_mmu(target_ulong addr, uint8_t val, int mmu_idx, uintptr_t ra);
+uint16_t __ext_ldw_mmu(target_ulong addr, int mmu_idx, uintptr_t ra);
+void __ext_stw_mmu(target_ulong addr, uint16_t val, int mmu_idx, uintptr_t ra);
+uint32_t __ext_ldl_mmu(target_ulong addr, int mmu_idx, uintptr_t ra);
+void __ext_stl_mmu(target_ulong addr, uint32_t val, int mmu_idx, uintptr_t ra);
+uint64_t __ext_ldq_mmu(target_ulong addr, int mmu_idx, uintptr_t ra);
+void __ext_stq_mmu(target_ulong addr, uint64_t val, int mmu_idx, uintptr_t ra);
+#endif  /* CONFIG_QEMU_LDST_OPTIMIZATION */
+
 uint8_t __ldb_cmmu(target_ulong addr, int mmu_idx);
 void __stb_cmmu(target_ulong addr, uint8_t val, int mmu_idx);
 uint16_t __ldw_cmmu(target_ulong addr, int mmu_idx);
diff --git a/softmmu_template.h b/softmmu_template.h
index b8bd700..d549800 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -66,6 +66,25 @@ 
 #define HELPER_PREFIX helper_
 #endif
 
+#ifdef USE_EXTENDED_HELPER
+/* Exteneded helper funtions have one more argument of address
+   to which pc is returned after setting TLB entry */
+#if !defined(CONFIG_QEMU_LDST_OPTIMIZATION) || defined(CONFIG_TCG_PASS_AREG0)
+#error You need CONFIG_QEMU_LDST_OPTIMIZATION or to disable CONFIG_TCG_PASS_AREG0!
+#endif
+#undef HELPER_PREFIX
+#define HELPER_PREFIX __ext_
+#define RET_PARAM , uintptr_t raddr
+#define RET_VAR raddr
+#define GET_RET_ADDR() RET_VAR
+#else
+#define RET_PARAM
+#define RET_VAR
+#define GET_RET_ADDR() GETPC()
+#endif  /* USE_EXTENDED_HELPER */
+
+
+#ifndef USE_EXTENDED_HELPER
 static DATA_TYPE glue(glue(slow_ld, SUFFIX), MMUSUFFIX)(ENV_PARAM
                                                         target_ulong addr,
                                                         int mmu_idx,
@@ -101,12 +120,14 @@  static inline DATA_TYPE glue(io_read, SUFFIX)(ENV_PARAM
 #endif /* SHIFT > 2 */
     return res;
 }
+#endif  /* !USE_EXTENDED_HELPER */
 
 /* handle all cases except unaligned access which span two pages */
 DATA_TYPE
 glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM
                                                        target_ulong addr,
-                                                       int mmu_idx)
+                                                       int mmu_idx
+                                                       RET_PARAM)
 {
     DATA_TYPE res;
     int index;
@@ -124,13 +145,13 @@  glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM
             /* IO access */
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
-            retaddr = GETPC();
+            retaddr = GET_RET_ADDR();
             ioaddr = env->iotlb[mmu_idx][index];
             res = glue(io_read, SUFFIX)(ENV_VAR ioaddr, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
             /* slow unaligned access (it spans two pages or IO) */
         do_unaligned_access:
-            retaddr = GETPC();
+            retaddr = GET_RET_ADDR();
 #ifdef ALIGNED_ONLY
             do_unaligned_access(ENV_VAR addr, READ_ACCESS_TYPE, mmu_idx, retaddr);
 #endif
@@ -141,7 +162,7 @@  glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM
             uintptr_t addend;
 #ifdef ALIGNED_ONLY
             if ((addr & (DATA_SIZE - 1)) != 0) {
-                retaddr = GETPC();
+                retaddr = GET_RET_ADDR();
                 do_unaligned_access(ENV_VAR addr, READ_ACCESS_TYPE, mmu_idx, retaddr);
             }
 #endif
@@ -151,7 +172,7 @@  glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM
         }
     } else {
         /* the page is not in the TLB : fill it */
-        retaddr = GETPC();
+        retaddr = GET_RET_ADDR();
 #ifdef ALIGNED_ONLY
         if ((addr & (DATA_SIZE - 1)) != 0)
             do_unaligned_access(ENV_VAR addr, READ_ACCESS_TYPE, mmu_idx, retaddr);
@@ -162,6 +183,7 @@  glue(glue(glue(HELPER_PREFIX, ld), SUFFIX), MMUSUFFIX)(ENV_PARAM
     return res;
 }
 
+#ifndef USE_EXTENDED_HELPER
 /* handle all unaligned cases */
 static DATA_TYPE
 glue(glue(slow_ld, SUFFIX), MMUSUFFIX)(ENV_PARAM
@@ -213,9 +235,11 @@  glue(glue(slow_ld, SUFFIX), MMUSUFFIX)(ENV_PARAM
     }
     return res;
 }
+#endif  /* !USE_EXTENDED_HELPER */
 
 #ifndef SOFTMMU_CODE_ACCESS
 
+#ifndef USE_EXTENDED_HELPER
 static void glue(glue(slow_st, SUFFIX), MMUSUFFIX)(ENV_PARAM
                                                    target_ulong addr,
                                                    DATA_TYPE val,
@@ -252,11 +276,13 @@  static inline void glue(io_write, SUFFIX)(ENV_PARAM
 #endif
 #endif /* SHIFT > 2 */
 }
+#endif  /* !USE_EXTENDED_HELPER */
 
 void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM
                                                             target_ulong addr,
                                                             DATA_TYPE val,
-                                                            int mmu_idx)
+                                                            int mmu_idx
+                                                            RET_PARAM)
 {
     target_phys_addr_t ioaddr;
     target_ulong tlb_addr;
@@ -271,12 +297,12 @@  void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM
             /* IO access */
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
-            retaddr = GETPC();
+            retaddr = GET_RET_ADDR();
             ioaddr = env->iotlb[mmu_idx][index];
             glue(io_write, SUFFIX)(ENV_VAR ioaddr, val, addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
         do_unaligned_access:
-            retaddr = GETPC();
+            retaddr = GET_RET_ADDR();
 #ifdef ALIGNED_ONLY
             do_unaligned_access(ENV_VAR addr, 1, mmu_idx, retaddr);
 #endif
@@ -287,7 +313,7 @@  void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM
             uintptr_t addend;
 #ifdef ALIGNED_ONLY
             if ((addr & (DATA_SIZE - 1)) != 0) {
-                retaddr = GETPC();
+                retaddr = GET_RET_ADDR();
                 do_unaligned_access(ENV_VAR addr, 1, mmu_idx, retaddr);
             }
 #endif
@@ -297,7 +323,7 @@  void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM
         }
     } else {
         /* the page is not in the TLB : fill it */
-        retaddr = GETPC();
+        retaddr = GET_RET_ADDR();
 #ifdef ALIGNED_ONLY
         if ((addr & (DATA_SIZE - 1)) != 0)
             do_unaligned_access(ENV_VAR addr, 1, mmu_idx, retaddr);
@@ -307,6 +333,7 @@  void glue(glue(glue(HELPER_PREFIX, st), SUFFIX), MMUSUFFIX)(ENV_PARAM
     }
 }
 
+#ifndef USE_EXTENDED_HELPER
 /* handles all unaligned cases */
 static void glue(glue(slow_st, SUFFIX), MMUSUFFIX)(ENV_PARAM
                                                    target_ulong addr,
@@ -356,6 +383,7 @@  static void glue(glue(slow_st, SUFFIX), MMUSUFFIX)(ENV_PARAM
         goto redo;
     }
 }
+#endif  /* !USE_EXTENDED_HELPER */
 
 #endif /* !defined(SOFTMMU_CODE_ACCESS) */
 
@@ -370,3 +398,6 @@  static void glue(glue(slow_st, SUFFIX), MMUSUFFIX)(ENV_PARAM
 #undef ENV_VAR
 #undef CPU_PREFIX
 #undef HELPER_PREFIX
+#undef RET_PARAM
+#undef RET_VAR
+#undef GET_RET_ADDR