On Mon, Apr 20, 2026 at 1:06 PM Sean Christopherson <[email protected]> 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!
