Thread (21 messages) 21 messages, 5 authors, 23d ago

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

From: Amit Machhiwal <hidden>
Date: 2026-05-29 10:28:48
Also in: kvm, linux-doc, lkml

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 [off-list ref] writes:
quoted
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.
quoted
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.
quoted
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.
quoted
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 <redacted>
Tested-by: Anushree Mathur <redacted>
Signed-off-by: Amit Machhiwal <redacted>
---
 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
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help