Re: [PATCH] clarify SHUTDOWN_suspend additional argument

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

On 08/05/14 11:40, Ian Campbell wrote:
> On Thu, 2014-05-08 at 11:21 +0100, Andrew Cooper wrote:
>> On 08/05/14 11:11, Ian Campbell wrote:
>>> On Wed, 2014-05-07 at 14:05 +0100, Stefano Stabellini wrote:
>>>> SCHEDOP_shutdown has a third argument that is unused on HVM and ARM
>>>> guests. Those guests pass 0 instead. Clarify the behaviour in the
>>>> hypercall description.
>>>>
>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>>>
>>> But:
>>>> diff --git a/xen/include/public/sched.h b/xen/include/public/sched.h
>>>> index a30b11d..c170556 100644
>>>> --- a/xen/include/public/sched.h
>>>> +++ b/xen/include/public/sched.h
>>>> @@ -77,8 +77,9 @@
>>>>   * @arg == pointer to sched_shutdown_t structure.
>>>>   *
>>>>   * If the sched_shutdown_t reason is SHUTDOWN_suspend then this
>>>> - * hypercall takes an additional extra argument which should be the
>>>> - * MFN of the guest's start_info_t.
>>>> + * hypercall takes an additional extra argument which should be:
>>>> + *  - the MFN of the guest's start_info_t for x86 PV guests;
>>>> + *  - 0 for x86 HVM guests and arm and arm64 guests.
>>> Is this strictly true or is the argument ignored for those guests?
>>> (Requiring it to be zero doesn't conflict with that hence the ack)
>> Having recently played with code here, I believe it is as follows.
>>
>> * Xen does absolutely nothing with the value whatsoever.
>> * PV Migration *must* check the value (and indeed converts it to a pfn
>> for transit)
>> * HVM guests (including arm for these purposes) don't have easy access
>> to the register, and just transmit the Xen architectural state blob as-is.
>>
>> Therefore, the only requirement I can see is that PV guests must point
>> it at the start_info_t mfn.
> This matched my understanding too, thanks for confirming.
>
> There's no harm in mandating it be zero I suppose, except perhaps we've
> just made some OSes undetectably buggy, which would only be a problem if
> in the future someone decided "oh, this must be 0, so we can now extend
> the interface safely".
>
> Ian.
>

Yes - retroactively mandating it to be 0 will 'break' current HVM
guests, which will have whatever value was in %rdx.  For a very long
time this hypercall was mis-documented as a 2-argument hypercall, making
it more likely that something non-0 was in there.

The best that can be safely stated in the comment is that the 3rd
parameter is free for VM use for x86 HVM and ARM guests.  (Unless I
suppose ARM want to restrict this before migration is supported?)

~Andrew