Re: [PATCH v2 3/3] KVM: Take gpa_t in kvm_vcpu_map[_readonly]()

Yosry Ahmed
2026-04-21T23:41:44Z

On Tue, Apr 21, 2026 at 4:29 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Apr 21, 2026, Sean Christopherson wrote:
> > On Tue, Apr 21, 2026, Yosry Ahmed wrote:
> > > On Tue, Apr 07, 2026 at 05:11:30PM -0700, Peter Fang wrote:
> > > > Move the conversion from a gpa_t to a gfn_t into kvm_vcpu_map() and
> > > > kvm_vcpu_map_readonly() so that they take a gpa_t directly, reducing
> > > > boilerplate at call sites.
> > > >
> > > > __kvm_vcpu_map() still takes a gfn_t because guest page mapping is
> > > > fundamentally GFN-based.
> > > >
> > > > No functional change intended.
> > > >
> > > > Compile-tested on x86 and ppc, which are the current users of these
> > > > interfaces.
> > > >
> > > > Suggested-by: Yosry Ahmed <yosry@kernel.org>
> > > > Signed-off-by: Peter Fang <peter.fang@intel.com>
> > > > ---
> > >
> > > I was going to suggest a WARN in kvm_vcpu_map() and
> > > kvm_vcpu_map_readonly() if the passed GPA is not page-aligned, but Sean
> > > usually hates my paranoid WARN suggestions.
> >
> > Heh, for good reason. Adding such a WARN would be triggered by this code:
> >
> > if (!kvm_vcpu_map(vcpu, vmcs12->posted_intr_desc_addr, map)) {
> > vmx->nested.pi_desc =
> > (struct pi_desc *)(((void *)map->hva) +
> > offset_in_page(vmcs12->posted_intr_desc_addr));
> >
> > The PI descriptor only needs to be 64-bit aligned, not page-aligned.
>
> To elaborate a bit, I'm all for adding WARNs in flows where something bad is all
> but guaranteed to happen if an assumption is violated, or in APIs where there's
> a history of goofs and/or subtlety in how the API behaves.
>
> What I'm against is adding WARNs because someone could write bad code in the
> future, or because KVM doesn't do XYZ at this time. Such WARNs usualy just add
> noise, and can even be actively harmful. E.g. in this case, ignoring the PID
> usage, a reader might look at the WARN and think it's _wrong_ to map a page in
> order to access a subset of the page, which is just not true.

Yeah I agree with most/all of your objections to my suggestions, it's
usually that I don't have enough context to understand how the WARN
could be harmful (like here), or am just being too paranoid or
defending against bad code as you mentioned. I was mentioning your
objections semi-sarcastically and intentionally bringing up the WARN
in case it's actually useful.

Taking a step back, what I really want to clarify and/or detect misuse
of, is that kvm_vcpu_map() will map exactly one page, the one that the
GPA lies in. For example, there's nothing protecting against the PID
address being the last byte of the page, in which case accessing all
of it would be wrong as it spans the mapped page boundary. This is
difficult to hit if you are passing in a GFN, as it's more obvious
that KVM is mapping one physical page.

Perhaps we just need to rename the functions (e.g.
kvm_vcpu_map_page()), or more intrusively pass in a size and do bounds
checking.