| Message ID | 20230918144111.641369-1-paul@xen.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: xen: update shared_info and vcpu_info handling | expand |
On Mon, 2023-09-18 at 14:41 +0000, Paul Durrant wrote: > From: Paul Durrant <pdurrant@amazon.com> > > The VMM should only need to set the KVM_XEN_VCPU_ATTR_TYPE_VCPU_INFO > attribute in response to a VCPUOP_register_vcpu_info hypercall. We can > handle the default case internally since we already know where the > shared_info page is. Modify get_vcpu_info_cache() to pass back a pointer > to the shared info pfn cache (and appropriate offset) for any of the > first 32 vCPUs if the attribute has not been set. > > A VMM will be able to determine whether it needs to set up default > vcpu_info using the previously defined KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA > Xen capability flag, which will be advertized in a subsequent patch. > > Also update the KVM API documentation to describe the new behaviour. > > Signed-off-by: Paul Durrant <pdurrant@amazon.com> > --- > Cc: Sean Christopherson <seanjc@google.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: David Woodhouse <dwmw2@infradead.org> > Cc: x86@kernel.org > > v3: > - Add a note to the API documentation discussing vcpu_info copying. > > v2: > - Dispense with the KVM_XEN_HVM_CONFIG_DEFAULT_VCPU_INFO capability. > - Add API documentation. > --- > Documentation/virt/kvm/api.rst | 22 +++++++++++++++------- > arch/x86/kvm/xen.c | 15 +++++++++++++++ > 2 files changed, 30 insertions(+), 7 deletions(-) > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index e9df4df6fe48..47bf3db74674 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -5442,13 +5442,7 @@ KVM_XEN_ATTR_TYPE_LONG_MODE > > KVM_XEN_ATTR_TYPE_SHARED_INFO > Sets the guest physical frame number at which the Xen shared_info > - page resides. Note that although Xen places vcpu_info for the first > - 32 vCPUs in the shared_info page, KVM does not automatically do so > - and instead requires that KVM_XEN_VCPU_ATTR_TYPE_VCPU_INFO be used > - explicitly even when the vcpu_info for a given vCPU resides at the > - "default" location in the shared_info page. This is because KVM may > - not be aware of the Xen CPU id which is used as the index into the > - vcpu_info[] array, so may know the correct default location. > + page resides. > > Note that the shared_info page may be constantly written to by KVM; > it contains the event channel bitmap used to deliver interrupts to > @@ -5564,12 +5558,26 @@ type values: > > KVM_XEN_VCPU_ATTR_TYPE_VCPU_INFO > Sets the guest physical address of the vcpu_info for a given vCPU. > + The vcpu_info for the first 32 vCPUs defaults to the structures > + embedded in the shared_info page. The above is true only if KVM has KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA. You kind of touch on that next, but perhaps the 'if the KVM_...' condition should be moved up? > + If the KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA flag is also set in the > + Xen capabilities then the VMM is not required to set this default > + location; KVM will handle that internally. Otherwise this attribute > + must be set for all vCPUs. > + > As with the shared_info page for the VM, the corresponding page may be > dirtied at any time if event channel interrupt delivery is enabled, so > userspace should always assume that the page is dirty without relying > on dirty logging. Setting the gpa to KVM_XEN_INVALID_GPA will disable > the vcpu_info. > > + Note that, if the guest sets an explicit vcpu_info location in guest > + memory then the VMM is expected to copy the content of the structure > + embedded in the shared_info page to the new location. It is therefore > + important that no event delivery is in progress at this time, otherwise > + events may be missed. > That's difficult. It means tearing down all interrupts from passthrough devices which are mapped via PIRQs, and also all IPIs. The IPI code *should* be able to fall back to just letting the VMM handle the hypercall in userspace. But PIRQs are harder. I'd be happier if our plan — handwavy though it may be — led to being able to use the existing slow path for delivering interrupts by just *invalidating* the cache. Maybe we *should* move the memcpy into the kernel, and let it lock *both* the shinfo and new vcpu_info caches while it's doing the copy? Given that that's the only valid transition, that shouldn't be so hard, should it? > KVM_XEN_VCPU_ATTR_TYPE_VCPU_TIME_INFO > Sets the guest physical address of an additional pvclock structure > for a given vCPU. This is typically used for guest vsyscall support. > diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c > index 459f3ca4710e..660a808c0b50 100644 > --- a/arch/x86/kvm/xen.c > +++ b/arch/x86/kvm/xen.c > @@ -491,6 +491,21 @@ static void kvm_xen_inject_vcpu_vector(struct kvm_vcpu *v) > > static struct gfn_to_pfn_cache *get_vcpu_info_cache(struct kvm_vcpu *v, unsigned long *offset) > { > + if (!v->arch.xen.vcpu_info_cache.active && v->arch.xen.vcpu_id < MAX_VIRT_CPUS) { > + struct kvm *kvm = v->kvm; > + > + if (offset) { > + if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) > + *offset = offsetof(struct shared_info, > + vcpu_info[v->arch.xen.vcpu_id]); > + else > + *offset = offsetof(struct compat_shared_info, > + vcpu_info[v->arch.xen.vcpu_id]); > + } > + > + return &kvm->arch.xen.shinfo_cache; > + } > + > if (offset) > *offset = 0; >
On 18/09/2023 17:07, David Woodhouse wrote: > On Mon, 2023-09-18 at 14:41 +0000, Paul Durrant wrote: >> From: Paul Durrant <pdurrant@amazon.com> >> >> The VMM should only need to set the KVM_XEN_VCPU_ATTR_TYPE_VCPU_INFO >> attribute in response to a VCPUOP_register_vcpu_info hypercall. We can >> handle the default case internally since we already know where the >> shared_info page is. Modify get_vcpu_info_cache() to pass back a pointer >> to the shared info pfn cache (and appropriate offset) for any of the >> first 32 vCPUs if the attribute has not been set. >> >> A VMM will be able to determine whether it needs to set up default >> vcpu_info using the previously defined KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA >> Xen capability flag, which will be advertized in a subsequent patch. >> >> Also update the KVM API documentation to describe the new behaviour. >> >> Signed-off-by: Paul Durrant <pdurrant@amazon.com> >> --- >> Cc: Sean Christopherson <seanjc@google.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Borislav Petkov <bp@alien8.de> >> Cc: Dave Hansen <dave.hansen@linux.intel.com> >> Cc: "H. Peter Anvin" <hpa@zytor.com> >> Cc: David Woodhouse <dwmw2@infradead.org> >> Cc: x86@kernel.org >> >> v3: >> - Add a note to the API documentation discussing vcpu_info copying. >> >> v2: >> - Dispense with the KVM_XEN_HVM_CONFIG_DEFAULT_VCPU_INFO capability. >> - Add API documentation. >> --- >> Documentation/virt/kvm/api.rst | 22 +++++++++++++++------- >> arch/x86/kvm/xen.c | 15 +++++++++++++++ >> 2 files changed, 30 insertions(+), 7 deletions(-) >> >> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst >> index e9df4df6fe48..47bf3db74674 100644 >> --- a/Documentation/virt/kvm/api.rst >> +++ b/Documentation/virt/kvm/api.rst >> @@ -5442,13 +5442,7 @@ KVM_XEN_ATTR_TYPE_LONG_MODE >> >> KVM_XEN_ATTR_TYPE_SHARED_INFO >> Sets the guest physical frame number at which the Xen shared_info >> - page resides. Note that although Xen places vcpu_info for the first >> - 32 vCPUs in the shared_info page, KVM does not automatically do so >> - and instead requires that KVM_XEN_VCPU_ATTR_TYPE_VCPU_INFO be used >> - explicitly even when the vcpu_info for a given vCPU resides at the >> - "default" location in the shared_info page. This is because KVM may >> - not be aware of the Xen CPU id which is used as the index into the >> - vcpu_info[] array, so may know the correct default location. >> + page resides. >> >> Note that the shared_info page may be constantly written to by KVM; >> it contains the event channel bitmap used to deliver interrupts to >> @@ -5564,12 +5558,26 @@ type values: >> >> KVM_XEN_VCPU_ATTR_TYPE_VCPU_INFO >> Sets the guest physical address of the vcpu_info for a given vCPU. >> + The vcpu_info for the first 32 vCPUs defaults to the structures >> + embedded in the shared_info page. > > The above is true only if KVM has KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA. > You kind of touch on that next, but perhaps the 'if the KVM_...' > condition should be moved up? > >> + If the KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA flag is also set in the >> + Xen capabilities then the VMM is not required to set this default >> + location; KVM will handle that internally. Otherwise this attribute >> + must be set for all vCPUs. >> + >> As with the shared_info page for the VM, the corresponding page may be >> dirtied at any time if event channel interrupt delivery is enabled, so >> userspace should always assume that the page is dirty without relying >> on dirty logging. Setting the gpa to KVM_XEN_INVALID_GPA will disable >> the vcpu_info. >> >> + Note that, if the guest sets an explicit vcpu_info location in guest >> + memory then the VMM is expected to copy the content of the structure >> + embedded in the shared_info page to the new location. It is therefore >> + important that no event delivery is in progress at this time, otherwise >> + events may be missed. >> > > That's difficult. It means tearing down all interrupts from passthrough > devices which are mapped via PIRQs, and also all IPIs. So those don't honour event channel masking? That seems like a problem. > > The IPI code *should* be able to fall back to just letting the VMM > handle the hypercall in userspace. But PIRQs are harder. I'd be happier > if our plan — handwavy though it may be — led to being able to use the > existing slow path for delivering interrupts by just *invalidating* the > cache. Maybe we *should* move the memcpy into the kernel, and let it > lock *both* the shinfo and new vcpu_info caches while it's doing the > copy? Given that that's the only valid transition, that shouldn't be so > hard, should it? > No, it just kind of oversteps the remit of the attribute... but I'll try adding it and see how messy it gets. Paul >> KVM_XEN_VCPU_ATTR_TYPE_VCPU_TIME_INFO >> Sets the guest physical address of an additional pvclock structure >> for a given vCPU. This is typically used for guest vsyscall support. >> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c >> index 459f3ca4710e..660a808c0b50 100644 >> --- a/arch/x86/kvm/xen.c >> +++ b/arch/x86/kvm/xen.c >> @@ -491,6 +491,21 @@ static void kvm_xen_inject_vcpu_vector(struct kvm_vcpu *v) >> >> static struct gfn_to_pfn_cache *get_vcpu_info_cache(struct kvm_vcpu *v, unsigned long *offset) >> { >> + if (!v->arch.xen.vcpu_info_cache.active && v->arch.xen.vcpu_id < MAX_VIRT_CPUS) { >> + struct kvm *kvm = v->kvm; >> + >> + if (offset) { >> + if (IS_ENABLED(CONFIG_64BIT) && kvm->arch.xen.long_mode) >> + *offset = offsetof(struct shared_info, >> + vcpu_info[v->arch.xen.vcpu_id]); >> + else >> + *offset = offsetof(struct compat_shared_info, >> + vcpu_info[v->arch.xen.vcpu_id]); >> + } >> + >> + return &kvm->arch.xen.shinfo_cache; >> + } >> + >> if (offset) >> *offset = 0; >> >
On Mon, Sep 18, 2023, Paul Durrant wrote: > From: Paul Durrant <pdurrant@amazon.com> > > Currently we treat the shared_info page as guest memory and the VMM informs > KVM of its location using a GFN. However it is not guest memory as such; > it's an overlay page. So we pointlessly invalidate and re-cache a mapping > to the *same page* of memory every time the guest requests that shared_info > be mapped into its address space. Let's avoid doing that by modifying the > pfncache code to allow activation using a fixed userspace HVA as well as > a GPA. > > Also, if the guest does not hypercall to explicitly set a pointer to a > vcpu_info in its own memory, the default vcpu_info embedded in the > shared_info page should be used. At the moment the VMM has to set up a > pointer to the structure explicitly (again treating it like it's in > guest memory, despite being in an overlay page). Let's also avoid the > need for that. We already have a cached mapping for the shared_info > page so just use that directly by default. 1. Please Cc me on *all* patches if you Cc me on one patch. I belive this is the preference of the vast majority of maintainers/reviewers/contributors. Having to go spelunking to find the rest of a series is annoying. 2. Wait a reasonable amount of time between posting versions. 1 hour is not reasonable. At an *absolute minimum*, wait 1 business day. 3. In the cover letter, summarize what's changed between versions. Lack of a summary exacerbates the problems from #1 and #2, e.g. I have a big pile of mails scattered across my mailboxes, and I am effectively forced to find and read them all if I want to have any clue as to why I have a 12 patch series on version 3 in less than two business days. P.S. I very much appreciate that y'all are doing review publicly, thank you!
On Mon, 2023-09-18 at 17:15 +0100, Paul Durrant wrote: > > > + Note that, if the guest sets an explicit vcpu_info location in guest > > > + memory then the VMM is expected to copy the content of the structure > > > + embedded in the shared_info page to the new location. It is therefore > > > + important that no event delivery is in progress at this time, otherwise > > > + events may be missed. > > > > > > > That's difficult. It means tearing down all interrupts from passthrough > > devices which are mapped via PIRQs, and also all IPIs. > > So those don't honour event channel masking? That seems like a problem. Oh, *mask*. Sure, it does honour masking. But... that would mean the VMM has to keep track of which ports were *really* masked by the guest, and which ones were just masked for the switchover. Including if the guest does some mask/unmask activity *while* the switchover is happening (or locking to prevent such). I still don't think that's a kind thing to be telling the VMMs they need to do. > > > > The IPI code *should* be able to fall back to just letting the VMM > > handle the hypercall in userspace. But PIRQs are harder. I'd be happier > > if our plan — handwavy though it may be — led to being able to use the > > existing slow path for delivering interrupts by just *invalidating* the > > cache. Maybe we *should* move the memcpy into the kernel, and let it > > lock *both* the shinfo and new vcpu_info caches while it's doing the > > copy? Given that that's the only valid transition, that shouldn't be so > > hard, should it? > > > > No, it just kind of oversteps the remit of the attribute... but I'll try > adding it and see how messy it gets. Well, there's a reason I left all the vcpu_info address magic in userspace in the first place. It was there in João's original patches and I ripped it all out. But I see your logic for wanting to put it back; I suspect moving the memcpy too is part of the cost of that? Should work out OK, I think.
On Mon, 2023-09-18 at 09:18 -0700, Sean Christopherson wrote: > On Mon, Sep 18, 2023, Paul Durrant wrote: > > From: Paul Durrant <pdurrant@amazon.com> > > > > Currently we treat the shared_info page as guest memory and the VMM informs > > KVM of its location using a GFN. However it is not guest memory as such; > > it's an overlay page. So we pointlessly invalidate and re-cache a mapping > > to the *same page* of memory every time the guest requests that shared_info > > be mapped into its address space. Let's avoid doing that by modifying the > > pfncache code to allow activation using a fixed userspace HVA as well as > > a GPA. > > > > Also, if the guest does not hypercall to explicitly set a pointer to a > > vcpu_info in its own memory, the default vcpu_info embedded in the > > shared_info page should be used. At the moment the VMM has to set up a > > pointer to the structure explicitly (again treating it like it's in > > guest memory, despite being in an overlay page). Let's also avoid the > > need for that. We already have a cached mapping for the shared_info > > page so just use that directly by default. > > 1. Please Cc me on *all* patches if you Cc me on one patch. I belive this is > the preference of the vast majority of maintainers/reviewers/contributors. > Having to go spelunking to find the rest of a series is annoying. > > 2. Wait a reasonable amount of time between posting versions. 1 hour is not > reasonable. At an *absolute minimum*, wait 1 business day. > > 3. In the cover letter, summarize what's changed between versions. Lack of a > summary exacerbates the problems from #1 and #2, e.g. I have a big pile of > mails scattered across my mailboxes, and I am effectively forced to find and > read them all if I want to have any clue as to why I have a 12 patch series > on version 3 in less than two business days. I meant to mention that too. > P.S. I very much appreciate that y'all are doing review publicly, thank you! Well, to a certain extent you can't have both #2 and the P.S. Or at least doesn't work very well if we try. Paul and I were literally sitting in the same room last Friday talking about this, and of course you saw the very first iteration of it on the mailing list rather than it being done in private and then presented as a fait accompli. We try to set that example for those around us. (Just as you saw the very first attempt at the exit-on-hlt thing, and the lore.kernel.org link was what I gave to my engineers to tell them to see what happens if they try that.) And there *are* going to be a couple of rounds of rapid review and adjustment as we start from scratch in the open, as I firmly believe that we should. I *want* to do it in public and I *want* you to be able to see it, but I don't necessarily think it works for us to *wait* for you. Maybe it makes more sense for you to dive deep into the details only after the rapid fire round has finished? Unless you don't *want* those first rounds to be in public? But I don't think that's the right approach. Suggestions welcome. Maybe in this particular case given that I said something along the lines of "knock something up and let's see how I feel about it when I see it", it should be using an 'RFC' tag until I actually approve it? Not sure how to extrapolate that to the general case, mind you.
On Mon, Sep 18, 2023, David Woodhouse wrote: > On Mon, 2023-09-18 at 09:18 -0700, Sean Christopherson wrote: > > On Mon, Sep 18, 2023, Paul Durrant wrote: > > > From: Paul Durrant <pdurrant@amazon.com> > > > > > > Currently we treat the shared_info page as guest memory and the VMM informs > > > KVM of its location using a GFN. However it is not guest memory as such; > > > it's an overlay page. So we pointlessly invalidate and re-cache a mapping > > > to the *same page* of memory every time the guest requests that shared_info > > > be mapped into its address space. Let's avoid doing that by modifying the > > > pfncache code to allow activation using a fixed userspace HVA as well as > > > a GPA. > > > > > > Also, if the guest does not hypercall to explicitly set a pointer to a > > > vcpu_info in its own memory, the default vcpu_info embedded in the > > > shared_info page should be used. At the moment the VMM has to set up a > > > pointer to the structure explicitly (again treating it like it's in > > > guest memory, despite being in an overlay page). Let's also avoid the > > > need for that. We already have a cached mapping for the shared_info > > > page so just use that directly by default. > > > > 1. Please Cc me on *all* patches if you Cc me on one patch. I belive this is > > the preference of the vast majority of maintainers/reviewers/contributors. > > Having to go spelunking to find the rest of a series is annoying. > > > > 2. Wait a reasonable amount of time between posting versions. 1 hour is not > > reasonable. At an *absolute minimum*, wait 1 business day. > > > > 3. In the cover letter, summarize what's changed between versions. Lack of a > > summary exacerbates the problems from #1 and #2, e.g. I have a big pile of > > mails scattered across my mailboxes, and I am effectively forced to find and > > read them all if I want to have any clue as to why I have a 12 patch series > > on version 3 in less than two business days. > > I meant to mention that too. > > > P.S. I very much appreciate that y'all are doing review publicly, thank you! > > Well, to a certain extent you can't have both #2 and the P.S. Or at > least doesn't work very well if we try. > > Paul and I were literally sitting in the same room last Friday talking > about this, and of course you saw the very first iteration of it on the > mailing list rather than it being done in private and then presented as > a fait accompli. We try to set that example for those around us. > > (Just as you saw the very first attempt at the exit-on-hlt thing, and > the lore.kernel.org link was what I gave to my engineers to tell them > to see what happens if they try that.) > > And there *are* going to be a couple of rounds of rapid review and > adjustment as we start from scratch in the open, as I firmly believe > that we should. I *want* to do it in public and I *want* you to be able > to see it, but I don't necessarily think it works for us to *wait* for > you. Maybe it makes more sense for you to dive deep into the details > only after the rapid fire round has finished? > > Unless you don't *want* those first rounds to be in public? But I don't > think that's the right approach. > > Suggestions welcome. > > Maybe in this particular case given that I said something along the > lines of "knock something up and let's see how I feel about it when I > see it", it should be using an 'RFC' tag until I actually approve it? > Not sure how to extrapolate that to the general case, mind you. Tag them RFC, explain your expectations, goals, and intent in the cover letter, don't copy+paste cover letters verbatim between versions, and summarize the RFC(s) when you get to a point where you're ready for others to jump in. The cover letter is *identical* from v1=>v2=>v3, how is anyone supposed to understand what on earth is going on unless they happened to be in the same room as ya'll on Friday? Doing rapid-fire, early review on beta-quality patches is totally fine, so long as it's clear that others can effectively ignore the early versions unless they are deeply interested or whatever. A disclaimer at the top of the cover letter, e.g. This series is a first attempt at an idea David had to improve performance of the pfncache when KVM is emulating Xen. David and I are still working out the details, it's probably not necessary for other folks to review this right now. along with a summary of previous version and a transition from RFC => non-RFC makes it clear when I and others are expected to get involved. In other words, use tags and the cover letter to communicate, don't just view the cover letter as a necessary evil to get people to care about your patches.
On 18/09/2023 18:12, Sean Christopherson wrote: [snip] > > Tag them RFC, explain your expectations, goals, and intent in the cover letter, > don't copy+paste cover letters verbatim between versions, and summarize the RFC(s) > when you get to a point where you're ready for others to jump in. The cover > letter is *identical* from v1=>v2=>v3, how is anyone supposed to understand what > on earth is going on unless they happened to be in the same room as ya'll on > Friday? The cover letter is indeed identical because the purpose of the series has not changed. Each individual patch has a commit comment summarizing what changed from version to version or whether it is new in a perticular version. I thought this would be enough for any reviewer to be able to see what is going on. In future I will also roll these up into the cover letter. > > Doing rapid-fire, early review on beta-quality patches is totally fine, so long > as it's clear that others can effectively ignore the early versions unless they > are deeply interested or whatever. > > A disclaimer at the top of the cover letter, e.g. > > This series is a first attempt at an idea David had to improve performance > of the pfncache when KVM is emulating Xen. David and I are still working out > the details, it's probably not necessary for other folks to review this right > now. > > along with a summary of previous version and a transition from RFC => non-RFC > makes it clear when I and others are expected to get involved. > > In other words, use tags and the cover letter to communicate, don't just view the > cover letter as a necessary evil to get people to care about your patches. That was not the intention at all; I put all the detailed explanation in the commit comments because I thought that would make review *easier*. Paul
On Tue, Sep 19, 2023, Paul Durrant wrote: > On 18/09/2023 18:12, Sean Christopherson wrote: > [snip] > > > > Tag them RFC, explain your expectations, goals, and intent in the cover letter, > > don't copy+paste cover letters verbatim between versions, and summarize the RFC(s) > > when you get to a point where you're ready for others to jump in. The cover > > letter is *identical* from v1=>v2=>v3, how is anyone supposed to understand what > > on earth is going on unless they happened to be in the same room as ya'll on > > Friday? > > The cover letter is indeed identical because the purpose of the series has > not changed. For anything out of the ordinary, e.g. posting v3 just a few hours after v2 is definitely not normal, use the cover letter to call out why you're posting a particular version of the series, not just the purpose of the series. > > In other words, use tags and the cover letter to communicate, don't just view the > > cover letter as a necessary evil to get people to care about your patches. > > That was not the intention at all; I put all the detailed explanation in the > commit comments because I thought that would make review *easier*. Per-patch comments *might* make individual patches easier to review, but (for me at least) they are waaay less helpful for reviewing series as a whole, and all but usless for initial triage. E.g. for a situation like this where a series has reached v4 before I've so much as glanced at the patches, having the history in the cover letter allows me to catch up and get a feel for how the series got to v4 in ~20 seconds. With per-patch comments, I have to go find each comment and then piece together the bigger picture. Per-patch comments also don't work well if a version makes minor changes to a large series (hunting through a 10+ patch series to figure out that only one patch changed is not exactly efficient), if a patch is dropped, if there are changes to the overall approach, etc.
From: Paul Durrant <pdurrant@amazon.com> Currently we treat the shared_info page as guest memory and the VMM informs KVM of its location using a GFN. However it is not guest memory as such; it's an overlay page. So we pointlessly invalidate and re-cache a mapping to the *same page* of memory every time the guest requests that shared_info be mapped into its address space. Let's avoid doing that by modifying the pfncache code to allow activation using a fixed userspace HVA as well as a GPA. Also, if the guest does not hypercall to explicitly set a pointer to a vcpu_info in its own memory, the default vcpu_info embedded in the shared_info page should be used. At the moment the VMM has to set up a pointer to the structure explicitly (again treating it like it's in guest memory, despite being in an overlay page). Let's also avoid the need for that. We already have a cached mapping for the shared_info page so just use that directly by default. Paul Durrant (13): KVM: pfncache: add a map helper function KVM: pfncache: add a mark-dirty helper KVM: pfncache: add a helper to get the gpa KVM: pfncache: base offset check on khva rather than gpa KVM: pfncache: allow a cache to be activated with a fixed (userspace) HVA KVM: xen: allow shared_info to be mapped by fixed HVA KVM: xen: prepare for using 'default' vcpu_info KVM: xen: prevent vcpu_id from changing whilst shared_info is valid KVM: xen: automatically use the vcpu_info embedded in shared_info KVM: selftests / xen: set KVM_XEN_VCPU_ATTR_TYPE_VCPU_ID KVM: selftests / xen: map shared_info using HVA rather than GFN KVM: selftests / xen: don't explicitly set the vcpu_info address KVM: xen: advertize the KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA capability Documentation/virt/kvm/api.rst | 49 ++++-- arch/x86/include/asm/kvm_host.h | 4 + arch/x86/kvm/x86.c | 17 +-- arch/x86/kvm/xen.c | 139 +++++++++++++----- arch/x86/kvm/xen.h | 6 +- include/linux/kvm_host.h | 43 ++++++ include/linux/kvm_types.h | 3 +- include/uapi/linux/kvm.h | 6 +- .../selftests/kvm/x86_64/xen_shinfo_test.c | 75 ++++++++-- virt/kvm/pfncache.c | 129 +++++++++++----- 10 files changed, 360 insertions(+), 111 deletions(-) --- Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Borislav Petkov <bp@alien8.de> Cc: Dave Hansen <dave.hansen@linux.intel.com> Cc: David Woodhouse <dwmw2@infradead.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Sean Christopherson <seanjc@google.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: x86@kernel.org