[Xen-devel,v2,10/24] xen/arm: traps: Introduce a helper to read the hypersivor fault register

Message ID 20170912100330.2168-11-julien.grall@arm.com
State Accepted
Commit c8a8923e15656e4424200d3215258fdb25d881eb
Headers show
Series
  • xen/arm: Memory subsystem clean-up
Related show

Commit Message

Julien Grall Sept. 12, 2017, 10:03 a.m.
While ARM32 has 2 distinct registers for the hypervisor fault register
(one for prefetch abort, the other for data abort), AArch64 has only
one.

Currently, the logic is open-code but a follow-up patch will require to
read it too. So move the logic in a separate helper and use it instead
of open-coding it.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>

---
    Changes in v2:
        - Add Andre's reviewed-by
---
 xen/arch/arm/traps.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

Comments

Stefano Stabellini Sept. 16, 2017, 12:08 a.m. | #1
On Tue, 12 Sep 2017, Julien Grall wrote:
> While ARM32 has 2 distinct registers for the hypervisor fault register
> (one for prefetch abort, the other for data abort), AArch64 has only
> one.
> 
> Currently, the logic is open-code but a follow-up patch will require to
> read it too. So move the logic in a separate helper and use it instead
> of open-coding it.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

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


> ---
>     Changes in v2:
>         - Add Andre's reviewed-by
> ---
>  xen/arch/arm/traps.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 967bfea4ed..2e48089d61 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2549,6 +2549,28 @@ done:
>      if (first) unmap_domain_page(first);
>  }
>  
> +/*
> + * Return the value of the hypervisor fault address register.
> + *
> + * On ARM32, the register will be different depending whether the
> + * fault is a prefetch abort or data abort.
> + */
> +static inline vaddr_t get_hfar(bool is_data)
> +{
> +    vaddr_t gva;
> +
> +#ifdef CONFIG_ARM_32
> +    if ( is_data )
> +        gva = READ_CP32(HDFAR);
> +    else
> +        gva = READ_CP32(HIFAR);
> +#else
> +    gva =  READ_SYSREG(FAR_EL2);
> +#endif
> +
> +    return gva;
> +}
> +
>  static inline paddr_t get_faulting_ipa(vaddr_t gva)
>  {
>      register_t hpfar = READ_SYSREG(HPFAR_EL2);
> @@ -2584,11 +2606,7 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
>      paddr_t gpa;
>      mfn_t mfn;
>  
> -#ifdef CONFIG_ARM_32
> -    gva = READ_CP32(HIFAR);
> -#else
> -    gva = READ_SYSREG64(FAR_EL2);
> -#endif
> +    gva = get_hfar(false /* is_data */);
>  
>      /*
>       * If this bit has been set, it means that this instruction abort is caused
> @@ -2730,11 +2748,8 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
>          return __do_trap_serror(regs, true);
>  
>      info.dabt = dabt;
> -#ifdef CONFIG_ARM_32
> -    info.gva = READ_CP32(HDFAR);
> -#else
> -    info.gva = READ_SYSREG64(FAR_EL2);
> -#endif
> +
> +    info.gva = get_hfar(true /* is_data */);
>  
>      if ( hpfar_is_valid(dabt.s1ptw, fsc) )
>          info.gpa = get_faulting_ipa(info.gva);
> -- 
> 2.11.0
>

Patch

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 967bfea4ed..2e48089d61 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2549,6 +2549,28 @@  done:
     if (first) unmap_domain_page(first);
 }
 
+/*
+ * Return the value of the hypervisor fault address register.
+ *
+ * On ARM32, the register will be different depending whether the
+ * fault is a prefetch abort or data abort.
+ */
+static inline vaddr_t get_hfar(bool is_data)
+{
+    vaddr_t gva;
+
+#ifdef CONFIG_ARM_32
+    if ( is_data )
+        gva = READ_CP32(HDFAR);
+    else
+        gva = READ_CP32(HIFAR);
+#else
+    gva =  READ_SYSREG(FAR_EL2);
+#endif
+
+    return gva;
+}
+
 static inline paddr_t get_faulting_ipa(vaddr_t gva)
 {
     register_t hpfar = READ_SYSREG(HPFAR_EL2);
@@ -2584,11 +2606,7 @@  static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
     paddr_t gpa;
     mfn_t mfn;
 
-#ifdef CONFIG_ARM_32
-    gva = READ_CP32(HIFAR);
-#else
-    gva = READ_SYSREG64(FAR_EL2);
-#endif
+    gva = get_hfar(false /* is_data */);
 
     /*
      * If this bit has been set, it means that this instruction abort is caused
@@ -2730,11 +2748,8 @@  static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
         return __do_trap_serror(regs, true);
 
     info.dabt = dabt;
-#ifdef CONFIG_ARM_32
-    info.gva = READ_CP32(HDFAR);
-#else
-    info.gva = READ_SYSREG64(FAR_EL2);
-#endif
+
+    info.gva = get_hfar(true /* is_data */);
 
     if ( hpfar_is_valid(dabt.s1ptw, fsc) )
         info.gpa = get_faulting_ipa(info.gva);