[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [RFC 01/22] xen/arm: do_trap_instr_abort_guest: Move the IPA computation out of the switch
On 31/08/16 20:43, Stefano Stabellini wrote:
On Tue, 16 Aug 2016, Julien Grall wrote:
Hi Stefano,
On 16/08/2016 01:21, Stefano Stabellini wrote:
On Thu, 28 Jul 2016, Julien Grall wrote:
A follow-up patch will add more case to the switch that will require the
IPA. So move the computation out of the switch.
Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
---
xen/arch/arm/traps.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 683bcb2..46e0663 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2403,35 +2403,35 @@ static void do_trap_instr_abort_guest(struct
cpu_user_regs *regs,
int rc;
register_t gva = READ_SYSREG(FAR_EL2);
uint8_t fsc = hsr.iabt.ifsc & ~FSC_LL_MASK;
+ paddr_t gpa;
+
+ if ( hpfar_is_valid(hsr.iabt.s1ptw, fsc) )
+ gpa = get_faulting_ipa(gva);
+ else
+ {
+ /*
+ * Flush the TLB to make sure the DTLB is clear before
+ * doing GVA->IPA translation. If we got here because of
+ * an entry only present in the ITLB, this translation may
+ * still be inaccurate.
+ */
+ flush_tlb_local();
+
+ rc = gva_to_ipa(gva, &gpa, GV2M_READ);
+ if ( rc == -EFAULT )
+ return; /* Try again */
The issue with this is that now for any cases that don't require a gpa
if gva_to_ipa fails we wrongly return -EFAULT.
Well, stage-1 fault is prioritized over stage-2 fault (see B3.12.3 in ARM DDI
0406C.b), so gva_to_ipa should never fail unless someone is playing with the
stage-1 page table at the same time or because of an erratum (see 834220). In
both case, we should replay the instruction to let the processor injecting the
correct fault.
FWIW, this is already what we do for the data abort handler.
I suggest having two switches or falling through from the first case to
the second.
I am not sure to understand your suggestion. Could you detail it?
I was merely suggesting to add another switch like:
+ switch ( fsc )
+ {
+ case FSC_FLT_PERM:
+ case BLA:
+ find gpa;
+ default:
+ }
switch ( fsc )
{
but given that we are already relying on the gva_to_ipa translation in
the data abort handler, your patch is fine too.
Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Thank you! FWIW, I am planning to merge the instruction and data abort
paths after Xen 4.8. I will avoid to diverge between the two
implementations.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
