public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: Fix kvm_vcpu_map[_readonly]() function prototypes
@ 2026-03-25  9:15 Peter Fang
  2026-03-31  2:22 ` Yosry Ahmed
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Fang @ 2026-03-25  9:15 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, Madhavan Srinivasan,
	Nicholas Piggin
  Cc: kvm, linuxppc-dev, linux-kernel, Peter Fang, KarimAllah Ahmed,
	Konrad Rzeszutek Wilk

kvm_vcpu_map() and kvm_vcpu_map_readonly() should take a gfn instead of
a gpa. This appears to be a result of the original kvm_vcpu_map() being
declared with the wrong function prototype in kvm_host.h, even though
it was correct in the actual implementation in kvm_main.c.

No actual harm has been done yet as all of the call sites are correctly
passing in a gfn. Plus, both gfn_t and gpa_t are typedef'd to u64 so
this change shouldn't have any functional impact.

Compile-tested on x86 and ppc, which are the current users of these
interfaces.

Fixes: e45adf665a53 ("KVM: Introduce a new guest mapping API")
Cc: KarimAllah Ahmed <karahmed@amazon.de>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Peter Fang <peter.fang@intel.com>
---
 include/linux/kvm_host.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 6b76e7a6f4c2..4e3bea92a06b 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1382,20 +1382,20 @@ void mark_page_dirty_in_slot(struct kvm *kvm, const struct kvm_memory_slot *mems
 void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
 void kvm_vcpu_mark_page_dirty(struct kvm_vcpu *vcpu, gfn_t gfn);
 
-int __kvm_vcpu_map(struct kvm_vcpu *vcpu, gpa_t gpa, struct kvm_host_map *map,
+int __kvm_vcpu_map(struct kvm_vcpu *vcpu, gfn_t gfn, struct kvm_host_map *map,
 		   bool writable);
 void kvm_vcpu_unmap(struct kvm_vcpu *vcpu, struct kvm_host_map *map);
 
-static inline int kvm_vcpu_map(struct kvm_vcpu *vcpu, gpa_t gpa,
+static inline int kvm_vcpu_map(struct kvm_vcpu *vcpu, gfn_t gfn,
 			       struct kvm_host_map *map)
 {
-	return __kvm_vcpu_map(vcpu, gpa, map, true);
+	return __kvm_vcpu_map(vcpu, gfn, map, true);
 }
 
-static inline int kvm_vcpu_map_readonly(struct kvm_vcpu *vcpu, gpa_t gpa,
+static inline int kvm_vcpu_map_readonly(struct kvm_vcpu *vcpu, gfn_t gfn,
 					struct kvm_host_map *map)
 {
-	return __kvm_vcpu_map(vcpu, gpa, map, false);
+	return __kvm_vcpu_map(vcpu, gfn, map, false);
 }
 
 static inline void kvm_vcpu_map_mark_dirty(struct kvm_vcpu *vcpu,

base-commit: d2ea4ff1ce50787a98a3900b3fb1636f3620b7cf
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] KVM: Fix kvm_vcpu_map[_readonly]() function prototypes
  2026-03-25  9:15 [PATCH] KVM: Fix kvm_vcpu_map[_readonly]() function prototypes Peter Fang
@ 2026-03-31  2:22 ` Yosry Ahmed
  2026-04-01  0:13   ` Fang, Peter
  2026-04-04  9:37   ` Ritesh Harjani
  0 siblings, 2 replies; 7+ messages in thread
From: Yosry Ahmed @ 2026-03-31  2:22 UTC (permalink / raw)
  To: Peter Fang
  Cc: Paolo Bonzini, Sean Christopherson, Madhavan Srinivasan,
	Nicholas Piggin, kvm, linuxppc-dev, linux-kernel,
	KarimAllah Ahmed, Konrad Rzeszutek Wilk

