[Xen-devel,10/13] xen/arm: vreg: Introduce vreg_emulate_cp{32, 64}

Message ID 1481114033-11024-11-git-send-email-julien.grall@arm.com
State New
Headers show

Commit Message

Julien Grall Dec. 7, 2016, 12:33 p.m.
Factorize the code to emulate 32-bit and 64-bit access to a co-processor
in specific helpers.

The new helpers will be used in different components to simplify the
emulation.

Finally, the prototypes for the callbacks to emulate 32-bit and 64-bit
co-processor access are the same as the sysreg one. Rather than
introducing new ones, repurpose the existent prototypes.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/vtimer.c      | 37 +++------------------------
 xen/include/asm-arm/vreg.h | 64 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 62 insertions(+), 39 deletions(-)

Comments

Stefano Stabellini Dec. 7, 2016, 10:41 p.m. | #1
On Wed, 7 Dec 2016, Julien Grall wrote:
> Factorize the code to emulate 32-bit and 64-bit access to a co-processor
> in specific helpers.
> 
> The new helpers will be used in different components to simplify the
> emulation.
> 
> Finally, the prototypes for the callbacks to emulate 32-bit and 64-bit
> co-processor access are the same as the sysreg one. Rather than
> introducing new ones, repurpose the existent prototypes.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/vtimer.c      | 37 +++------------------------
>  xen/include/asm-arm/vreg.h | 64 ++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 62 insertions(+), 39 deletions(-)
> 
> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> index 091b5e7..4ec3b95 100644
> --- a/xen/arch/arm/vtimer.c
> +++ b/xen/arch/arm/vtimer.c
> @@ -252,49 +252,28 @@ static bool vtimer_cntp_cval(struct cpu_user_regs *regs, uint64_t *r,
>  static bool vtimer_emulate_cp32(struct cpu_user_regs *regs, union hsr hsr)
>  {
>      struct hsr_cp32 cp32 = hsr.cp32;
> -    /*
> -     * Initialize to zero to avoid leaking data if there is an
> -     * implementation error in the emulation (such as not correctly
> -     * setting r).
> -     */
> -    uint32_t r = 0;
> -    bool res;
> -
>  
>      if ( cp32.read )
>          perfc_incr(vtimer_cp32_reads);
>      else
>          perfc_incr(vtimer_cp32_writes);
>  
> -    if ( !cp32.read )
> -        r = get_user_reg(regs, cp32.reg);
> -
>      switch ( hsr.bits & HSR_CP32_REGS_MASK )
>      {
>      case HSR_CPREG32(CNTP_CTL):
> -        res = vtimer_cntp_ctl(regs, &r, cp32.read);
> -        break;
> +        return vreg_emulate_cp32(regs, hsr, vtimer_cntp_ctl);
>  
>      case HSR_CPREG32(CNTP_TVAL):
> -        res = vtimer_cntp_tval(regs, &r, cp32.read);
> -        break;
> +        return vreg_emulate_cp32(regs, hsr, vtimer_cntp_tval);
>  
>      default:
>          return false;
>      }
> -
> -    if ( res && cp32.read )
> -        set_user_reg(regs, cp32.reg, r);
> -
> -    return res;
>  }
>  
>  static bool vtimer_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr)
>  {
>      struct hsr_cp64 cp64 = hsr.cp64;
> -    uint32_t r1 = get_user_reg(regs, cp64.reg1);
> -    uint32_t r2 = get_user_reg(regs, cp64.reg2);
> -    uint64_t x = (uint64_t)r1 | ((uint64_t)r2 << 32);
>  
>      if ( cp64.read )
>          perfc_incr(vtimer_cp64_reads);
> @@ -304,21 +283,11 @@ static bool vtimer_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr)
>      switch ( hsr.bits & HSR_CP64_REGS_MASK )
>      {
>      case HSR_CPREG64(CNTP_CVAL):
> -        if ( !vtimer_cntp_cval(regs, &x, cp64.read) )
> -            return false;
> -        break;
> +        return vreg_emulate_cp64(regs, hsr, vtimer_cntp_cval);
>  
>      default:
>          return false;
>      }
> -
> -    if ( cp64.read )
> -    {
> -        set_user_reg(regs, cp64.reg1, x & 0xffffffff);
> -        set_user_reg(regs, cp64.reg2, x >> 32);
> -    }
> -
> -    return true;
>  }
>  
>  #ifdef CONFIG_ARM_64
> diff --git a/xen/include/asm-arm/vreg.h b/xen/include/asm-arm/vreg.h
> index 2671f6e..ed2bd6f 100644
> --- a/xen/include/asm-arm/vreg.h
> +++ b/xen/include/asm-arm/vreg.h
> @@ -4,14 +4,68 @@
>  #ifndef __ASM_ARM_VREG__
>  #define __ASM_ARM_VREG__
>  
> -#ifdef CONFIG_ARM_64
> -typedef bool (*vreg_sysreg32_fn_t)(struct cpu_user_regs *regs, uint32_t *r,
> +typedef bool (*vreg_reg32_fn_t)(struct cpu_user_regs *regs, uint32_t *r,
>                                     bool read);
> -typedef bool (*vreg_sysreg64_fn_t)(struct cpu_user_regs *regs, uint64_t *r,
> +typedef bool (*vreg_reg64_fn_t)(struct cpu_user_regs *regs, uint64_t *r,
>                                     bool read);
>  
> +static inline bool vreg_emulate_cp32(struct cpu_user_regs *regs, union hsr hsr,
> +                                     vreg_reg32_fn_t fn)
> +{
> +    struct hsr_cp32 cp32 = hsr.cp32;
> +    /*
> +     * Initialize to zero to avoid leaking data if there is an
> +     * implementation error in the emulation (such as not correctly
> +     * setting r).
> +     */
> +    uint32_t r = 0;
> +    bool ret;
> +
> +    if ( !cp32.read )
> +        r = get_user_reg(regs, cp32.reg);
> +
> +    ret = fn(regs, &r, cp32.read);
> +
> +    if ( ret && cp32.read )
> +        set_user_reg(regs, cp32.reg, r);
> +
> +    return ret;
> +}
> +
> +static inline bool vreg_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr,
> +                                     vreg_reg64_fn_t fn)
> +{
> +    struct hsr_cp64 cp64 = hsr.cp64;
> +    /*
> +     * Initialize to zero to avoid leaking data if there is an
> +     * implementation error in the emulation (such as not correctly
> +     * setting x).
> +     */
> +    uint64_t x = 0;
> +    bool ret;
> +
> +    if ( !cp64.read )
> +    {
> +        uint32_t r1 = get_user_reg(regs, cp64.reg1);
> +        uint32_t r2 = get_user_reg(regs, cp64.reg2);
> +
> +        x = (uint64_t)r1 | ((uint64_t)r2 << 32);
> +    }
> +
> +    ret = fn(regs, &x, cp64.read);
> +
> +    if ( ret && cp64.read )
> +    {
> +        set_user_reg(regs, cp64.reg1, x & 0xffffffff);
> +        set_user_reg(regs, cp64.reg2, x >> 32);
> +    }
> +
> +    return ret;
> +}
> +
> +#ifdef CONFIG_ARM_64
>  static inline bool vreg_emulate_sysreg32(struct cpu_user_regs *regs, union hsr hsr,
> -                                         vreg_sysreg32_fn_t fn)
> +                                         vreg_reg32_fn_t fn)
>  {
>      struct hsr_sysreg sysreg = hsr.sysreg;
>      uint32_t r = 0;
> @@ -29,7 +83,7 @@ static inline bool vreg_emulate_sysreg32(struct cpu_user_regs *regs, union hsr h
>  }
>  
>  static inline bool vreg_emulate_sysreg64(struct cpu_user_regs *regs, union hsr hsr,
> -                                         vreg_sysreg64_fn_t fn)
> +                                         vreg_reg64_fn_t fn)
>  {
>      struct hsr_sysreg sysreg = hsr.sysreg;
>      /*
> -- 
> 1.9.1
>

Patch hide | download patch | download mbox

diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index 091b5e7..4ec3b95 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -252,49 +252,28 @@  static bool vtimer_cntp_cval(struct cpu_user_regs *regs, uint64_t *r,
 static bool vtimer_emulate_cp32(struct cpu_user_regs *regs, union hsr hsr)
 {
     struct hsr_cp32 cp32 = hsr.cp32;
-    /*
-     * Initialize to zero to avoid leaking data if there is an
-     * implementation error in the emulation (such as not correctly
-     * setting r).
-     */
-    uint32_t r = 0;
-    bool res;
-
 
     if ( cp32.read )
         perfc_incr(vtimer_cp32_reads);
     else
         perfc_incr(vtimer_cp32_writes);
 
-    if ( !cp32.read )
-        r = get_user_reg(regs, cp32.reg);
-
     switch ( hsr.bits & HSR_CP32_REGS_MASK )
     {
     case HSR_CPREG32(CNTP_CTL):
-        res = vtimer_cntp_ctl(regs, &r, cp32.read);
-        break;
+        return vreg_emulate_cp32(regs, hsr, vtimer_cntp_ctl);
 
     case HSR_CPREG32(CNTP_TVAL):
-        res = vtimer_cntp_tval(regs, &r, cp32.read);
-        break;
+        return vreg_emulate_cp32(regs, hsr, vtimer_cntp_tval);
 
     default:
         return false;
     }
-
-    if ( res && cp32.read )
-        set_user_reg(regs, cp32.reg, r);
-
-    return res;
 }
 
 static bool vtimer_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr)
 {
     struct hsr_cp64 cp64 = hsr.cp64;
-    uint32_t r1 = get_user_reg(regs, cp64.reg1);
-    uint32_t r2 = get_user_reg(regs, cp64.reg2);
-    uint64_t x = (uint64_t)r1 | ((uint64_t)r2 << 32);
 
     if ( cp64.read )
         perfc_incr(vtimer_cp64_reads);
@@ -304,21 +283,11 @@  static bool vtimer_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr)
     switch ( hsr.bits & HSR_CP64_REGS_MASK )
     {
     case HSR_CPREG64(CNTP_CVAL):
-        if ( !vtimer_cntp_cval(regs, &x, cp64.read) )
-            return false;
-        break;
+        return vreg_emulate_cp64(regs, hsr, vtimer_cntp_cval);
 
     default:
         return false;
     }
