On 14/09/16 08:53, Sergej Proskurin wrote:
Hi Julien
[...]
static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
const union hsr hsr)
{
@@ -2445,6 +2465,14 @@ static void do_trap_instr_abort_guest(struct
cpu_user_regs *regs,
break;
case FSC_FLT_TRANS:
/*
+ * The guest shall retry accessing the page if the altp2m
handler
+ * succeeds. Otherwise, we continue injecting an instruction
abort
+ * exception.
+ */
+ if ( try_handle_altp2m(current, gpa, npfec) )
+ return;
I would have expected that altp2m is the last thing we want to check
in the abort handler. I.e after p2m_lookup.
Alright, I will reorder both calls, thank you.
Actually, reordering of these calls (try_handle_altp2m/altp2m_lazy_copy
and p2m_lookup) would make the system stall if altp2m is active. This is
because the p2m_lookup routine considers only the host's p2m, which will
most likely return a mfn != INVALID_MFN and thus entirely skip the call
to altp2m_lazy_copy. Thus, both calls should rather be conditional as
shown in the following:
if ( likely(!altp2m_active(current->domain)) )
{
/*
* 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;
}
else
/*
* The guest shall retry accessing the page if the altp2m handler
* succeeds. Otherwise, we continue injecting an instruction abort
* exception.
*/
if ( altp2m_lazy_copy(current, _gfn(paddr_to_pfn(gpa))) )
return;
I think this solution is racy because altp2m_active could be changed by
another physical CPU whilst we are in the abort handler.
So the best here is to keep the previous solution:
if ( altp2m_lazy_copy(current, _gfn(paddr_to_pfn(gpa))) )
return;
if ( p2m_lookup(current->domain, _gfn(paddr_to_pfn(gpa)), NULL )
return;
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