On Wed, Mar 25, 2026 at 02:15:11AM -0700, Peter Fang wrote:
> kvm_vcpu_map() and kvm_vcpu_map_readonly() should take a gfn instead of
> a gpa. This appears to be a result of the original kvm_vcpu_map() being
> declared with the wrong function prototype in kvm_host.h, even though
> it was correct in the actual implementation in kvm_main.c.
> 
> No actual harm has been done yet as all of the call sites are correctly
> passing in a gfn. Plus, both gfn_t and gpa_t are typedef'd to u64 so
> this change shouldn't have any functional impact.
> 
> Compile-tested on x86 and ppc, which are the current users of these
> interfaces.
> 
> Fixes: e45adf665a53 ("KVM: Introduce a new guest mapping API")
> Cc: KarimAllah Ahmed <karahmed@amazon.de>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Peter Fang <peter.fang@intel.com>

Most callers are converting a GPA to a GFN, I wonder if we should make
the function take in a GPA instead? But then we'll need to the GPA not
being aligned to a page boundary (either do gpa_to_gfn() in
__kvm_vcpu_map() or fail if it's not aligned).

Not sure if that's a net improvement, mostly thinking out loud here.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KVM: Fix kvm_vcpu_map[_readonly]() function prototypes
  2026-03-31  2:22 ` Yosry Ahmed
@ 2026-04-01  0:13   ` Fang, Peter
  2026-04-02 23:52     ` Sean Christopherson
  2026-04-04  9:37   ` Ritesh Harjani
  1 sibling, 1 reply; 7+ messages in thread
From: Fang, Peter @ 2026-04-01  0:13 UTC (permalink / raw)
  To: Yosry Ahmed
  Cc: Paolo Bonzini, Sean Christopherson, Madhavan Srinivasan,
	Nicholas Piggin, kvm, linuxppc-dev, linux-kernel,
	KarimAllah Ahmed, Konrad Rzeszutek Wilk

On Tue, Mar 31, 2026 at 02:22:47AM +0000, Yosry Ahmed wrote:
> 
> Most callers are converting a GPA to a GFN, I wonder if we should make
> the function take in a GPA instead? But then we'll need to the GPA not
> being aligned to a page boundary (either do gpa_to_gfn() in
> __kvm_vcpu_map() or fail if it's not aligned).

Thanks for the feedback!

Mapping guest memory into the host feels more like a GFN-based operation
to me. struct kvm_host_map is also designed around GFNs/PFNs so I think
using gfn_t in the function prototypes seems more natural. The caller
can handle the offset-in-page cases without creating a lot of complexity
in the APIs. But I'm happy to rework this if there's a desire to make
them more GPA-friendly.

> 
> Not sure if that's a net improvement, mostly thinking out loud here.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KVM: Fix kvm_vcpu_map[_readonly]() function prototypes
  2026-04-01  0:13   ` Fang, Peter
@ 2026-04-02 23:52     ` Sean Christopherson
  2026-04-03 10:18       ` Fang, Peter
  0 siblings, 1 reply; 7+ messages in thread
From: Sean Christopherson @ 2026-04-02 23:52 UTC (permalink / raw)
  To: Peter Fang
  Cc: Yosry Ahmed, Paolo Bonzini, Madhavan Srinivasan, Nicholas Piggin,
	kvm, linuxppc-dev, linux-kernel, KarimAllah Ahmed,
	Konrad Rzeszutek Wilk

On Tue, Mar 31, 2026, Peter Fang wrote:
> On Tue, Mar 31, 2026 at 02:22:47AM +0000, Yosry Ahmed wrote:
> > 
> > Most callers are converting a GPA to a GFN, I wonder if we should make

Not most, all.  The two outliers just do "gpa >> PAGE_SHIFT" instead of
using gpa_to_gfn().

> > the function take in a GPA instead? But then we'll need to the GPA not
> > being aligned to a page boundary (either do gpa_to_gfn() in
> > __kvm_vcpu_map() or fail if it's not aligned).

Just do gpa_to_pfn().  If someone gets confused, we can add a comment explaining
that kvm_vcpu_map() maps the entire page containing the gpa, but that should really
go without saying...

> Thanks for the feedback!
> 
> Mapping guest memory into the host feels more like a GFN-based operation
> to me. struct kvm_host_map is also designed around GFNs/PFNs so I think
> using gfn_t in the function prototypes seems more natural. The caller
> can handle the offset-in-page cases without creating a lot of complexity
> in the APIs. But I'm happy to rework this if there's a desire to make
> them more GPA-friendly.

I vote to rework the APIs (after first fixing the prototypes) to take a GPA.
I agree that mapping a page at a given gfn is conceptually more natural, but
as Yosry points out, requiring literally every caller to convert to a gfn doesn't
make a whole lot of sense from a code maintenance perspective.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KVM: Fix kvm_vcpu_map[_readonly]() function prototypes
  2026-04-02 23:52     ` Sean Christopherson
@ 2026-04-03 10:18       ` Fang, Peter
  0 siblings, 0 replies; 7+ messages in thread
From: Fang, Peter @ 2026-04-03 10:18 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Yosry Ahmed, Paolo Bonzini, Madhavan Srinivasan, Nicholas Piggin,
	kvm, linuxppc-dev, linux-kernel, KarimAllah Ahmed,
	Konrad Rzeszutek Wilk

On Thu, Apr 02, 2026 at 04:52:24PM -0700, Sean Christopherson wrote:
> On Tue, Mar 31, 2026, Peter Fang wrote:
> > On Tue, Mar 31, 2026 at 02:22:47AM +0000, Yosry Ahmed wrote:
> > > 
> > > Most callers are converting a GPA to a GFN, I wonder if we should make
> 
> Not most, all.  The two outliers just do "gpa >> PAGE_SHIFT" instead of
> using gpa_to_gfn().
> 
> > > the function take in a GPA instead? But then we'll need to the GPA not
> > > being aligned to a page boundary (either do gpa_to_gfn() in
> > > __kvm_vcpu_map() or fail if it's not aligned).
> 
> Just do gpa_to_pfn().  If someone gets confused, we can add a comment explaining
> that kvm_vcpu_map() maps the entire page containing the gpa, but that should really
> go without saying...
> 
> > Thanks for the feedback!
> > 
> > Mapping guest memory into the host feels more like a GFN-based operation
> > to me. struct kvm_host_map is also designed around GFNs/PFNs so I think
> > using gfn_t in the function prototypes seems more natural. The caller
> > can handle the offset-in-page cases without creating a lot of complexity
> > in the APIs. But I'm happy to rework this if there's a desire to make
> > them more GPA-friendly.
> 
> I vote to rework the APIs (after first fixing the prototypes) to take a GPA.
> I agree that mapping a page at a given gfn is conceptually more natural, but
> as Yosry points out, requiring literally every caller to convert to a gfn doesn't
> make a whole lot of sense from a code maintenance perspective.

Thanks! I'll prepare a v2 series to clean up the call sites as well.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KVM: Fix kvm_vcpu_map[_readonly]() function prototypes
  2026-03-31  2:22 ` Yosry Ahmed
  2026-04-01  0:13   ` Fang, Peter
@ 2026-04-04  9:37   ` Ritesh Harjani
  2026-04-06 20:19     ` Peter Fang
  1 sibling, 1 reply; 7+ messages in thread
From: Ritesh Harjani @ 2026-04-04  9:37 UTC (permalink / raw)
  To: Yosry Ahmed, Peter Fang
  Cc: Paolo Bonzini, Sean Christopherson, Madhavan Srinivasan,
	Nicholas Piggin, kvm, linuxppc-dev, linux-kernel,
	KarimAllah Ahmed, Konrad Rzeszutek Wilk

Yosry Ahmed <yosry@kernel.org> writes:

> On Wed, Mar 25, 2026 at 02:15:11AM -0700, Peter Fang wrote:
>> kvm_vcpu_map() and kvm_vcpu_map_readonly() should take a gfn instead of
>> a gpa. This appears to be a result of the original kvm_vcpu_map() being
>> declared with the wrong function prototype in kvm_host.h, even though
>> it was correct in the actual implementation in kvm_main.c.
>> 
>> No actual harm has been done yet as all of the call sites are correctly
>> passing in a gfn. Plus, both gfn_t and gpa_t are typedef'd to u64 so
>> this change shouldn't have any functional impact.
>> 
>> Compile-tested on x86 and ppc, which are the current users of these
>> interfaces.

Mostly a mechanical change. I had looked at ppc call site and looks ok
to me.

>> 
>> Fixes: e45adf665a53 ("KVM: Introduce a new guest mapping API")
>> Cc: KarimAllah Ahmed <karahmed@amazon.de>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Signed-off-by: Peter Fang <peter.fang@intel.com>
>
> Most callers are converting a GPA to a GFN, I wonder if we should make
> the function take in a GPA instead? But then we'll need to the GPA not
> being aligned to a page boundary (either do gpa_to_gfn() in
> __kvm_vcpu_map() or fail if it's not aligned).
>
> Not sure if that's a net improvement, mostly thinking out loud here.

The suggestion from Yosry and Sean sounds good too. Instead of every
caller of kvm_vcpu_map() doing gpa_to_gfn(), we may as well make
kvm_vcpu_map() accept gpa_t instead and do the gpa_to_gfn() inside
kvm_vcpu_map() in the call to __kvm_vcpu_map(vcpu, gpa_to_gfn(gpa),...),
or within __kvm_vcpu_map() which is fine too, however, the former is a
better approach, IMO.

-ritesh

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KVM: Fix kvm_vcpu_map[_readonly]() function prototypes
  2026-04-04  9:37   ` Ritesh Harjani
@ 2026-04-06 20:19     ` Peter Fang
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Fang @ 2026-04-06 20:19 UTC (permalink / raw)
  To: Ritesh Harjani
  Cc: Yosry Ahmed, Paolo Bonzini, Sean Christopherson,
	Madhavan Srinivasan, Nicholas Piggin, kvm, linuxppc-dev,
	linux-kernel, KarimAllah Ahmed, Konrad Rzeszutek Wilk

On Sat, Apr 04, 2026 at 03:07:01PM +0530, Ritesh Harjani wrote:
> Yosry Ahmed <yosry@kernel.org> writes:
> 
> > On Wed, Mar 25, 2026 at 02:15:11AM -0700, Peter Fang wrote:
> >> kvm_vcpu_map() and kvm_vcpu_map_readonly() should take a gfn instead of
> >> a gpa. This appears to be a result of the original kvm_vcpu_map() being
> >> declared with the wrong function prototype in kvm_host.h, even though
> >> it was correct in the actual implementation in kvm_main.c.
> >> 
> >> No actual harm has been done yet as all of the call sites are correctly
> >> passing in a gfn. Plus, both gfn_t and gpa_t are typedef'd to u64 so
> >> this change shouldn't have any functional impact.
> >> 
> >> Compile-tested on x86 and ppc, which are the current users of these
> >> interfaces.
> 
> Mostly a mechanical change. I had looked at ppc call site and looks ok
> to me.

Thanks for reviewing the ppc part! v2 with the suggested changes will be
posted soon.

> 
> >> 
> >> Fixes: e45adf665a53 ("KVM: Introduce a new guest mapping API")
> >> Cc: KarimAllah Ahmed <karahmed@amazon.de>
> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >> Signed-off-by: Peter Fang <peter.fang@intel.com>
> >
> > Most callers are converting a GPA to a GFN, I wonder if we should make
> > the function take in a GPA instead? But then we'll need to the GPA not
> > being aligned to a page boundary (either do gpa_to_gfn() in
> > __kvm_vcpu_map() or fail if it's not aligned).
> >
> > Not sure if that's a net improvement, mostly thinking out loud here.
> 
> The suggestion from Yosry and Sean sounds good too. Instead of every
> caller of kvm_vcpu_map() doing gpa_to_gfn(), we may as well make
> kvm_vcpu_map() accept gpa_t instead and do the gpa_to_gfn() inside
> kvm_vcpu_map() in the call to __kvm_vcpu_map(vcpu, gpa_to_gfn(gpa),...),
> or within __kvm_vcpu_map() which is fine too, however, the former is a
> better approach, IMO.
> 
> -ritesh

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-04-06 20:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-25  9:15 [PATCH] KVM: Fix kvm_vcpu_map[_readonly]() function prototypes Peter Fang
2026-03-31  2:22 ` Yosry Ahmed
2026-04-01  0:13   ` Fang, Peter
2026-04-02 23:52     ` Sean Christopherson
2026-04-03 10:18       ` Fang, Peter
2026-04-04  9:37   ` Ritesh Harjani
2026-04-06 20:19     ` Peter Fang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox