[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH for-4.12 V4] x86/altp2m: fix HVMOP_altp2m_set_domain_state race

> On Feb 12, 2019, at 11:42 AM, Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx> 
> wrote:
> HVMOP_altp2m_set_domain_state does not domain_pause(), presumably
> on purpose (as it was originally supposed to cater to a in-guest
> agent, and a domain pausing itself is not a good idea).

Sorry to come in here on v4 and suggest changing everything, but I don’t really 
like the solution you have here.  Not setting altp2m to ‘active’ until after 
the vcpus are set up makes sense; but passing this true/false value in seems 
ugly, and still seems a bit racy (i.e., what if p2m_active() is disabled 
between the check in HVMOP_altp2m_switch_p2m and the time we actually call 

I certainly don’t think domain_pause() should be our go-to solution for race 
avoidance, but in this case it really seems like switching the global p2m for 
every vcpu at once makes the most sense; and trying to safely change this on an 
unpaused domain is not only overly complicated, but probably not what we wanted 

p2m_altp2m_destroy_by_id() and p2m_switch_domain_altp2m_by_id() already use 
domain_pause_except_self(); so it seems like not using it for 
altp2m_set_domain_state was probably more of an oversight than an intentional 
decision.  Using that here seems like a more robust solution.

The one issue is that domain_pause_except_self() currently is actually a 
deadlock risk if two different vcpus start it at the same time.  I think the 
attached patch (compile-tested only) should fix this issue; after this patch 
you should be able to use domain_pause_except_self() in altp2m_set_domain_state 

Does that sound reasonable?


Attachment: 0001-altp2m-Prevent-deadlocks-when-a-domain-performs-altp.patch
Description: 0001-altp2m-Prevent-deadlocks-when-a-domain-performs-altp.patch

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.