Messages in this thread | | | Date | Tue, 31 Oct 2023 16:37:06 -0700 | Subject | Re: [PATCH v7 03/11] KVM: pfncache: add a helper to get the gpa | From | Sean Christopherson <> |
| |
On Mon, Oct 02, 2023, Paul Durrant wrote: > From: Paul Durrant <pdurrant@amazon.com> > > A subsequent patch will rename this field since it will become overloaded.
Too. Many. Pronouns.
Add a helper to get the gpa of a gpc cache, as a subsequent patch will rename "gpa" to "addr".
> To avoid churn in places that currently retrieve the gpa, add a helper for > that purpose now.
This is silly. If this series added any protection against incorrect usage then I could understand the helper, but this just end up being
return gpc->addr_is_gpa ? gpc->addr : INVALID_GPA;
which is nasty. IIUC, there's no WARN because kvm_xen_vcpu_get_attr() doesn't pre-check that the cache is in the correct mode. That's a really silly reason to not harden the rest of KVM.
> diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c > index b68ed7fa56a2..17afbb464a70 100644 > --- a/virt/kvm/pfncache.c > +++ b/virt/kvm/pfncache.c > @@ -386,6 +386,12 @@ int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len) > } > EXPORT_SYMBOL_GPL(kvm_gpc_activate); > > +gpa_t kvm_gpc_gpa(struct gfn_to_pfn_cache *gpc) > +{ > + return gpc->gpa; > +} > +EXPORT_SYMBOL_GPL(kvm_gpc_gpa);
Any reason not to make this static inline? Even in the final form, not making this inlined seems silly.
Belatedly, same question for kvm_gpc_mark_dirty() I suppose.
> void kvm_gpc_mark_dirty(struct gfn_to_pfn_cache *gpc) > { > mark_page_dirty_in_slot(gpc->kvm, gpc->memslot, gpc->gpa >> PAGE_SHIFT); > -- > 2.39.2 >
| |