Hi Stefano,
On 23/08/16 02:05, Stefano Stabellini wrote:
On Thu, 28 Jul 2016, Julien Grall wrote:
A data/instruction abort may have occurred if another CPU was playing
with the stage-2 page table when following the break-before-make
sequence (see D4.7.1 in ARM DDI 0487A.j). Rather than injecting directly
the fault to the guest, we need to check whether the mapping exists. If
it exists, return to the guest to replay the instruction.
Signed-off-by: Julien Grall <julien.gr...@arm.com>
---
xen/arch/arm/traps.c | 40 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index b46284c..da56cc0 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2404,6 +2404,7 @@ static void do_trap_instr_abort_guest(struct
cpu_user_regs *regs,
register_t gva = READ_SYSREG(FAR_EL2);
uint8_t fsc = hsr.iabt.ifsc & ~FSC_LL_MASK;
paddr_t gpa;
+ mfn_t mfn;
if ( hpfar_is_valid(hsr.iabt.s1ptw, fsc) )
gpa = get_faulting_ipa(gva);
@@ -2417,6 +2418,11 @@ static void do_trap_instr_abort_guest(struct
cpu_user_regs *regs,
*/
flush_tlb_local();
+ /*
+ * We may not be able to translate because someone is
+ * playing with the Stage-2 page table of the domain.
+ * Return to the guest.
+ */
rc = gva_to_ipa(gva, &gpa, GV2M_READ);
if ( rc == -EFAULT )
return; /* Try again */
@@ -2437,8 +2443,17 @@ static void do_trap_instr_abort_guest(struct
cpu_user_regs *regs,
/* Trap was triggered by mem_access, work here is done */
if ( !rc )
return;
+ break;
}
- break;
+ case FSC_FLT_TRANS:
Please add brackets under the case statement for code style
This is not part of the coding style. We only use the brackets when
local variable is defined for a specific case. See vgic-{v2,v3}.c for
instance.
Note that I am a bit surprised you complain here about the missing
brackets but you did not on commit def4273 "xen/arm: traps: MMIO should
only be emulated for fault translation" at the beginning of august.
+ /*
+ * The PT walk may have failed because someone was playing
+ * with the Stage-2 page table. Walk the Stage-2 PT to check
+ * if the entry exists. If it's the case, return to the guest
+ */
+ mfn = p2m_lookup(current->domain, _gfn(paddr_to_pfn(gpa)), NULL);
+ if ( !mfn_eq(mfn, INVALID_MFN) )
+ return;
Just checking, but isn't it possible to get a genuine translation abort
with an mfn != invalid_mfn?
A translation fault means the entry is not present in the page table at
the time the processor did the page table walk.
This may happen because the hypervisor is modifying the stage-2 page
table on another processor (for instance with the break-before-make
sequence).
p2m_lookup will do a software page walk to get entry. As the function is
using a read lock, it will wait until the other processor finishes to
update the page tables before doing the lookup. So if the mfn returned
is valid, then we know that the translation fault is spurious and should
return to the guest to retry the execution of the faulting instruction.
}
inject_iabt_exception(regs, gva, hsr.len);
@@ -2455,7 +2470,7 @@ static bool_t try_handle_mmio(struct cpu_user_regs *regs,
return 0;
/* All the instructions used on emulated MMIO region should be valid */
- if ( !dabt.valid )
+ if ( !info->dabt.valid )
return 0;
Spurious change?
Yes. I will drop it.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
