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

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] KVM_CAP_PPC_COMPAT_CAPS is unconditionally advertised on all pseries hosts, but the corresponding ioctl fails on non-nestedv2 hosts.
- [Medium] kvmppc_get_compat_caps returns -EINVAL for unsupported configurations instead of -ENOTTY or -EOPNOTSUPP.
--

--- Patch [2]: [PATCH v8 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM ---
commit 73749763b436fd33b42f2ac5536c1c5a4bf826ad
Author: Amit Machhiwal <[email protected]>

KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM

This commit implements the retrieval of host CPU compatibility capabilities
for nested KVM guests on PowerVM. It defines KVM-specific capability constants
and exposes the cached nested capabilities via the 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;

[Severity: Medium]
Should this return -ENOTTY or -EOPNOTSUPP instead of -EINVAL when the
configuration is unsupported?

-EINVAL conventionally indicates invalid userspace arguments rather than a
lack of host support. This might complicate graceful fallback logic if
userspace cannot differentiate between malformed requests and missing features.

> +
> +	if (kvmhv_on_pseries()) {
> +		if (kvmhv_is_nestedv2()) {

[Severity: High]
If KVM_CAP_PPC_COMPAT_CAPS returns 1 for all pseries hosts (including
nestedv1) in kvm_vm_ioctl_check_extension(), will failing this ioctl here
on non-nestedv2 hosts break userspace?

Userspace might rely on the capability advertisement and fail unexpectedly
when the ioctl returns an error instead of succeeding with empty or base
capabilities.

> +			WARN_ON_ONCE(!nested_capabilities);
> +			capabilities = nested_capabilities;
> +			rc = 0;
> +		}
> +	}
> +
> +	host_caps->compat_capabilities = capabilities & KVM_PPC_COMPAT_BITMASK;
> +
> +	return rc;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2