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

On 2026/08/07 10:24 AM, Ritesh Harjani wrote:

Amit Machhiwal [off-list ref] writes:

<snip>

quoted

 
 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;
+				}
+			}

What happens when we don't have "cpu-version" DT property?
Check this commit
5a61ef74f269f2 ("powerpc/64s: Support new device tree binding for discovering CPU features")

And also why do we need to parse the DT properties again?
Shouldn't we check something like this?

	if (cpu_has_feature(CPU_FTR_P11_PVR))
		capabilities |= KVM_PPC_COMPAT_CAP_POWER11;
	if (cpu_has_feature(CPU_FTR_ARCH_31))
		capabilities |= KVM_PPC_COMPAT_CAP_POWER10;
	if (cpu_has_feature(CPU_FTR_ARCH_300))
		capabilities |= KVM_PPC_COMPAT_CAP_POWER9;
	

Thanks for the suggestion, Ritesh! After closer analysis, cpu_has_feature()
would give the same result on pseries, but I believe that using 'cpu-version'
directly is both more correct and more explicit for this context.

Here's why:

1. 'ibm,powerpc-cpu-features' / dt-cpu-ftrs is baremetal (powernv/OPAL) only.
   SLOF firmware used by pseries guests never provides that node. On pseries,
   dt_cpu_ftrs_in_use() is always false, and CPU feature bits including
   CPU_FTR_ARCH_300, CPU_FTR_ARCH_31, CPU_FTR_P11_PVR are set by identify_cpu()
   from the cpu-version DT property at prom.c:423:

     if (!dt_cpu_ftrs_in_use()) {
         prop = of_get_flat_dt_prop(node, "cpu-version", NULL);
         if (prop && (be32_to_cpup(prop) & 0xff000000) == 0x0f000000) {
             identify_cpu(0, be32_to_cpup(prop));

   So cpu_has_feature() on pseries is just an indirect readback of what
   identify_cpu() already derived from 'cpu-version' — the source of truth is
   still cpu-version.

2. Commit e4de1b9cb3b5 ("powerpc/dt_cpu_ftrs: Set CPU_FTR_P11_PVR for Power11
   and later processors") explicitly documents this split:

     "This issue does not affect pseries guests, where SLOF firmware
      does not provide this node, causing the kernel to fall back to
      the traditional cputable path (identify_cpu) which correctly
      sets CPU_FTR_P11_PVR during PVR-based CPU identification."

   That fix was needed on powernv only — and our code is in the
   kvmhv_on_pseries() branch, so the dt-cpu-ftrs path is never taken.

3. 'cpu-version' is the PAPR-defined compat level indicator — it is what PHYP
   and QEMU explicitly set to communicate the negotiated compat mode. Reading it
   directly is semantically correct: we are reporting the compat level the
   hypervisor advertised, not a kernel-internal feature bit derived from it.

I also responded to a similar concern from Sashiko covering points 1 and 3 here:

  https://lore.kernel.org/all/20260806214117.8a2ca150-97-amachhiw@linux.ibm.com/ (local)

So I'd prefer to keep the direct cpu-version lookup.

Thanks,
Amit