Re: [PATCH v3 1/5] KVM: PPC: Book3S HV: Validate arch_compat against host compatibility mode

[Amit Machhiwal <amachhiw@linux.ibm.com>]

Hi Ritesh,

Thanks for taking a look at this patch. Please find my response inline
below:

On 2026/05/28 08:43 AM, Ritesh Harjani wrote:
> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
> 
> > On IBM POWER systems, newer processor generations can operate in
> > compatibility modes corresponding to earlier generations. This becomes
> > relevant for nested virtualization, where nested KVM guests may need to
> > run with a specific processor compatibility level.
> >
> > Currently, when running a nested KVM guest (L2) inside a Power11 pSeries
> > logical partition (L1) booted in Power10 compatibility mode, the guest
> > fails to boot while setting 'arch_compat'. This happens because the CPU
> > class is derived from the hardware PVR (via mfspr()), which reflects the
> > physical processor generation (Power11), rather than the effective
> > compatibility mode (Power10).
> >
> > As a result, userspace may request a Power11 arch_compat for the L2
> > guest. However, the L1 partition, running in Power10 compatibility, has
> > only negotiated support up to Power10 with the Power Hypervisor (L0).
> > When H_SET_STATE is invoked with a Power11 Logical PVR, the hypervisor
> 
> s/H_SET_STATE/H_GUEST_SET_STATE 

Good catch! I'll fix this in the next version.

> 
> > rejects the request, leading to a late guest boot failure:
> >
> >   KVM-NESTEDv2: couldn't set guest wide elements
> >   [..KVM reg dump..]
> >
> 
> I think irrespective of the other UAPI changes, we should still get this
> fixed - so that we don't see a late KVM guest boot failure msgs.
> 
> So, in this review, I would like to mainly look at fixing this issue
> first and would request if we can defer the UAPI changes as a separate
> patch series please.

This patch only enables the guest boot to bail out early instead of going upto
making an H_GUEST_SET_STATE hcall with a non supported compatibility mode. In
addition to that, it does not fix the real problem where guest fails to boot on
Power11 LPAR booted in a Power10 compatibility mode.

I understand your point and to make the segregation explicitly clear, I shall
update the cover letter to mention that patch 1 only takes care of failing
earlier as soon as an invalid compatibility mode is detected and Patch 2-5
introduce a new uAPI for evaluating the right compatibility mode.

The actual L2 guest boot fix with L1 booted in a compatibility mode is done via
the next 4 patches which enables correct compatibility mode detection using the
newly introduced uAPI. So, we would still want to prioritize the whole series
instead of just this one patch.

> 
> 
> > This situation should be detected earlier. Rejecting unsupported
> > 'arch_compat' values in 'kvmppc_set_arch_compat()' avoids issuing an
> > invalid H_SET_STATE hcall and provides a clearer failure mode.
> 
> s/H_SET_STATE/H_GUEST_SET_STATE

Will rectify in the next version.

> 
> >
> > Add a check to reject Power11 'arch_compat' requests when the host is
> > running in Power10 compatibility mode, returning -EINVAL early instead
> > of deferring the failure to the hypervisor.
> >
> > Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> > Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
> > Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> > ---
> >  arch/powerpc/kvm/book3s_hv.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > index 61dbeea317f3..249d1f2e4e2c 100644
> > --- a/arch/powerpc/kvm/book3s_hv.c
> > +++ b/arch/powerpc/kvm/book3s_hv.c
> > @@ -446,7 +446,19 @@ static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, u32 arch_compat)
> >  			guest_pcr_bit = PCR_ARCH_300;
> >  			break;
> >  		case PVR_ARCH_31:
> > +			guest_pcr_bit = PCR_ARCH_31;
> > +			break;
> >  		case PVR_ARCH_31_P11:
> > +			/*
> > +			 * Need to check this for ISA 3.1, as Power10 and
> > +			 * Power11 share the same PCR. For any subsequent ISA
> > +			 * versions, this will be taken care of by the guest vs
> > +			 * host PCR comparison below.
> > +			 */
> > +			if ((PVR_ARCH_31 & cur_cpu_spec->pvr_mask) ==
> > +				cur_cpu_spec->pvr_value) {
> > +				return -EINVAL;
> > +			}
> 
> Instead of the complicated check can we simply do this?
> 			if (!cpu_has_feature(CPU_FTR_P11_PVR))
> 				return -EINVAL;

Sure, I can base this check on CPU features.

Thanks,
Amit

> 
> which means that if the Qemu is trying to set the arch_compat with P11
> PVR (arch_compat) and if the host cpu FTR doesn't support P11 PVR, then
> simply return -EINVAL
> 
> -ritesh
>