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

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?

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