Re: [PATCH v6 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] `kvmppc_get_compat_caps()` incorrectly returns `-EINVAL` when the 'cpu-version' property is absent, breaking the `KVM_CAP_PPC_COMPAT_CAPS` contract.
--

--- Patch [3]: [PATCH v6 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV ---
commit b4de9c28bb9d6eeb95ed5310169709b332b6f664
Author: Amit Machhiwal <[email protected]>

KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV

This commit extends host CPU compatibility capability reporting to support
nested virtualization on PowerNV systems. For nested API v1, it derives
compatibility capabilities from the L1 guest by reading the cpu-version
property from the device tree and maps it to the corresponding compatibility
capability bitmap.

> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 8799124b80960..77558f6eed864 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c

[ ... ]

>  static int kvmppc_get_compat_caps(struct kvm_ppc_compat_caps *host_caps)
>  {
> +	struct device_node *np;
>  	unsigned long capabilities = 0;
>  	long rc = -EINVAL;
> +	u32 cpu_version = 0;
>  
>  	if (kvmhv_on_pseries()) {
>  		if (kvmhv_is_nestedv2()) {
>  			WARN_ON_ONCE(!nested_capabilities);
>  			capabilities = nested_capabilities;
>  			rc = 0;
> +		} else {
> +			for_each_node_by_type(np, "cpu") {
> +				if (!of_property_read_u32(np, "cpu-version",
> +							  &cpu_version)) {
> +					of_node_put(np);
> +					break;
> +				}
> +			}
> +			if (!cpu_version)
> +				return -EINVAL;

[Severity: High]
If an L1 guest is booted in native mode (e.g. without a 'compat=' setting in
QEMU), the cpu-version property is intentionally omitted from the device
tree.

Doesn't returning -EINVAL here cause KVM_PPC_GET_COMPAT_CAPS to fail,
thereby breaking nested virtualization initialization for native-mode L1
guests?

Could this fall back to deriving capabilities from the physical native PVR
instead of returning an error?

> +			rc = kvmppc_map_compat_capabilities(cpu_version,
> +							    &capabilities);
>  		}
>  	}

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