Re: [Xen-devel] [PATCH 3/3] x86/mm-locks: apply a bias to lock levels for current domain

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 12/19/18 2:59 PM, Roger Pau Monné wrote:
>> Using 'current' means that potential deadlocks which would now cause a
>> BUG() won't anymore.  I'm fine with not adding extra protections that
>> aren't there now; but I don't want to remove protections that are.
> 
> The lock ordering enforcement is still kept as-is, but Xen is allowed
> to lock the caller mm locks in the right order (be it privileged or
> not) after having locked a subject domain ones also in the correct
> order.

Suppose we have mm_lock_x, with value 8, and mm_lock_y with value 16.

Suppose we have H1, which grabs current->mm_lock_x and tgt->mm_lock_y.

And suppose we have H2, which grabs current->mm_lock_y, and tgt->mm_lock_x.

And suppose domA calls H1 on domB at the same time that domB calls H2 on
domA.  We could have the following sequence:

* H1: grab A->mm_lock_x
* H2: grab B->mm_lock_y
* H1: wait on B->mm_lock_y
* H2: wait on A->mm_lock_x #DEADLOCK

With the current mm lock checking, H2 would cause a BUG() the first time
it was called.

With my "special privilege only" boosting you'd also get a BUG() in at
least one of the invocations.

With your "current bias" patch, no BUG() would be encountered; we'd only
discover the deadlock once a live server had actually deadlocked.

This is what I'm talking about -- with "boost dom0", you have a global
order to the locks.  It goes:

1-8: All domU locks (in the order listed in mm-locks.h)
9-16: All dom0 locks

Thus we know for certain that there can't be a caller / callee deadlock
that's not detected.  With your patch, there isn't a global order: the
order changes based on who made the hypercall, so it's more difficult to
reason about whether there's a deadlock or not.

So, do H1 and H2 exist right now?  I think probably not, but I can't
immediately say.  Will such a pair *never* exist?  I don't think I can
guarantee that either.  That's why I want something to check.

> Using is_control_domain will limit the usage of paging_log_dirty_op to
> Dom0 (because that's the only domain that has is_privileged set,
> there's no way for the toolstack to create a domain with is_privileged
> set), no other paging domain will be able to use such hypercall
> without triggering the lock level check. Is that really what we
> want?
>
> I assume that under a distributed Xen system it would be possible to
> issue this hypercall from a domain different that Dom0 if it's given
> the right privileges?

I take it you mean a "disaggregated" system?  This is what Andy was
talking about with XSM.  As I said to him, at the moment, this will
_already_ fail, so it's not a regression.

I'm not willing to open up new potential holes for deadlocks,
particularly for a hypothetical use case that nobody has asked for and
doesn't work upstream at the moment.

If you want to make the calls succeed under a more "peer-to-peer"
arrangement, you're going to have to come up with something else:
Change the order of the mm locks, or grab the caller's p2m lock before
grabbing the paging lock, or refactoring what's covered by what, or
something.

Or, we can fix what's actually broken at the moment, and fix other
things when we encounter them.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel