LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Amit Machhiwal <amachhiw@linux.ibm.com>
To: Ritesh Harjani <ritesh.list@gmail.com>
Cc: Amit Machhiwal <amachhiw@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org,
	Madhavan Srinivasan <maddy@linux.ibm.com>,
	Vaibhav Jain <vaibhav@linux.ibm.com>,
	Harsh Prateek Bora <harshpb@linux.ibm.com>,
	Anushree Mathur <anushree.mathur@linux.ibm.com>,
	Gautam Menghani <gautam@linux.ibm.com>,
	Mukesh Kumar Chaurasiya <mkchauras@gmail.com>,
	Nicholas Piggin <npiggin@gmail.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	"Christophe Leroy (CS GROUP)" <chleroy@kernel.org>,
	Thomas Huth <thuth@redhat.com>,
	kvm@vger.kernel.org, stable@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] KVM: PPC: Book3S HV: Validate arch_compat against host compatibility mode
Date: Tue, 16 Jun 2026 16:24:13 +0530	[thread overview]
Message-ID: <20260616161011.835c90f0-38-amachhiw@linux.ibm.com> (raw)
In-Reply-To: <cxxqerzk.ritesh.list@gmail.com>

Hi Ritesh,

Thank you for the review and feedback. Please find my response below.

On 2026/06/16 03:17 PM, 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_GUEST_SET_STATE is invoked with a Power11 Logical PVR, the
> > hypervisor rejects the request, leading to a late guest boot failure:
> >
> >   KVM-NESTEDv2: couldn't set guest wide elements
> >   [..KVM reg dump..]
> >
> > This situation should be detected earlier and rejected by KVM. Without
> > proper validation, if userspace ignores the error, the guest may continue
> > to boot in Power11 raw mode on a Power10 compatibility host, which should
> > not be allowed.
> >
> > Introduce a validation mechanism that detects unsupported arch_compat
> > values early in the guest initialization path. When an unsupported
> > arch_compat is requested (e.g., Power11 on a Power10 compatibility mode
> > host), kvmppc_set_arch_compat() uses cpu_has_feature(CPU_FTR_P11_PVR) to
> > detect the mismatch and sets arch_compat to PVR_ARCH_INVALID. This
> > triggers kvmppc_sanity_check() to mark the vCPU as invalid by setting
> > vcpu->arch.sane to false. On the next vCPU run, kvmppc_vcpu_run_hv()
> > checks this flag and returns -EINVAL, preventing the guest from running
> > with an invalid processor compatibility configuration.
> >
> > With this, when a Power11 arch_compat is requested on a Power10
> > compatibility mode host, the guest fails early during boot with:
> >
> >   error: kvm run failed Invalid argument
> >
> > This provides a much clearer failure mode compared to the previous
> > behavior where the guest could boot in Power11 raw mode (if userspace
> > ignored the error) or fail late during H_GUEST_SET_STATE.
> >
> > Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> > Reviewed-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> > Cc: stable@vger.kernel.org # v6.13+
> > Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> > ---
> > Changes in v3:
> > * Fixed null pointer dereference in kvmppc_sanity_check(): added check for
> >   vcpu->arch.vcore before accessing arch_compat, as vcore is NULL for Book3S
> >   PR and BookE guests (only Book3S HV uses vcore) [Reported by Sashiko AI]
> > * Added Reviewed-by tag from Vaibhav
> >
> > Changes in v2:
> > * Fixed issue where v1 allowed guest to boot in Power11 raw mode when
> >   userspace ignored the error, by adding validation in kvmppc_sanity_check()
> >   to ensure early failure during vCPU run [Found the issue after posting v1,
> >   also reported by Gautam.]
> 
> Would be nice if we could post the matrix test results which Gautam
> posted earlier with this v3. I guess you meant you already tested all of
> those - it would be nice if we could explicitely put that info in the changelog.

Regarding the test matrix: Both Anushree and I have tested all the
scenarios comprehensively. Anushree has shared her detailed test results
in her reply to the patch [1], covering:

  - P11 guest on P11 host -> Works
  - P10 guest on P11 host -> Works
  - P11 guest on compat-P10 host -> Correctly fails with "Invalid argument"
  - P10 guest on compat-P10 host -> Works

