[PATCH v2 0/3] KVM: Fix and clean up kvm_vcpu_map[_readonly]() usages

Peter Fang posted 3 patches 2 months, 1 week ago
arch/powerpc/kvm/book3s_pr.c |  2 +-
arch/x86/kvm/svm/nested.c    |  4 ++--
arch/x86/kvm/svm/sev.c       |  2 +-
arch/x86/kvm/svm/svm.c       |  8 +++----
arch/x86/kvm/vmx/nested.c    | 11 ++++-----
include/linux/kvm_host.h     | 46 ++++++++++++++++++------------------
6 files changed, 36 insertions(+), 37 deletions(-)
[PATCH v2 0/3] KVM: Fix and clean up kvm_vcpu_map[_readonly]() usages
Posted by Peter Fang 2 months, 1 week ago
kvm_vcpu_map() and kvm_vcpu_map_readonly() are declared to take a gpa_t
in kvm_host.h when they're supposed to take a gfn_t. First fix the
function prototypes, and then refactor them to correctly take a gpa_t,
reducing boilerplate gpa->gfn conversions at all call sites.

No actual harm has been done yet as all of the call sites are correctly
passing in a gfn.

No functional change intended. All changes are compile-tested on x86 and
ppc, which are the current users of these APIs.

---

v1 -> v2:
  - Rebased on top of latest kvm.git#master
  - As suggested by Yosry, refactor the APIs to reduce boilerplate code
    at call sites

v1: https://lore.kernel.org/kvm/20260325092001.613025-1-peter.fang@intel.com/

Peter Fang (3):
  KVM: Fix kvm_vcpu_map[_readonly]() function prototypes
  KVM: Move page mapping/unmapping APIs in kvm_host.h
  KVM: Take gpa_t in kvm_vcpu_map[_readonly]()

 arch/powerpc/kvm/book3s_pr.c |  2 +-
 arch/x86/kvm/svm/nested.c    |  4 ++--
 arch/x86/kvm/svm/sev.c       |  2 +-
 arch/x86/kvm/svm/svm.c       |  8 +++----
 arch/x86/kvm/vmx/nested.c    | 11 ++++-----
 include/linux/kvm_host.h     | 46 ++++++++++++++++++------------------
 6 files changed, 36 insertions(+), 37 deletions(-)


base-commit: df83746075778958954aa0460cca55f4b3fc9c02
-- 
2.53.0
Re: [PATCH v2 0/3] KVM: Fix and clean up kvm_vcpu_map[_readonly]() usages
Posted by Sean Christopherson 3 weeks, 6 days ago
On Tue, 07 Apr 2026 17:11:27 -0700, Peter Fang wrote:
> kvm_vcpu_map() and kvm_vcpu_map_readonly() are declared to take a gpa_t
> in kvm_host.h when they're supposed to take a gfn_t. First fix the
> function prototypes, and then refactor them to correctly take a gpa_t,
> reducing boilerplate gpa->gfn conversions at all call sites.
> 
> No actual harm has been done yet as all of the call sites are correctly
> passing in a gfn.
> 
> [...]

Applied patch 1 to kvm-x86 generic.  I'm moderately optimistic that the gpc
stuff will land soon enough that I won't regret skipping 2 and 3 :-)

Thanks much!

[1/3] KVM: Fix kvm_vcpu_map[_readonly]() function prototypes
      https://github.com/kvm-x86/linux/commit/ccd6c77223bb

--
https://github.com/kvm-x86/linux/tree/next
Re: [PATCH v2 0/3] KVM: Fix and clean up kvm_vcpu_map[_readonly]() usages
Posted by David Woodhouse 1 month, 3 weeks ago
On Tue, 2026-04-07 at 17:11 -0700, Peter Fang wrote:
> kvm_vcpu_map() and kvm_vcpu_map_readonly() are declared to take a gpa_t
> in kvm_host.h when they're supposed to take a gfn_t. First fix the
> function prototypes, and then refactor them to correctly take a gpa_t,
> reducing boilerplate gpa->gfn conversions at all call sites.
> 
> No actual harm has been done yet as all of the call sites are correctly
> passing in a gfn.
> 
> No functional change intended. All changes are compile-tested on x86 and
> ppc, which are the current users of these APIs.

Fred is already removing all the usage of kvm_vcpu_map() in nested VMX¹
and nested SVM probably wants the same treatment. And the PowerPC one
looks like it could just as easily operate on the userspace address? 

Could we just kill kvm_vcpu_map() completely?



¹ https://lore.kernel.org/kvm/20260102142429.896101-1-griffoul@gmail.com/
Re: [PATCH v2 0/3] KVM: Fix and clean up kvm_vcpu_map[_readonly]() usages
Posted by Peter Fang 1 month, 2 weeks ago
On Fri, Apr 24, 2026 at 11:27:03AM +0100, David Woodhouse wrote:
> 
> Fred is already removing all the usage of kvm_vcpu_map() in nested VMX¹
> and nested SVM probably wants the same treatment. And the PowerPC one
> looks like it could just as easily operate on the userspace address? 
> 
> Could we just kill kvm_vcpu_map() completely?

Thanks David!

