| Message ID | 1466601669-25398-9-git-send-email-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
On 14/07/16 16:27, Stefano Stabellini wrote: > On Wed, 22 Jun 2016, Julien Grall wrote: >> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c >> index 591de3c..0edc2cc 100644 >> --- a/xen/arch/arm/traps.c >> +++ b/xen/arch/arm/traps.c >> @@ -2383,13 +2383,28 @@ static inline paddr_t get_faulting_ipa(vaddr_t gva) [..] >> 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: >> { >> @@ -2400,7 +2415,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 >> { >> @@ -2435,6 +2450,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; > > You should be able to modify the switch in this case too, right? Correct. I am thinking to pull the changes in patch #4 to avoid extra-changes in this patch. Regards,
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 591de3c..0edc2cc 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -2383,13 +2383,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: { @@ -2400,7 +2415,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 { @@ -2435,6 +2450,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 @@ -2443,7 +2459,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) ) info.gpa = get_faulting_ipa(info.gva); else {
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@arm.com> --- xen/arch/arm/traps.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)