Re: [RFC PATCH 1/1] xen: Use a global mapping for runstate

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.05.2020 11:18, Bertrand Marquis wrote:
> Hi Jan,
> 
>> On 29 May 2020, at 09:45, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 29.05.2020 10:13, Bertrand Marquis wrote:
>>>> On 28 May 2020, at 19:54, Julien Grall <julien@xxxxxxx> wrote:
>>>> AFAICT, there is no restriction on when the runstate hypercall can be 
>>>> called. So this can even be called before the vCPU is brought up.
>>>
>>> I understand the remark but it still feels very weird to allow an invalid 
>>> address in an hypercall.
>>> Wouldn’t we have a lot of potential issues accepting an address that we 
>>> cannot check ?
>>
>> I don't think so: The hypervisor uses copy_to_guest() to protect
>> itself from the addresses to be invalid at the time of copying.
>> If the guest doesn't make sure they're valid at that time, it
>> simply won't get the information (perhaps until Xen's next
>> attempt to copy it out).
>>
>> You may want to take a look at the x86 side of this (also the
>> vCPU time updating): Due to the way x86-64 PV guests work, the
>> address may legitimately be unmapped at the time Xen wants to
>> copy it, when the vCPU is currently executing guest user mode
>> code. In such a case the copy gets retried the next time the
>> guest transitions from user to kernel mode (which involves a
>> page table change).
> 
> If I understand everything correctly runstate is updated only if there is
> a context switch in xen while the guest is running in kernel mode and
> if the address is mapped at that time.
> 
> So this is a best effort in Xen and the guest cannot really rely on the
> runstate information (as it might not be up to date).
> Could this have impacts somehow if this is used for scheduling ?

Yes, it could, and hence it's not really best effort only, but
said retry upon guest mode switch had been added years ago.
(This was one of the things overlooked when x86-64 support was
introduced, as x86-32 doesn't have this same problem.) The
updating is best-effort only as far as a misbehaving guest goes;
in all other aspects it's reliable, assuming that vCPU's only
look at their own data for the purpose of making decisions
(logging and alike are of course still fine, as long as people
are aware of the data possibly being stale).

> In the end the only accepted trade off would be to:
> - reduce error verbosity and just ignore it
> - introduce a new system call using a physical address
>   -> Using a virtual address with restrictions sounds very complex
>   to document (current core, no remapping).
> 
> But it feels like having only one hypercall using guest physical addresses
> would not really be logic and this kind of change should be made across
> all hypercalls if it is done.

Hence my request to preferably first settle on a model, such
that the change here could be simply the first step on that
journey.

Jan