diff mbox

[v2,2/4] target-arm: Support coprocessor registers which do I/O

Message ID 1376065080-26661-3-git-send-email-peter.maydell@linaro.org
State Accepted
Commit 2452731c883cb0acd4e47b23039c46cd880cf2c6
Headers show

Commit Message

Peter Maydell Aug. 9, 2013, 4:17 p.m. UTC 
Add an ARM_CP_IO flag which an ARMCPRegInfo definition can use to
indicate that the register's implementation does I/O and thus
its accesses need to be surrounded by gen_io_start()/gen_io_end()
in order for icount to work. Most notably, cp registers which
implement clocks or timers need this.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>
---
 target-arm/cpu.h       |    6 +++++-
 target-arm/translate.c |   16 +++++++++++++---
 2 files changed, 18 insertions(+), 4 deletions(-)

Comments

Edgar E. Iglesias Aug. 14, 2013, 5:20 a.m. UTC | #1 
On Fri, Aug 09, 2013 at 05:17:58PM +0100, Peter Maydell wrote:
> Add an ARM_CP_IO flag which an ARMCPRegInfo definition can use to
> indicate that the register's implementation does I/O and thus
> its accesses need to be surrounded by gen_io_start()/gen_io_end()
> in order for icount to work. Most notably, cp registers which
> implement clocks or timers need this.

Good timing, I was just looking for this kind of mechanism... :)

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>
> ---
>  target-arm/cpu.h       |    6 +++++-
>  target-arm/translate.c |   16 +++++++++++++---
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index b2dc494..770a240 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -469,6 +469,9 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
>   * old must have the OVERRIDE bit set.
>   * NO_MIGRATE indicates that this register should be ignored for migration;
>   * (eg because any state is accessed via some other coprocessor register).
> + * IO indicates that this register does I/O and therefore its accesses
> + * need to be surrounded by gen_io_start()/gen_io_end(). In particular,
> + * registers which implement clocks or timers require this.
>   */
>  #define ARM_CP_SPECIAL 1
>  #define ARM_CP_CONST 2
> @@ -476,13 +479,14 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
>  #define ARM_CP_SUPPRESS_TB_END 8
>  #define ARM_CP_OVERRIDE 16
>  #define ARM_CP_NO_MIGRATE 32
> +#define ARM_CP_IO 64
>  #define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8))
>  #define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8))
>  #define ARM_LAST_SPECIAL ARM_CP_WFI
>  /* Used only as a terminator for ARMCPRegInfo lists */
>  #define ARM_CP_SENTINEL 0xffff
>  /* Mask of only the flag bits in a type field */
> -#define ARM_CP_FLAG_MASK 0x3f
> +#define ARM_CP_FLAG_MASK 0x7f
>  
>  /* Return true if cptype is a valid type field. This is used to try to
>   * catch errors where the sentinel has been accidentally left off the end
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 6db4c50..d1e8538 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -6280,6 +6280,10 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
>              break;
>          }
>  
> +        if (use_icount && (ri->type & ARM_CP_IO)) {
> +            gen_io_start();
> +        }
> +
>          if (isread) {
>              /* Read */
>              if (is64) {
> @@ -6369,14 +6373,20 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
>                      store_cpu_offset(tmp, ri->fieldoffset);
>                  }
>              }
> +        }
> +
> +        if (use_icount && (ri->type & ARM_CP_IO)) {
> +            /* I/O operations must end the TB here (whether read or write) */
> +            gen_io_end();
> +            gen_lookup_tb(s);
> +        } else if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) {
>              /* We default to ending the TB on a coprocessor register write,
>               * but allow this to be suppressed by the register definition
>               * (usually only necessary to work around guest bugs).
>               */
> -            if (!(ri->type & ARM_CP_SUPPRESS_TB_END)) {
> -                gen_lookup_tb(s);
> -            }
> +            gen_lookup_tb(s);
>          }
> +
>          return 0;
>      }
>  
> -- 
> 1.7.9.5
> 
>
diff mbox

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index b2dc494..770a240 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -469,6 +469,9 @@  static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
  * old must have the OVERRIDE bit set.
  * NO_MIGRATE indicates that this register should be ignored for migration;
  * (eg because any state is accessed via some other coprocessor register).
+ * IO indicates that this register does I/O and therefore its accesses
+ * need to be surrounded by gen_io_start()/gen_io_end(). In particular,
+ * registers which implement clocks or timers require this.
  */
 #define ARM_CP_SPECIAL 1
 #define ARM_CP_CONST 2
@@ -476,13 +479,14 @@  static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
 #define ARM_CP_SUPPRESS_TB_END 8
 #define ARM_CP_OVERRIDE 16
 #define ARM_CP_NO_MIGRATE 32
+#define ARM_CP_IO 64
 #define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8))
 #define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8))
 #define ARM_LAST_SPECIAL ARM_CP_WFI
 /* Used only as a terminator for ARMCPRegInfo lists */
 #define ARM_CP_SENTINEL 0xffff
 /* Mask of only the flag bits in a type field */
-#define ARM_CP_FLAG_MASK 0x3f
+#define ARM_CP_FLAG_MASK 0x7f
 
 /* Return true if cptype is a valid type field. This is used to try to
  * catch errors where the sentinel has been accidentally left off the end
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 6db4c50..d1e8538 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -6280,6 +6280,10 @@  static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
             break;
         }
 
+        if (use_icount && (ri->type & ARM_CP_IO)) {
+            gen_io_start();
+        }
+
         if (isread) {
             /* Read */
             if (is64) {
@@ -6369,14 +6373,20 @@  static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
                     store_cpu_offset(tmp, ri->fieldoffset);
                 }
             }
+        }
+
+        if (use_icount && (ri->type & ARM_CP_IO)) {
+            /* I/O operations must end the TB here (whether read or write) */
+            gen_io_end();
+            gen_lookup_tb(s);
+        } else if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) {
             /* We default to ending the TB on a coprocessor register write,
              * but allow this to be suppressed by the register definition
              * (usually only necessary to work around guest bugs).
              */
-            if (!(ri->type & ARM_CP_SUPPRESS_TB_END)) {
-                gen_lookup_tb(s);
-            }
+            gen_lookup_tb(s);
         }
+
         return 0;
     }