Re: [PATCH v6 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM

On 2026/08/04 06:21 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Mismatch between KVM_CAP_PPC_COMPAT_CAPS advertisement and ioctl implementation for unsupported environments.
> --
> 
> --- Patch [2]: [PATCH v6 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM ---
> commit 8305652489744988576b7fa55e0424452c48a633
> Author: Amit Machhiwal <[email protected]>
> 
> KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM
>     
> This commit adds support for retrieving host CPU compatibility capabilities
> for nested guests on PowerVM. The capability bitmap is retrieved from the
> cached nested_capabilities value and exposed through the new
> KVM_PPC_GET_COMPAT_CAPS ioctl.
> 
> > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > index 61dbeea317f3b..8799124b80960 100644
> > --- a/arch/powerpc/kvm/book3s_hv.c
> > +++ b/arch/powerpc/kvm/book3s_hv.c
> 
> [ ... ]
> 
> > @@ -6510,6 +6510,25 @@ static bool kvmppc_hash_v3_possible(void)
> >  	return true;
> >  }
> >  
> > +
> > +static int kvmppc_get_compat_caps(struct kvm_ppc_compat_caps *host_caps)
> > +{
> > +	unsigned long capabilities = 0;
> > +	long rc = -EINVAL;
> > +
> > +	if (kvmhv_on_pseries()) {
> > +		if (kvmhv_is_nestedv2()) {
> > +			WARN_ON_ONCE(!nested_capabilities);
> > +			capabilities = nested_capabilities;
> > +			rc = 0;
> > +		}
> > +	}
> > +
> > +	host_caps->compat_capabilities = capabilities & KVM_PPC_COMPAT_BITMASK;
> > +
> > +	return rc;
> > +}
> 
> [Severity: Medium]
> Will this return -EINVAL for pseries hosts when nestedv1 is used?

This is analysing patch 2 in isolation. The series is applied in order:
patch 3 adds the nestedv1 (KVM on PowerNV) path to
kvmppc_get_compat_caps(), filling in the `else` branch for the case
where kvmhv_is_nestedv2() is false. After the full series is applied, a
pseries nestedv1 host takes the DT-based path added in patch 3, not the
-EINVAL path. There is no issue in the complete series.

> 
> If KVM_CAP_PPC_COMPAT_CAPS advertises support by returning 1 for all pseries
> hosts (which appears to be the case in kvm_vm_ioctl_check_extension), 
> userspace might query KVM_PPC_GET_COMPAT_CAPS and hit this path when
> kvmhv_is_nestedv2() is false.
> 
> For example, if userspace checks KVM_CAP_PPC_COMPAT_CAPS and gets a positive
> result, but kvmhv_is_nestedv2() returns false here, we return -EINVAL.
> 
> Since -EINVAL typically indicates invalid structure size or bad flags, could
> userspace fail to differentiate an unsupported backend from invalid inputs,
> leading to initialization failures? 
> 
> Should this gracefully return 0 capabilities (or -ENOTTY), or should the
> capability check itself verify kvmhv_is_nestedv2() before advertising support?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2