[PATCH v2 01/10] KVM: selftests: Use gva_t instead of vm_vaddr_t

[David Matlack]

On Mon, Apr 20, 2026 at 1:06 PM Sean Christopherson <seanjc at google.com> wrote:
>
> On Fri, Feb 20, 2026, David Matlack wrote:
> > Replace all occurrences of vm_vaddr_t with gva_t to align with KVM code
> > and with the conversion helpers (e.g. addr_gva2hva()). Also replace
> > vm_vaddr in function names with gva to align with the new type name.
>
> ...
>
> > @@ -716,22 +716,22 @@ void vm_mem_region_move(struct kvm_vm *vm, uint32_t slot, uint64_t new_gpa);
> >  void vm_mem_region_delete(struct kvm_vm *vm, uint32_t slot);
> >  struct kvm_vcpu *__vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id);
> >  void vm_populate_vaddr_bitmap(struct kvm_vm *vm);
> > -vm_vaddr_t vm_vaddr_unused_gap(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min);
> > -vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min);
> > -vm_vaddr_t __vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min,
> > -                         enum kvm_mem_region_type type);
> > -vm_vaddr_t vm_vaddr_alloc_shared(struct kvm_vm *vm, size_t sz,
> > -                              vm_vaddr_t vaddr_min,
> > -                              enum kvm_mem_region_type type);
> > -vm_vaddr_t vm_vaddr_alloc_pages(struct kvm_vm *vm, int nr_pages);
> > -vm_vaddr_t __vm_vaddr_alloc_page(struct kvm_vm *vm,
> > -                              enum kvm_mem_region_type type);
> > -vm_vaddr_t vm_vaddr_alloc_page(struct kvm_vm *vm);
> > +gva_t gva_unused_gap(struct kvm_vm *vm, size_t sz, gva_t vaddr_min);
> > +gva_t gva_alloc(struct kvm_vm *vm, size_t sz, gva_t vaddr_min);
> > +gva_t __gva_alloc(struct kvm_vm *vm, size_t sz, gva_t vaddr_min,
> > +               enum kvm_mem_region_type type);
> > +gva_t gva_alloc_shared(struct kvm_vm *vm, size_t sz,
> > +                    gva_t vaddr_min,
> > +                    enum kvm_mem_region_type type);
> > +gva_t gva_alloc_pages(struct kvm_vm *vm, int nr_pages);
> > +gva_t __gva_alloc_page(struct kvm_vm *vm,
> > +                    enum kvm_mem_region_type type);
> > +gva_t gva_alloc_page(struct kvm_vm *vm);
>
> The existing vm_vaddr_alloc() and friends are pretty bad names.  gva_alloc() is
> far, far worse.  The APIs aren't just allocation a guest virtual address, they're
> allocating guest physical memory, finding a usable virtual address, and creating
> mappings.
>
> I don't see any reason to have vaddr or gva in the name.  E.g. malloc() isn't
> virt_malloc().  But I do think they need to be explicitly scoped to KVM, and to
> a VM.  I'll drop API renames from this patch, and rename them to vm_alloc() and
> friends in a separate patch.  Amusingly, that naming scheme will still work if
> "vm" is misconstrued as "virtual memory" instead of "virtual machine".

Sounds good to me.

> P.S. This is a great example of why I insist on one logical change per patch, with
> judicious exemptions for opportunistic cleanups/changes.  If this has been a
> separate patch, it would have taken me all of two seconds to unwind.  As it was,
> I spent a good 10-15 minutes dealing with this.  In large part because I kept
> making goofs, but that's the whole point: there was no reason to put me in a
> position to make goofs.
>
> The other argument against these sorts of "Also do xyz" add-ons is that of a
> slippery slope.  Why rename these APIs in this patch, but not the myriad vaddr
> variables?  Then after a few "I'll just clean this up too" changes, there's an
> entire series in what is purportedly just a typedef rename.

You're right, the API rename should have been split off into its own patch.
Thanks for the guidance!