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

On 2026/08/04 06:27 PM, [email protected] wrote:
> 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?

The premise is incorrect for the context where this code runs. This code
path is guarded by kvmhv_on_pseries(), which is true only when the
kernel is running as a PAPR guest (under PHYP or KVM on PowerNV acting
as L0). In that context, the `cpu-version` property in the guest DT is
always populated by the hypervisor (PHYP or QEMU) with a logical PVR
value including for native-mode guests. This is standard PAPR firmware
behavior and is documented in the PAPR specification.

For KVM on PowerNV (L0), the L1 guest's DT is built by QEMU which sets
cpu-version to the negotiated CPU version. For PHYP-managed LPARs, PHYP
always provides cpu-version regardless of whether the guest is in compat
mode or native mode.

The `if (!cpu_version) return -EINVAL` guard handles only the case where
of_property_read_u32() fails and leaves cpu_version at its 0
initialisation value — which would indicate an unexpected firmware or DT
construction error, not a normal native-mode guest scenario.

> 
> > +			rc = kvmppc_map_compat_capabilities(cpu_version,
> > +							    &capabilities);
> >  		}
> >  	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3