[Xen-devel] [PATCH v3 5/6] xen/arm: traps: Avoid unnecessary VA -> IPA translation in abort handlers

[Julien Grall <julien.grall@xxxxxxx>]

Translating a VA to a IPA is expensive. Currently, Xen is assuming that
HPFAR_EL2 is only valid when the stage-2 data/instruction abort happened
during a translation table walk of a first stage translation (i.e S1PTW
is set).

However, based on the ARM ARM (D7.2.34 in DDI 0487A.j), the register is
also valid when the data/instruction abort occured for a translation
fault.

With this change, the VA -> IPA translation will only happen for
permission faults that are not related to a translation table of a
first stage translation.

Signed-off-by: Julien Grall <julien.grall@xxxxxxx>

---
    Changes in v3:
        - Use the correct union to get the s1ptw bit.

    Changes in v2:
        - Use fsc in the switch in do_trap_data_abort_guest
---
 xen/arch/arm/traps.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 46922eb..0ec0361 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2382,13 +2382,28 @@ static inline paddr_t get_faulting_ipa(vaddr_t gva)
     return ipa;
 }
 
+static inline bool hpfar_is_valid(bool s1ptw, uint8_t fsc)
+{
+    /*
+     * HPFAR is valid if one of the following cases are true:
+     *  1. the stage 2 fault happen during a stage 1 page table walk
+     *  (the bit ESR_EL2.S1PTW is set)
+     *  2. the fault was due to a translation fault
+     *
+     * Note that technically HPFAR is valid for other cases, but they
+     * are currently not supported by Xen.
+     */
+    return s1ptw || (fsc == FSC_FLT_TRANS);
+}
+
 static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
                                       const union hsr hsr)
 {
     int rc;
     register_t gva = READ_SYSREG(FAR_EL2);
+    uint8_t fsc = hsr.iabt.ifsc & ~FSC_LL_MASK;
 
-    switch ( hsr.iabt.ifsc & ~FSC_LL_MASK )
+    switch ( fsc )
     {
     case FSC_FLT_PERM:
     {
@@ -2399,7 +2414,7 @@ static void do_trap_instr_abort_guest(struct 
cpu_user_regs *regs,
             .kind = hsr.iabt.s1ptw ? npfec_kind_in_gpt : npfec_kind_with_gla
         };
 
-        if ( hsr.iabt.s1ptw )
+        if ( hpfar_is_valid(hsr.iabt.s1ptw, fsc) )
             gpa = get_faulting_ipa(gva);
         else
         {
@@ -2434,6 +2449,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs 
*regs,
     const struct hsr_dabt dabt = hsr.dabt;
     int rc;
     mmio_info_t info;
+    uint8_t fsc = hsr.dabt.dfsc & ~FSC_LL_MASK;
 
     info.dabt = dabt;
 #ifdef CONFIG_ARM_32
@@ -2442,7 +2458,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs 
*regs,
     info.gva = READ_SYSREG64(FAR_EL2);
 #endif
 
-    if ( dabt.s1ptw )
+    if ( hpfar_is_valid(dabt.s1ptw, fsc) )
         info.gpa = get_faulting_ipa(info.gva);
     else
     {
@@ -2451,7 +2467,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs 
*regs,
             return; /* Try again */
     }
 
-    switch ( dabt.dfsc & ~FSC_LL_MASK )
+    switch ( fsc )
     {
     case FSC_FLT_PERM:
     {
-- 
1.9.1


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