Re: [PATCH] x86/domctl: Adjust size calculations for XEN_DOMCTL_get{_ext_vcpucontext, vcpuextstate}

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 28/04/14 12:37, Jan Beulich wrote:
>>>> On 28.04.14 at 12:59, <andrew.cooper3@citrix.com> wrote:
>> On 28/04/14 11:34, Jan Beulich wrote:
>>>>>> On 28.04.14 at 11:43, <andrew.cooper3@citrix.com> wrote:
>>>> XEN_DOMCTL_get_ext_vcpucontext suffers from the same issue but while trying to
>>>> fix that in similar way, I discovered that it had a genuine bug when returning
>>>> the count of MSRs to the toolstack.  When running the hypercall on an active
>>>> vcpu, the vcpu can arbitrarily alter the count returned to the toolstack by
>>>> clearing and setting relevant MSRs.
>>> Did you perhaps overlook the vcpu_pause() there?
>> There is a vcpu pause in the hypercall, so for the duration of the
>> hypercall the returned value will be consistent.
>>
>> However without the toolstack pausing the domain, issuing this hypercall
>> twice, first to get the size and second to get the data might still
>> result in -ENOBUFS if the vcpu suddenly writes non-0 values to the MSRs.
> And in what way is this different from e.g. XEN_DOMCTL_get_vcpuextstate?

As xcr0_accum is strictly increasing and only in a few possible steps,
the size returned can never decrease.  As it is context switch material,
the chances are very good that it will reach the maximum the guest
kernel is willing to use a long time before migration happens.

But this does highlight a slight race condition in the migration code
which I will fix up in the v2 series.

>
>>> I'm also not really in favor of forcing the tools to allocate memory
>>> for the array if in fact no MSRs are being used by the guest.
>> If there are no msrs to receive, then passing a NULL guest handle is
>> still fine.
> But the caller can't know whether the count was non-zero just because
> that's the theoretical maximum or because some MSR really is in use.
>
> Jan
>

Why is that a problem?

If the toolstack wants to save any possible MSRs the guest is using,
then it is going to have to provide a buffer large enough for any
eventual number of MSRs.  In the case that the buffer is sufficiently
sized, Xen writes back msr_count with the number of MSRs written, so the
toolstack can detect when fewer MSRs have been written back.

~Andrew