Re: [PATCH v9 20/41] KVM: Let userspace disable per-VM mem attributes, enable per-gmem attributes

On Tue, Aug 04, 2026, Ackerley Tng wrote:
> Xiaoyao Li <[email protected]> writes:
> 
> >
> > [...snip...]
> >
> >> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> >> index ea2752989f8bd..df67a6188aa99 100644
> >> --- a/virt/kvm/guest_memfd.c
> >> +++ b/virt/kvm/guest_memfd.c
> >> @@ -1137,10 +1137,12 @@ static bool kvm_range_is_private(struct file *file, pgoff_t index,
> >>   {
> >>   	struct inode *inode = file_inode(file);
> >>
> >> +#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
> >>   	if (!gmem_in_place_conversion)
> >>   		return kvm_range_has_vm_memory_attributes(kvm, gfn, gfn + nr_pages,
> >>   							  KVM_MEMORY_ATTRIBUTE_PRIVATE,
> >>   							  KVM_MEMORY_ATTRIBUTE_PRIVATE);
> >> +#endif
> >>
> >>   	return kvm_gmem_range_has_attributes(inode, index, nr_pages,
> >>   					     KVM_MEMORY_ATTRIBUTE_PRIVATE);
> >
> > With my suggestion[1] on patch 7, this diff block can be dropped.
> >
> > [1]
> > https://lore.kernel.org/all/[email protected]/
> >
> > The code looks good to me,
> >
> > Reviewed-by: Xiaoyao Li <[email protected]>
> >
> 
> I think your earlier suggestion on only defining the module_param when
> CONFIG_KVM_VM_MEMORY_ATTRIBUTES is selected still applies, since it
> removes an avenue for a user to turn off gmem_in_place_conversion if
> the other option (VM memory attributes) isn't available.
> 
>   +#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
>   +module_param(gmem_in_place_conversion, bool, 0444);
>   +#endif
> 
> Given that we're now dropping the diff block as you suggested, do you
> agree that we should keep this #ifdef?

Yes, the param needs to remain hidden if CONFIG_KVM_VM_MEMORY_ATTRIBUTES=n, so
that userspace attempts to disable gmem_in_place_conversion are ignored (and
logged as such).

The obvious downside is that the param won't exist when in-place conversion is
forced on, which is kinda weird, but I think it's the lesser evil?