Re: [PATCH RFC v2 45/50] samples/kvm: Implement attach and release provider ops in sample provider

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] samples/kvm: build failure when CONFIG_KVM_GUEST_MEMFD is disabled
--

commit da89517936ff62d4ed67bf323c9cade71c875336
Author: Ackerley Tng <ackerleytng@google.com>

samples/kvm: Implement attach and release provider ops in sample provider

This commit implements the .attach and .release guest_memfd provider
operations in the sample memory provider module and wires them up to
mps_file_fops.gmem_provider_ops.

> diff --git a/samples/kvm/mem_provider_sample.c b/samples/kvm/mem_provider_sample.c
> index 397ebb3773f39..d9e57484c3933 100644
> --- a/samples/kvm/mem_provider_sample.c
> +++ b/samples/kvm/mem_provider_sample.c

[ ... ]

> @@ -118,11 +119,30 @@ static long mps_file_ioctl(struct file *file, unsigned int cmd,
>  	}
>  }
>  
> +static void *mps_gmem_attach(struct inode *gmem_inode,
> +			     struct file *resource_file)
> +{
> +	return get_file(resource_file);
> +}
> +
> +static void mps_gmem_release(void *provider)
> +{
> +	struct file *resource_file = provider;
> +
> +	fput(resource_file);
> +}
> +
> +static const struct guest_memfd_provider_operations mps_gmem_provider_ops = {
> +	.attach		= mps_gmem_attach,
> +	.release	= mps_gmem_release,
> +};
> +
>  static const struct file_operations mps_file_fops = {
>  	.owner		= THIS_MODULE,
>  	.release	= mps_file_release,
>  	.unlocked_ioctl	= mps_file_ioctl,
>  	.compat_ioctl	= compat_ptr_ioctl,
> +	.gmem_provider_ops = &mps_gmem_provider_ops,
>  };

[Severity: Low]
Does initializing .gmem_provider_ops in mps_file_fops in
samples/kvm/mem_provider_sample.c cause a build regression when
CONFIG_SAMPLE_KVM_MEM_PROVIDER=m is enabled while CONFIG_KVM_GUEST_MEMFD is
disabled?

In include/linux/fs.h, gmem_provider_ops in struct file_operations is guarded
by CONFIG_KVM_GUEST_MEMFD:

    #ifdef CONFIG_KVM_GUEST_MEMFD
        const struct guest_memfd_provider_operations *gmem_provider_ops;
    #endif

However, at this point in the series, SAMPLE_KVM_MEM_PROVIDER in
samples/Kconfig only depends on CONTIG_ALLOC && X86_64 && m:

    config SAMPLE_KVM_MEM_PROVIDER
        tristate "Build sample memory provider -- loadable module only"
        depends on CONTIG_ALLOC && X86_64 && m

A later commit in this series, 279ad9d61dfc ("samples/kvm: Support dynamic
revocation via kvm_gmem_revoke"), adds && KVM_GUEST_MEMFD to
SAMPLE_KVM_MEM_PROVIDER in samples/Kconfig. Should that Kconfig dependency be
moved to this commit to avoid breaking bisection when CONFIG_KVM_GUEST_MEMFD
is not enabled?

>  
>  static long mps_ctl_new_file(void __user *uarg)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-gmem-alternative-providers-v2-0-5dd1dc6c660a@google.com?part=45