I think I'd need at least input from the maintainers on this but just by
code inspection, the kvm_vcpu_map() usage in sev.c seems a bit tricky.
Unmapping doesn't happen until right before switching to the guest, so
this might fall into the "keep the mapping around for a longer time"
category [1].

[1] https://lore.kernel.org/kvm/20211115165030.7422-8-dwmw2@infradead.org/

> 
> 
> 
> ¹ https://lore.kernel.org/kvm/20260102142429.896101-1-griffoul@gmail.com/


Re: [PATCH v2 0/3] KVM: Fix and clean up kvm_vcpu_map[_readonly]() usages
Posted by Sean Christopherson 1 month, 1 week ago
On Mon, Apr 27, 2026, Peter Fang wrote:
> On Fri, Apr 24, 2026 at 11:27:03AM +0100, David Woodhouse wrote:
> > 
> > Fred is already removing all the usage of kvm_vcpu_map() in nested VMX¹
> > and nested SVM probably wants the same treatment. And the PowerPC one
> > looks like it could just as easily operate on the userspace address? 
> > 
> > Could we just kill kvm_vcpu_map() completely?

Yeah, that's probably for the best in the long run.

> Thanks David!
> 
> I think I'd need at least input from the maintainers on this but just by
> code inspection, the kvm_vcpu_map() usage in sev.c seems a bit tricky.
> Unmapping doesn't happen until right before switching to the guest, so
> this might fall into the "keep the mapping around for a longer time"
> category [1].

It definitely falls into that category.  But that code is also rather gross, i.e.
could use some cleanup no matter what, so I don't think it's a good argument for
keeping kvm_vcpu_map() around.

To avoid a bunch of pointless work and churn, let's hold off on hardening and/or
renaming kvm_vcpu_map() for now.  I'll take this v2 as-is; even though taking a
gpa instead of a gfn will conflict with the nVMX series, it's dead simple and a
worthwhile cleanup even if some of the conversions get discarded shortly after.
Re: [PATCH v2 0/3] KVM: Fix and clean up kvm_vcpu_map[_readonly]() usages
Posted by Peter Fang 1 month, 1 week ago
On Mon, May 04, 2026 at 10:59:06AM -0700, Sean Christopherson wrote:
> On Mon, Apr 27, 2026, Peter Fang wrote:
> 
> > Thanks David!
> > 
> > I think I'd need at least input from the maintainers on this but just by
> > code inspection, the kvm_vcpu_map() usage in sev.c seems a bit tricky.
> > Unmapping doesn't happen until right before switching to the guest, so
> > this might fall into the "keep the mapping around for a longer time"
> > category [1].
> 
> It definitely falls into that category.  But that code is also rather gross, i.e.
> could use some cleanup no matter what, so I don't think it's a good argument for
> keeping kvm_vcpu_map() around.
> 
> To avoid a bunch of pointless work and churn, let's hold off on hardening and/or
> renaming kvm_vcpu_map() for now.  I'll take this v2 as-is; even though taking a
> gpa instead of a gfn will conflict with the nVMX series, it's dead simple and a
> worthwhile cleanup even if some of the conversions get discarded shortly after.

Makes sense to me. Thanks for the review Sean!
Re: [PATCH v2 0/3] KVM: Fix and clean up kvm_vcpu_map[_readonly]() usages
Posted by Sean Christopherson 1 month ago
On Thu, May 07, 2026, Peter Fang wrote:
> On Mon, May 04, 2026 at 10:59:06AM -0700, Sean Christopherson wrote:
> > On Mon, Apr 27, 2026, Peter Fang wrote:
> > 
> > > Thanks David!
> > > 
> > > I think I'd need at least input from the maintainers on this but just by
> > > code inspection, the kvm_vcpu_map() usage in sev.c seems a bit tricky.
> > > Unmapping doesn't happen until right before switching to the guest, so
> > > this might fall into the "keep the mapping around for a longer time"
> > > category [1].
> > 
> > It definitely falls into that category.  But that code is also rather gross, i.e.
> > could use some cleanup no matter what, so I don't think it's a good argument for
> > keeping kvm_vcpu_map() around.
> > 
> > To avoid a bunch of pointless work and churn, let's hold off on hardening and/or
> > renaming kvm_vcpu_map() for now.  I'll take this v2 as-is; even though taking a
> > gpa instead of a gfn will conflict with the nVMX series, it's dead simple and a
> > worthwhile cleanup even if some of the conversions get discarded shortly after.

I had a change of heart after looking at the applied code, and after going through
Fred's gpc+nVMX series.  I don't want to have a discrepancy between kvm_vcpu_map()
and __kvm_vcpu_map(), even for a "short" amount of time, and I do think it makes
sense to pursue switching to gpcs for the nested code.  But, I also agree with the
changelog's statement that __kvm_vcpu_map() fundamentally operates on gfns, i.e. I
don't want to "fix" the discrepancy.

The other thing that swayed me is patch 2; I have a separate patch (amusingly
related to gpc stuff) to extra gpa_to_gfn (and others) into kvm_types.h, and so
I don't want to take patch 2 either.

Long story short, I'm going to grab only patch 1.