Re: [PATCH 30/60] kvm: Add KVM_EXIT_PLANE_EVENT

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing capability negotiation for the new exit reason KVM_EXIT_PLANE_EVENT.
- [Low] Architecture-specific parameter (`apic_id`) used in generic KVM code.
--

commit 7ac5546369ac2dcb5fc9dd9195a781cf261e7620
Author: Paolo Bonzini <pbonzini@redhat.com>

Add a new exit-type to KVM for telling user-space that a plane-vcpu is missing.

This commit adds KVM_EXIT_PLANE_EVENT and a helper kvm_request_create_plane()
to populate the kvm_run exit structure.

> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 3ecd472c7cfa..90b97137840e 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1649,6 +1649,8 @@ void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu);
>  void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu);
>  bool kvm_vcpu_wake_up(struct kvm_vcpu *vcpu);
>  
> +int kvm_request_create_plane(struct kvm_vcpu *vcpu, unsigned plane, u64 apic_id);

[Severity: Low]
Does this introduce an architecture-specific parameter into generic KVM code?

The apic_id parameter is an x86-specific concept, but this declaration is
exposed in the generic kvm_host.h API. Would it be better to use an
architecture-neutral term like vcpu_id or hw_id?

[ ... ]

> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 24e34b8e4819..a88d987c7882 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
[ ... ]
> @@ -193,6 +200,7 @@ struct kvm_exit_snp_req_certs {
>  #define KVM_EXIT_ARM_SEA          41
>  #define KVM_EXIT_ARM_LDST64B      42
>  #define KVM_EXIT_SNP_REQ_CERTS    43
> +#define KVM_EXIT_PLANE_EVENT      44

[Severity: High]
Should there be a capability negotiation flag added for this new exit reason?

If a host kernel begins emitting KVM_EXIT_PLANE_EVENT without prior
capability negotiation (e.g., via a KVM_CAP_PLANE_EVENT definition), it
could cause older, unaware userspace processes to crash when they
encounter the unknown exit reason.

[ ... ]

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 03a44ff62f0f..f0f78bb74e51 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
[ ... ]
> @@ -3973,6 +3973,18 @@ bool kvm_vcpu_wake_up(struct kvm_vcpu *vcpu)
>  }
>  EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_vcpu_wake_up);
>  
> +int kvm_request_create_plane(struct kvm_vcpu *vcpu, unsigned plane, u64 apic_id)
> +{
> +	vcpu->run->exit_reason = KVM_EXIT_PLANE_EVENT;

[Severity: Low]
Can this function signature be updated to use architecture-neutral terminology?

This generic helper implementation uses apic_id, matching the header 
declaration. Using a neutral parameter name here might better maintain 
subsystem layering.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608144252.351443-1-joro@8bytes.org?part=30