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);@@ -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)) {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