Re: [PATCH v1 0/2] KVM: x86/xen: Runstate cleanups on top of kvm/queue

[Paolo Bonzini <pbonzini@redhat.com>]

On 11/30/22 20:51, David Woodhouse wrote:
> On Wed, 2022-11-30 at 17:03 +0100, Paolo Bonzini wrote:
>> On 11/27/22 13:22, David Woodhouse wrote:
>>> Clean the update code up a little bit by unifying the fast and slow
>>> paths as discussed, and make the update flag conditional to avoid
>>> confusing older guests that don't ask for it.
>>>
>>> On top of kvm/queue as of today at commit da5f28e10aa7d.
>>>
>>> (This is identical to what I sent a couple of minutes ago, except that
>>> this time I didn't forget to Cc the list)
>>>
>>>
>>
>> Merged, thanks.
> 
> Thanks. I've rebased the remaining GPC fixes on top and pushed them out
> (along with Metin's SCHEDOP_poll 32-bit compat support) to
> 
> https://git.infradead.org/users/dwmw2/linux.git/shortlog/refs/heads/gpc-fixes

Oh, so we do pull requests now too?  I'm all for it, but please use 
signed tags. :)

> I still haven't reinstated the last of those patches to make gpc->len
> immutable, although I think we concluded it's fine just to make the
> runstate code claim gpc->len=1 and manage its own destiny, right?

Yeah, I'm not super keen on that either, but I guess it can work with 
any of len == 1 or len == PAGE_SIZE - offset.

Related to this, for 6.3 I will send a cleanup of the API to put 
together lock and check.

Paolo

> In reviewing the merge/squash I spotted a minor cosmetic nit in my
> 'Allow XEN_RUNSTATE_UPDATE flag behaviour to be configured' commit.
> It'd be slightly prettier like this, although the compiler really ought
> to emit identical code.
> 
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 5208e05ca9a6..76b7fc6d543a 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -382,7 +382,7 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
>   	entry_time = vx->runstate_entry_time;
>   	if (update_bit) {
>   		entry_time |= XEN_RUNSTATE_UPDATE;
> -		*update_bit = (vx->runstate_entry_time | XEN_RUNSTATE_UPDATE) >> 56;
> +		*update_bit = entry_time >> 56;
>   		smp_wmb();
>   	}
>   
> 
>