On 4/1/25 11:10, Paolo Bonzini wrote:
> Add the file_operations for planes, the means to create new file
> descriptors for them, and the KVM_CHECK_EXTENSION implementation for
> the two new capabilities.
>
> KVM_SIGNAL_MSI and KVM_SET_MEMORY_ATTRIBUTES are now available
> through both vm and plane file descriptors, forward them to the
> same function that is used by the file_operations for planes.
> KVM_CHECK_EXTENSION instead remains separate, because it only
> advertises a very small subset of capabilities when applied to
> plane file descriptors.
>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
> include/linux/kvm_host.h | 19 +++++
> include/uapi/linux/kvm.h | 2 +
> virt/kvm/kvm_main.c | 154 +++++++++++++++++++++++++++++++++------
> 3 files changed, 154 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 0a91b556767e..dbca418d64f5 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -342,6 +342,8 @@ struct kvm_vcpu {
> unsigned long guest_debug;
>
> struct mutex mutex;
> +
> + /* Shared for all planes */
> struct kvm_run *run;
>
> #ifndef __KVM_HAVE_ARCH_WQP
> @@ -922,6 +924,23 @@ static inline void kvm_vm_bugged(struct kvm *kvm)
> }
>
>
> +#if KVM_MAX_VCPU_PLANES == 1
> +static inline int kvm_arch_nr_vcpu_planes(struct kvm *kvm)
> +{
> + return KVM_MAX_VCPU_PLANES;
> +}
Should this be outside of the #if above?
> +
> +static inline struct kvm_plane *vcpu_to_plane(struct kvm_vcpu *vcpu)
> +{
> + return vcpu->kvm->planes[0];
> +}
> +#else
> +static inline struct kvm_plane *vcpu_to_plane(struct kvm_vcpu *vcpu)
> +{
> + return vcpu->kvm->planes[vcpu->plane_id];
> +}
> +#endif
Are two different functions needed? The vcpu->plane_id will be zero in
the KVM_MAX_VCPU_PLANES == 1 case, so that should get the same result as
the hard-coded 0, right?
> +
> @@ -5236,16 +5363,12 @@ static long kvm_vm_ioctl(struct file *filp,
> break;
> }
> #ifdef CONFIG_HAVE_KVM_MSI
> - case KVM_SIGNAL_MSI: {
> - struct kvm_msi msi;
> -
> - r = -EFAULT;
> - if (copy_from_user(&msi, argp, sizeof(msi)))
> - goto out;
> - r = kvm_send_userspace_msi(kvm->planes[0], &msi);
> - break;
> - }
> + case KVM_SIGNAL_MSI:
> #endif
> +#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
> + case KVM_SET_MEMORY_ATTRIBUTES:
> +#endif /* CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES */
If both of these aren't #defined, then you'll end up with just the
return statement from the next line, which will cause a build failure.
Thanks,
Tom
> + return __kvm_plane_ioctl(kvm->planes[0], ioctl, arg);
> #ifdef __KVM_HAVE_IRQ_LINE
> case KVM_IRQ_LINE_STATUS:
> case KVM_IRQ_LINE: {
> @@ -5301,18 +5424,6 @@ static long kvm_vm_ioctl(struct file *filp,
> break;
> }
> #endif /* CONFIG_HAVE_KVM_IRQ_ROUTING */
> -#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
> - case KVM_SET_MEMORY_ATTRIBUTES: {
> - struct kvm_memory_attributes attrs;
> -
> - r = -EFAULT;
> - if (copy_from_user(&attrs, argp, sizeof(attrs)))
> - goto out;
> -
> - r = kvm_vm_ioctl_set_mem_attributes(kvm->planes[0], &attrs);
> - break;
> - }
> -#endif /* CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES */
> case KVM_CREATE_DEVICE: {
> struct kvm_create_device cd;
>
> @@ -6467,6 +6578,7 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module)
> kvm_chardev_ops.owner = module;
> kvm_vm_fops.owner = module;
> kvm_vcpu_fops.owner = module;
> + kvm_plane_fops.owner = module;
> kvm_device_fops.owner = module;
>
> kvm_preempt_ops.sched_in = kvm_sched_in;
