| Message ID | 20221127122210.248427-1-dwmw2@infradead.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: x86/xen: Runstate cleanups on top of kvm/queue | expand |
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. Paolo
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 Metin Kaya (1): KVM: x86/xen: add support for 32-bit guests in SCHEDOP_poll Michal Luczaj (4): KVM: Store immutable gfn_to_pfn_cache properties KVM: Use gfn_to_pfn_cache's immutable "kvm" in kvm_gpc_check() KVM: Clean up hva_to_pfn_retry() KVM: Use gfn_to_pfn_cache's immutable "kvm" in kvm_gpc_refresh() Sean Christopherson (4): KVM: Drop KVM's API to allow temporarily unmapping gfn=>pfn cache KVM: Do not partially reinitialize gfn=>pfn cache during activation KVM: Drop @gpa from exported gfn=>pfn cache check() and refresh() helpers KVM: Skip unnecessary "unmap" if gpc is already valid during refresh 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? 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(); }
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(); > } > > >
On Fri, Dec 02, 2022, Paolo Bonzini wrote: > 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. We ended up with multiple threads on this topic. Maybe pick up the conversation here? https://lore.kernel.org/all/Y4ovbTiLQ2Jy0em9@google.com
On Fri, 2022-12-02 at 20:00 +0100, Paolo Bonzini wrote: > > > 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. :) Not really intended as a pull request (unless you really want one). I've just kept it rebased on top of the other parts as they've evolved, so we always have the latest state in a known location. Didn't seem worth the noise of actually *resending* them repeatedly. > > 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. I suspect now's the time to actually resend those patches on top of the current kvm/queue, and we can work out the length and lock/check things on top?