On 2026/08/07 10:24 AM, Ritesh Harjani wrote:
> Amit Machhiwal <[email protected]> writes:
<snip>
> >
> > 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/[email protected]/
So I'd prefer to keep the direct cpu-version lookup.
Thanks,
Amit