I have also verified all these scenarios with the same results.

If you'd prefer that I add the test matrix explicitly to the changelog
and send a new version, please let me know and I'll be happy to do so.

[1] https://lore.kernel.org/all/4e833bec-67ae-4b78-9a50-e1b9dec4029a@linux.ibm.com/

> 
> > * Introduced PVR_ARCH_INVALID constant for marking invalid arch_compat
> > * Dropped all Reviewed-by and Tested-by tags due to code changes; requesting
> >   fresh reviews
> > * v1: https://lore.kernel.org/all/20260603141539.47620-1-amachhiw@linux.ibm.com/
> >
> > Changes in v1:
> > * Moved this patch out of the v3 series [1] as discussed here [2]
> > * Addressed below review comments from Ritesh:
> >   - Based the PVR validation on cpu features
> >   - Fixed hcall name typo
> >   - Stable backport
> >
> > [1] https://lore.kernel.org/all/20260522152744.55251-1-amachhiw@linux.ibm.com/
> > [2] https://lore.kernel.org/all/20260522152744.55251-2-amachhiw@linux.ibm.com/
> > ---
> >  arch/powerpc/include/asm/reg.h |  1 +
> >  arch/powerpc/kvm/book3s_hv.c   | 15 ++++++++++++++-
> >  arch/powerpc/kvm/powerpc.c     |  4 ++++
> >  3 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> > index 3449dd2b577d..7472b9522f71 100644
> > --- a/arch/powerpc/include/asm/reg.h
> > +++ b/arch/powerpc/include/asm/reg.h
> > @@ -1356,6 +1356,7 @@
> >  #define PVR_ARCH_300	0x0f000005
> >  #define PVR_ARCH_31	0x0f000006
> >  #define PVR_ARCH_31_P11	0x0f000007
> > +#define PVR_ARCH_INVALID	0xffffffff
> 
> Logical processor version is defined as part of the PAPR spec. We should
> ensure that this invalid PVR is also documented in the PAPR spec.
> 
> If you have already taken care of that, then please confirm and feel free to add:

Regarding the PAPR specification documentation: The PAPR spec documents
the valid Processor Version Register (PVR) values for each processor
generation (POWER8, POWER9, POWER10, POWER11, etc.). However, the
PVR_ARCH_INVALID value (0xffffffff) introduced in this patch series is a
KVM implementation detail used internally to mark invalid compatibility
mode requests - it's not an architectural value that would be defined in
PAPR itself.

The validation logic and the use of PVR_ARCH_INVALID as a sentinel value
are documented in the kernel code and commit message.

Please let me know if this addresses your concern, or if you'd like me
to add specific documentation.

> 
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

Thanks you again for taking the time out of the review.

~Amit

> 


  reply	other threads:[~2026-06-16 10:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-09  5:33 [PATCH v3] KVM: PPC: Book3S HV: Validate arch_compat against host compatibility mode Amit Machhiwal
2026-06-16  9:47 ` Ritesh Harjani
2026-06-16 10:54   ` Amit Machhiwal [this message]
2026-06-16 12:08     ` Ritesh Harjani
2026-06-16 12:59       ` Amit Machhiwal
2026-06-16 13:09         ` Ritesh Harjani
2026-06-16 13:37           ` Amit Machhiwal
2026-06-16 10:19 ` Anushree Mathur
2026-06-16 12:22 ` Gautam Menghani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260616161011.835c90f0-38-amachhiw@linux.ibm.com \
    --to=amachhiw@linux.ibm.com \
    --cc=anushree.mathur@linux.ibm.com \
    --cc=chleroy@kernel.org \
    --cc=gautam@linux.ibm.com \
    --cc=harshpb@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mkchauras@gmail.com \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=ritesh.list@gmail.com \
    --cc=stable@vger.kernel.org \
    --cc=thuth@redhat.com \
    --cc=vaibhav@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox