| Message ID | 20231219161109.1318-9-paul@xen.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | KVM: xen: update shared_info and vcpu_info handling | expand |
On Tue, Dec 19, 2023 at 04:10:58PM +0000, Paul Durrant wrote: > From: Paul Durrant <pdurrant@amazon.com> > > Some pfncache pages may actually be overlays on guest memory that have a > fixed HVA within the VMM. It's pointless to invalidate such cached > mappings if the overlay is moved so allow a cache to be activated directly > with the HVA to cater for such cases. A subsequent patch will make use > of this facility. > > Signed-off-by: Paul Durrant <pdurrant@amazon.com> > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk> > --- > Cc: Sean Christopherson <seanjc@google.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: David Woodhouse <dwmw2@infradead.org> > > v11: > - Fixed kvm_gpc_check() to ignore memslot generation if the cache is not > activated with a GPA. (This breakage occured during the re-work for v8). > > v9: > - Pass both GPA and HVA into __kvm_gpc_refresh() rather than overloading > the address paraneter and using a bool flag to indicated what it is. > > v8: > - Re-worked to avoid messing with struct gfn_to_pfn_cache. > --- > include/linux/kvm_host.h | 20 +++++++++++++++++++- > virt/kvm/pfncache.c | 40 +++++++++++++++++++++++++++++++--------- > 2 files changed, 50 insertions(+), 10 deletions(-) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 6097f076a7b0..8120674b87b0 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -1345,6 +1345,22 @@ void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm); > */ > int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len); > > +/** > + * kvm_gpc_activate_hva - prepare a cached kernel mapping and HPA for a given HVA. > + * > + * @gpc: struct gfn_to_pfn_cache object. > + * @hva: userspace virtual address to map. > + * @len: sanity check; the range being access must fit a single page. > + * > + * @return: 0 for success. > + * -EINVAL for a mapping which would cross a page boundary. > + * -EFAULT for an untranslatable guest physical address. > + * > + * The semantics of this function are the same as those of kvm_gpc_activate(). It > + * merely bypasses a layer of address translation. > + */ > +int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gpc, unsigned long hva, unsigned long len); > + > /** > * kvm_gpc_check - check validity of a gfn_to_pfn_cache. > * > @@ -1399,7 +1415,9 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc); > static inline void kvm_gpc_mark_dirty(struct gfn_to_pfn_cache *gpc) > { > lockdep_assert_held(&gpc->lock); > - mark_page_dirty_in_slot(gpc->kvm, gpc->memslot, gpc->gpa >> PAGE_SHIFT); > + > + if (gpc->gpa != KVM_XEN_INVALID_GPA) > + mark_page_dirty_in_slot(gpc->kvm, gpc->memslot, gpc->gpa >> PAGE_SHIFT); > } > > void kvm_sigset_activate(struct kvm_vcpu *vcpu); > diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c > index 97eec8ee3449..ae822bff812f 100644 > --- a/virt/kvm/pfncache.c > +++ b/virt/kvm/pfncache.c > @@ -48,7 +48,10 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len) > if (!gpc->active) > return false; > > - if (gpc->generation != slots->generation || kvm_is_error_hva(gpc->uhva)) > + if (gpc->gpa != KVM_XEN_INVALID_GPA && gpc->generation != slots->generation) > + return false; > + > + if (kvm_is_error_hva(gpc->uhva)) > return false; > > if (offset_in_page(gpc->uhva) + len > PAGE_SIZE) > @@ -209,11 +212,13 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc) > return -EFAULT; > } > > -static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, > +static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva, > unsigned long len) > { > struct kvm_memslots *slots = kvm_memslots(gpc->kvm); > - unsigned long page_offset = offset_in_page(gpa); > + unsigned long page_offset = (gpa != KVM_XEN_INVALID_GPA) ? > + offset_in_page(gpa) : > + offset_in_page(uhva); > bool unmap_old = false; > unsigned long old_uhva; > kvm_pfn_t old_pfn; > @@ -246,9 +251,15 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, > old_khva = (void *)PAGE_ALIGN_DOWN((uintptr_t)gpc->khva); > old_uhva = PAGE_ALIGN_DOWN(gpc->uhva); > > - /* Refresh the userspace HVA if necessary */ > - if (gpc->gpa != gpa || gpc->generation != slots->generation || > - kvm_is_error_hva(gpc->uhva)) { > + if (gpa == KVM_XEN_INVALID_GPA) { > + gpc->gpa = KVM_XEN_INVALID_GPA; > + gpc->uhva = PAGE_ALIGN_DOWN(uhva); > + > + if (gpc->uhva != old_uhva) > + hva_change = true; > + } else if (gpc->gpa != gpa || > + gpc->generation != slots->generation || > + kvm_is_error_hva(gpc->uhva)) { > gfn_t gfn = gpa_to_gfn(gpa); > > gpc->gpa = gpa; > @@ -319,7 +330,7 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, > > int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len) > { > - return __kvm_gpc_refresh(gpc, gpc->gpa, len); > + return __kvm_gpc_refresh(gpc, gpc->gpa, gpc->uhva, len); > } > > void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm) > @@ -332,7 +343,8 @@ void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm) > gpc->uhva = KVM_HVA_ERR_BAD; > } > > -int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len) > +static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva, > + unsigned long len) > { > struct kvm *kvm = gpc->kvm; > > @@ -353,7 +365,17 @@ int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len) > gpc->active = true; > write_unlock_irq(&gpc->lock); > } > - return __kvm_gpc_refresh(gpc, gpa, len); > + return __kvm_gpc_refresh(gpc, gpa, uhva, len); > +} > + > +int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len) > +{ > + return __kvm_gpc_activate(gpc, gpa, KVM_HVA_ERR_BAD, len); > +} > + > +int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gpc, unsigned long uhva, unsigned long len) > +{ > + return __kvm_gpc_activate(gpc, KVM_XEN_INVALID_GPA, uhva, len); > } > > void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc) The code looks good to me, but I feel odd that a *gfn*_to_pfn_cache is used, but gfn is not taken into account. I think if it is possible we introduce an hva_to_pfn_cache(hpc) that actually does most of the job in this file. Xen could directly use hpc, and gfn_to_pfn_cache works on top of hpc. BTW: I also see there is a gfn_to_hva_cache which does pretty much the same as gpc's first half job. Is it possible to unify them like: struct gfn_to_pfn_cache { struct gfn_to_hva_cache ghc; struct hva_to_pfn_cache hpc; ... } Just my two cents. Thanks, Yilun > -- > 2.39.2 > >
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 6097f076a7b0..8120674b87b0 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -1345,6 +1345,22 @@ void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm); */ int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len); +/** + * kvm_gpc_activate_hva - prepare a cached kernel mapping and HPA for a given HVA. + * + * @gpc: struct gfn_to_pfn_cache object. + * @hva: userspace virtual address to map. + * @len: sanity check; the range being access must fit a single page. + * + * @return: 0 for success. + * -EINVAL for a mapping which would cross a page boundary. + * -EFAULT for an untranslatable guest physical address. + * + * The semantics of this function are the same as those of kvm_gpc_activate(). It + * merely bypasses a layer of address translation. + */ +int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gpc, unsigned long hva, unsigned long len); + /** * kvm_gpc_check - check validity of a gfn_to_pfn_cache. * @@ -1399,7 +1415,9 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc); static inline void kvm_gpc_mark_dirty(struct gfn_to_pfn_cache *gpc) { lockdep_assert_held(&gpc->lock); - mark_page_dirty_in_slot(gpc->kvm, gpc->memslot, gpc->gpa >> PAGE_SHIFT); + + if (gpc->gpa != KVM_XEN_INVALID_GPA) + mark_page_dirty_in_slot(gpc->kvm, gpc->memslot, gpc->gpa >> PAGE_SHIFT); } void kvm_sigset_activate(struct kvm_vcpu *vcpu); diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c index 97eec8ee3449..ae822bff812f 100644 --- a/virt/kvm/pfncache.c +++ b/virt/kvm/pfncache.c @@ -48,7 +48,10 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len) if (!gpc->active) return false; - if (gpc->generation != slots->generation || kvm_is_error_hva(gpc->uhva)) + if (gpc->gpa != KVM_XEN_INVALID_GPA && gpc->generation != slots->generation) + return false; + + if (kvm_is_error_hva(gpc->uhva)) return false; if (offset_in_page(gpc->uhva) + len > PAGE_SIZE) @@ -209,11 +212,13 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc) return -EFAULT; } -static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, +static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva, unsigned long len) { struct kvm_memslots *slots = kvm_memslots(gpc->kvm); - unsigned long page_offset = offset_in_page(gpa); + unsigned long page_offset = (gpa != KVM_XEN_INVALID_GPA) ? + offset_in_page(gpa) : + offset_in_page(uhva); bool unmap_old = false; unsigned long old_uhva; kvm_pfn_t old_pfn; @@ -246,9 +251,15 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, old_khva = (void *)PAGE_ALIGN_DOWN((uintptr_t)gpc->khva); old_uhva = PAGE_ALIGN_DOWN(gpc->uhva); - /* Refresh the userspace HVA if necessary */ - if (gpc->gpa != gpa || gpc->generation != slots->generation || - kvm_is_error_hva(gpc->uhva)) { + if (gpa == KVM_XEN_INVALID_GPA) { + gpc->gpa = KVM_XEN_INVALID_GPA; + gpc->uhva = PAGE_ALIGN_DOWN(uhva); + + if (gpc->uhva != old_uhva) + hva_change = true; + } else if (gpc->gpa != gpa || + gpc->generation != slots->generation || + kvm_is_error_hva(gpc->uhva)) { gfn_t gfn = gpa_to_gfn(gpa); gpc->gpa = gpa; @@ -319,7 +330,7 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len) { - return __kvm_gpc_refresh(gpc, gpc->gpa, len); + return __kvm_gpc_refresh(gpc, gpc->gpa, gpc->uhva, len); } void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm) @@ -332,7 +343,8 @@ void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm) gpc->uhva = KVM_HVA_ERR_BAD; } -int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len) +static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva, + unsigned long len) { struct kvm *kvm = gpc->kvm; @@ -353,7 +365,17 @@ int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len) gpc->active = true; write_unlock_irq(&gpc->lock); } - return __kvm_gpc_refresh(gpc, gpa, len); + return __kvm_gpc_refresh(gpc, gpa, uhva, len); +} + +int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len) +{ + return __kvm_gpc_activate(gpc, gpa, KVM_HVA_ERR_BAD, len); +} + +int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gpc, unsigned long uhva, unsigned long len) +{ + return __kvm_gpc_activate(gpc, KVM_XEN_INVALID_GPA, uhva, len); } void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc)