|
Hi Julien,
On 07/27/2016 07:09 PM, Julien Grall
wrote:
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 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 ea105f2..83a30fa 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(hsr.iabt.s1ptw, fsc) )
I belive this should be:
hpfar_is_valid(hsr.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:
{
Cheers,
~Sergej
|
