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

Sean Christopherson
2026-04-21T23:29:07Z

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.