-
-    if ( cp64.read )
-    {
-        set_user_reg(regs, cp64.reg1, x & 0xffffffff);
-        set_user_reg(regs, cp64.reg2, x >> 32);
-    }
-
-    return true;
 }
 
 #ifdef CONFIG_ARM_64
diff --git a/xen/include/asm-arm/vreg.h b/xen/include/asm-arm/vreg.h
index 2671f6e..ed2bd6f 100644
--- a/xen/include/asm-arm/vreg.h
+++ b/xen/include/asm-arm/vreg.h
@@ -4,14 +4,68 @@ 
 #ifndef __ASM_ARM_VREG__
 #define __ASM_ARM_VREG__
 
-#ifdef CONFIG_ARM_64
-typedef bool (*vreg_sysreg32_fn_t)(struct cpu_user_regs *regs, uint32_t *r,
+typedef bool (*vreg_reg32_fn_t)(struct cpu_user_regs *regs, uint32_t *r,
                                    bool read);
-typedef bool (*vreg_sysreg64_fn_t)(struct cpu_user_regs *regs, uint64_t *r,
+typedef bool (*vreg_reg64_fn_t)(struct cpu_user_regs *regs, uint64_t *r,
                                    bool read);
 
+static inline bool vreg_emulate_cp32(struct cpu_user_regs *regs, union hsr hsr,
+                                     vreg_reg32_fn_t fn)
+{
+    struct hsr_cp32 cp32 = hsr.cp32;
+    /*
+     * Initialize to zero to avoid leaking data if there is an
+     * implementation error in the emulation (such as not correctly
+     * setting r).
+     */
+    uint32_t r = 0;
+    bool ret;
+
+    if ( !cp32.read )
+        r = get_user_reg(regs, cp32.reg);
+
+    ret = fn(regs, &r, cp32.read);
+
+    if ( ret && cp32.read )
+        set_user_reg(regs, cp32.reg, r);
+
+    return ret;
+}
+
+static inline bool vreg_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr,
+                                     vreg_reg64_fn_t fn)
+{
+    struct hsr_cp64 cp64 = hsr.cp64;
+    /*
+     * Initialize to zero to avoid leaking data if there is an
+     * implementation error in the emulation (such as not correctly
+     * setting x).
+     */
+    uint64_t x = 0;
+    bool ret;
+
+    if ( !cp64.read )
+    {
+        uint32_t r1 = get_user_reg(regs, cp64.reg1);
+        uint32_t r2 = get_user_reg(regs, cp64.reg2);
+
+        x = (uint64_t)r1 | ((uint64_t)r2 << 32);
+    }
+
+    ret = fn(regs, &x, cp64.read);
+
+    if ( ret && cp64.read )
+    {
+        set_user_reg(regs, cp64.reg1, x & 0xffffffff);
+        set_user_reg(regs, cp64.reg2, x >> 32);
+    }
+
+    return ret;
+}
+
+#ifdef CONFIG_ARM_64
 static inline bool vreg_emulate_sysreg32(struct cpu_user_regs *regs, union hsr hsr,
-                                         vreg_sysreg32_fn_t fn)
+                                         vreg_reg32_fn_t fn)
 {
     struct hsr_sysreg sysreg = hsr.sysreg;
     uint32_t r = 0;
@@ -29,7 +83,7 @@  static inline bool vreg_emulate_sysreg32(struct cpu_user_regs *regs, union hsr h
 }
 
 static inline bool vreg_emulate_sysreg64(struct cpu_user_regs *regs, union hsr hsr,
-                                         vreg_sysreg64_fn_t fn)
+                                         vreg_reg64_fn_t fn)
 {
     struct hsr_sysreg sysreg = hsr.sysreg;
     /*