Re: [Xen-devel] [PATCH] x86/PV: fix unintended dependency of m2p-strict mode on migration-v2

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.02.16 at 18:31, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 01/02/16 16:51, Jan Beulich wrote:
>>>>> On 01.02.16 at 17:34, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 01/02/16 16:28, Jan Beulich wrote:
>>>>>>> On 01.02.16 at 15:07, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> On 01/02/16 13:20, Jan Beulich wrote:
>>>>>> Ping? (I'd really like to get this resolved, so we don't need to
>>>>>> indefinitely run with non-upstream behavior in our distros.)
>>>>> My remaining issue is whether this loop gets executed by default.
>>>>>
>>>>> I realise that there is a difference between legacy and v2 migration,
>>>>> and that v2 migration by default worked.  If that means we managed to
>>>>> skip this loop in its entirety for v2, then I am far less concerned
>>>>> about the overhead.
>>>> But had been there before: Of course we could skip the loop if
>>>> the bit in d->vm_assist doesn't change. But as expressed before,
>>>> with you having already indicated that perhaps it would be better
>>>> to have v2 migration do the relevant operations in the other (v1)
>>>> order, the moment that was actually done the benefit of avoiding
>>>> the loop would be gone.
>>>>
>>>> To be clear - if rendering the code dead (which is what you ask
>>>> for) until v2 migration happens to get changed is the only way to
>>>> get this code in, I will submit a v2 with that extra conditional.
>>> Migration v2 currently loads vcpu context before pinning the pagetables,
>>> which means that the vm_assist should get set up properly, before L4
>>> tables are processed.
>>>
>>> It was my understanding that this is the correct way around, and
>>> m2p-strict mode only broke when you backported it to migration v1 systems?
>> Yes, but when we discussed this you said "in hindsight, it would have
>> been more sensible to swap pin and vCPU context, but I guess that
>> would break m2p-strict in the same way", and whether one is more
>> "correct" than the other is pretty questionable.
> 
> Right, but as it currently stands, migration v2 gets things the correct
> way around?

Yes.

> Does that mean that, at the moment, this loop ends up getting skipped?

No.

> If it does, then the patch is fine.
> 
> The suggestion to swap the actions around was only to work around the
> apparent error caused by continutions while loading vcpu0's state,
> caused by auditing and pinning all the pagetables hanging off %cr3/%cr1.
> 
> If however there is a legitimate reason, such as this, to keep the
> current order of operations, then it would be counterproductive to make
> any changes to migration v2.

Okay. I'll make the adjustment then.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel