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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.12.18 at 14:47, <george.dunlap@xxxxxxxxxx> wrote:
> On 12/20/18 9:05 AM, Roger Pau Monné wrote:
>> On Wed, Dec 19, 2018 at 06:10:04PM +0000, George Dunlap wrote:
>>> 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.
>> 
>> With my proposal H1 won't be a valid lock sequence, since
>> current->mm_lock_x level > tgt->mm_lock_y level.
> 
> I made a mistake; here's a corrected and expanded example.
> 
> Suppose we have mm_lock_x, with value 8, and mm_lock_y with value 16.
> 
> Suppose we have H1, which grabs tgt->mm_lock_x and current->mm_lock_y.
> 
> And suppose we have H2, which grabs tgt->mm_lock_y, and current->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:
> 
> 1. H1: grab B->mm_lock_x
> 2. H2: grab A->mm_lock_y
> 3. H1: wait on A->mm_lock_y
> 4. H2: wait on B->mm_lock_x #DEADLOCK
> 
> With the current mm lock checking:
> * H1 grabs a lock of level 8, then a lock of level 16, so no BUG check.
> * H2 grabs a lock of level 16 and then a lock of level 8, and so BUG
> checks, catching this potential deadlock.
> 
> With your "current bias" patch:
> * H1 grabs a lock of level 8, then a lock of level 80 (16 + 64), so no
> BUG check.
> * H2 grabs a lock of level 16, then a lock of level 72 (8 + 64), so no
> BUG check, missing the potential deadlock.
> 
> With "dom0 bias", you basically have to choose one of the hypercalls to
> be privileged only.  If we choose H2 to be privileged, then B must be
> dom0 and A must be a domU.  So we get:
> * H1 grabs lock level 72 (16 + 64), then lock level 16, causing a BUG check.
> * H2 grabs lock level 16, then lock level 80, no bug check.
> 
> The practical implication of dom0 bias is that any hypercall which grabs
> two mm locks, one of two things must be true:
> * When it is called from one domU to another domU, locks must follow the
> "normal" locking order listed in mm-locks.h

This would be a problem in the stubdom case.

> * When it is called between a dom0 and a domU, it must be consistenly
> called either one way or the other; i.e., it must always be dom0 calling
> it with a domU target, or domU calling it with a dom0 target (otherwise
> the dom0 / domU locking order is violated).

Obviously (I hope) "domU calling it with a dom0 target" is an
uninteresting case, because we don't allow a domU to act on
dom0. Hence the consistency is achieved by it always going
to be "dom0 calling it with a domU target".

Jan


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