[for-4.7 v2 0/2] xen/arm: traps: Correctly interpret the content of the register HPFAR_EL2

[for-4.7 v2 0/2] xen/arm: traps: Correctly interpret the content of the register HPFAR_EL2

[Julien Grall]

Hello,

This small patch series is a bug fix for Xen 4.7 and should also be backported
to Xen 4.6.

Without it, the faulting IPA reported to memaccess may be wrong.

For all the changes see in each patch.

Regards,

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

Julien Grall (2):
  xen/bitops: Introduce GENMASK to generate mask
  xen/arm: traps: Correctly interpret the content of the register
    HPFAR_EL2

 xen/arch/arm/traps.c            | 11 +++++++++--
 xen/include/asm-arm/processor.h |  7 +++++++
 xen/include/xen/bitops.h        |  8 ++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[for-4.7 v2 1/2] xen/bitops: Introduce GENMASK to generate mask

[Julien Grall]

The code has been imported from the header include/linux/bitops.h in
Linux v4.6-rc3.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>

---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>

    Changes in v2:
        - Remove GENMASK_ULL
        - Add Stefano's acked-by
---
 xen/include/xen/bitops.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
index cb56f24..bd0883a 100644
--- a/xen/include/xen/bitops.h
+++ b/xen/include/xen/bitops.h
@@ -3,6 +3,14 @@
 #include <asm/types.h>
 
 /*
+ * Create a contiguous bitmask starting at bit position @l and ending at
+ * position @h. For example
+ * GENMASK(30, 21) gives us the 32bit vector 0x01fe00000.
+ */
+#define GENMASK(h, l) \
+    (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
+
+/*
  * ffs: find first bit set. This is defined the same way as
  * the libc and compiler builtin ffs routines, therefore
  * differs in spirit from the above ffz (man ffs).
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[for-4.7 v2 2/2] xen/arm: traps: Correctly interpret the content of the register HPFAR_EL2

[Julien Grall]

The register HPFAR_EL2 (resp. HPFAR on arm32) contains the bits [47:12]
(resp. [39:12]) of the faulting IPA. Unlike other registers that represent
an address, the upper bits of the IPA are stored in the register bits
[4:39] (resp. [4:21]).

However, Xen assumes that the register contains the faulting IPA correctly
offsetted. This will result to get a wrong IPA when the fault is happening
during a translation table walk. Note this is only affecting  memaccess.

Introduce a new helper to get the faulting IPA from HPFAR_EL2 and
replace direct read from the register by the helper.

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

---
Cc: tamas@tklengyel.com

This is a bug fix for Xen 4.7 and should also be backported to Xen 4.6.
Without this patch, the faulting IPA reported to memaccess may be wrong.

    Changes in v2:
        - Add Andre's and Stefano's reviewed-by
---
 xen/arch/arm/traps.c            | 11 +++++++++--
 xen/include/asm-arm/processor.h |  7 +++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 1516abd..5e865cf 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2363,6 +2363,13 @@ done:
     if (first) unmap_domain_page(first);
 }
 
+static inline paddr_t get_faulting_ipa(void)
+{
+    register_t hpfar = READ_SYSREG(HPFAR_EL2);
+
+    return ((paddr_t)(hpfar & HPFAR_MASK) << (12 - 4));
+}
+
 static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
                                       const union hsr hsr)
 {
@@ -2381,7 +2388,7 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
         };
 
         if ( hsr.iabt.s1ptw )
-            gpa = READ_SYSREG(HPFAR_EL2);
+            gpa = get_faulting_ipa();
         else
         {
             /*
@@ -2431,7 +2438,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
 #endif
 
     if ( dabt.s1ptw )
-        info.gpa = READ_SYSREG(HPFAR_EL2);
+        info.gpa = get_faulting_ipa();
     else
     {
         rc = gva_to_ipa(info.gva, &info.gpa, GV2M_READ);
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 7e6eb66..6789cd0 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -565,6 +565,13 @@ union hsr {
 
 #define FSC_LL_MASK    (_AC(0x03,U)<<0)
 
+/* HPFAR_EL2: Hypervisor IPA Fault Address Register */
+#ifdef CONFIG_ARM_64
+#define HPFAR_MASK	GENMASK(39, 4)
+#else
+#define HPFAR_MASK	GENMASK(31, 4)
+#endif
+
 /* Time counter hypervisor control register */
 #define CNTHCTL_EL2_EL1PCTEN (1u<<0) /* Kernel/user access to physical counter */
 #define CNTHCTL_EL2_EL1PCEN  (1u<<1) /* Kernel/user access to CNTP timer regs */
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [for-4.7 v2 1/2] xen/bitops: Introduce GENMASK to generate mask

[Jan Beulich]

>>> On 22.04.16 at 17:58, <julien.grall@arm.com> wrote:
> The code has been imported from the header include/linux/bitops.h in
> Linux v4.6-rc3.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Acked-by: Stefano Stabellini <sstabellini@kernel.org>

And just to double check - Stefano, this ack was meant also as a
REST maintainer, not just an ARM one (as the latter was the
context it was originally given in)?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [for-4.7 v2 1/2] xen/bitops: Introduce GENMASK to generate mask

[Stefano Stabellini]

On Fri, 22 Apr 2016, Jan Beulich wrote:
> >>> On 22.04.16 at 17:58, <julien.grall@arm.com> wrote:
> > The code has been imported from the header include/linux/bitops.h in
> > Linux v4.6-rc3.
> > 
> > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> And just to double check - Stefano, this ack was meant also as a
> REST maintainer, not just an ARM one (as the latter was the
> context it was originally given in)?

Yeah, the macro is OK for me.

It is just a macro - we don't have to introduce it if you dislike it, we
could simply explode it in xen/include/asm-arm/processor.h. On the other
hand, if you don't have an opinion about it, I would go ahead with this
patch. I am happy either way.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [for-4.7 v2 1/2] xen/bitops: Introduce GENMASK to generate mask

[Ian Jackson]

Jan Beulich writes ("Re: [for-4.7 v2 1/2] xen/bitops: Introduce GENMASK to generate mask"):
> On 22.04.16 at 17:58, <julien.grall@arm.com> wrote:
> > The code has been imported from the header include/linux/bitops.h in
> > Linux v4.6-rc3.
> > 
> > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> And just to double check - Stefano, this ack was meant also as a
> REST maintainer, not just an ARM one (as the latter was the
> context it was originally given in)?

For what it's worth, on the matter of taste I agree with Julien.
Having read the occasional ARM manual, the proposed macro will make it
much easier to check that the code matches the book.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel