Kernel KVM virtualization development
 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>,
	Anushree Mathur <anushree.mathur@linux.ibm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Nicholas Piggin <npiggin@gmail.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	"Christophe Leroy (CS GROUP)" <chleroy@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH v6 0/4] KVM: PPC: Expose CPU compatibility modes for nested guests
Date: Thu, 6 Aug 2026 20:28:11 +0530	[thread overview]
Message-ID: <20260806195943.1fa6cacd-be-amachhiw@linux.ibm.com> (raw)
In-Reply-To: <mruzl7qc.ritesh.list@gmail.com>

On 2026/08/06 06:35 PM, Ritesh Harjani wrote:
> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
> 
> > Hi Ritesh,
> >
> > Thanks for taking a look. Please find my response inline.
> >
> > On 2026/08/06 12:09 AM, Ritesh Harjani wrote:
> >> 
> >> Hi Amit,
> >> 
> >> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
> >> 
> >> > On POWER systems, newer processor generations can operate in compatibility
> >> > modes corresponding to earlier generations (e.g., a Power11 system running
> >> > in Power10 compatibility mode). In such cases, the effective CPU level
> >> > exposed to guests differs from the physical processor generation.
> >> >
> >> > This creates a problem for nested virtualization. When booting a nested KVM
> >> > guest (L2) inside a host KVM guest (L1) running in a compatibility mode,
> >> > userspace (e.g., QEMU) may derive the CPU model from the raw hardware PVR
> >> > and attempt to configure the nested guest accordingly. However, the L1
> >> > partition is constrained by the compatibility level negotiated with the
> >> > hypervisor (L0), and requests exceeding that level are rejected, leading to
> >> > guest boot failures such as:
> >> >
> >> >   KVM-NESTEDv2: couldn't set guest wide elements
> >> >
> >> > This series provides a mechanism for userspace to query the effective CPU
> >> > compatibility modes supported by the host, so it can select an appropriate
> >> > CPU model for nested guests.
> >> >
> >> > To achieve this, the series introduces a new KVM capability and ioctl
> >> > (KVM_CAP_PPC_COMPAT_CAPS / KVM_PPC_GET_COMPAT_CAPS) that expose the
> >> > compatibility modes supported by the host.
> >> >
> >> 
> >> Sorry, but I am somehow not convinced on whether we need all of this
> >> machinary just to get these 3 bits of information, which we are
> >> returning today.
> >> 
> >> Since KVM_CHECK_EXTENSION can already return an int, so why can't we use
> >> KVM_CAP_PPC_COMPAT_CAPS itself and return the bitmap of supported compat
> >> modes to the user?
> >> Say if the cap is not supported, we can return 0, otherwise we can
> >> return the bitmap of supported compat modes. This will easily allow us
> >> to use 31-bits which as I see would be hardly a problem in the near
> >> future. In the future if it grows - we can always use KVM_CAP_PPC_COMPAT_CAPS2.
> >> 
> >> This should reduce the code complexity both in the kernel and
> >> userspace and we don't even need a new ioctl then.
> >
> > Thanks for the suggestion. I considered this approach
> 
> Then we should have brought that up early on during the design
> discussion. But for the sake of discussion let's call this as
> approach-2.
> 
> > but would like to
> > go with a dedicated ioctl for the following reasons:
> >
> 
> > 1. Intended semantics: The KVM API documentation states:
> >
> >    ..kvm defines extension identifiers and a facility to query
> >    whether a particular extension identifier is available.  If it is, a
> >    set of ioctls is available for application use.
> >
> >    [...]
> >
> >    KVM defines many constants of the form KVM_CAP_*, each corresponding
> >    to a set of functionality provided by one or more ioctls. Availability
> >    of these capabilities can be checked with KVM_CHECK_EXTENSION.
> >
> >    The intended role of KVM_CAP_* is to signal ioctl availability, not
> >    to serve as a data retrieval mechanism itself.
> 
> That's not entirely true. We do return data as part of check extension
> for e.g. for getting the SMT modes check KVM_CAP_PPC_SMT_POSSIBLE.
> 
> >
> >    You may take a look at KVM_CAP_PPC_GET_CPU_CHAR for instance.
> >
> > 2. Return type constraint: KVM_CHECK_EXTENSION returns a signed 32-bit int. The
> >    capability bits are defined as (1ULL << 62), (1ULL << 61), and (1ULL << 60) —
> >    64-bit values that cannot fit in a 32-bit return. Renumbering them to small
> >    integers would be a UAPI change and would lose alignment with the
> 
> There is no _change_ in the UAPI so far. This is the patch which is
> defining that in the first place.
> 
> >    H_GUEST_CAP_* values from the hypervisor ABI.
> 
> No please. Those are 2 different ABIs and there is no need to set a hard
> dependency among the two.
> 
> >
> > 3. Extensibility: The struct-based approach with the size field provides clean
> >    forward and backward ABI versioning via copy_struct_from/to_user(), without
> >    needing a KVM_CAP_PPC_COMPAT_CAPS2 in the future.
> >
> 
> This only make sense if we really have a usecase already in mind which
> you are planning to extend it for. Otherwise, IMO, this is a lot of
> machinary and I think we should consider the simpler approach.
> 
> IMO - I think approach-2 is a much simpler for this usecase. I don't see
> any valid reason on why we should not do that instead. We don't need an
> extra ioctl and all the struct machinary along with that just for
> returning a bitmask. The existing check extension ioctl can be
> easily used for this purpose.
> 
> Would it be possible for you to give, approach-2 a try? Do you see any
> geniunine roadblock or limitation with that?

As discussed, one additional and deeper technical reason is that the
Power Hypervisor (PHYP) returns a 64-bit capabilities mask as part of
the H_GUEST_GET_CAPABILITIES hcall (defined in the PAPR specification).
Our KVM_PPC_COMPAT_CAP_* bits are deliberately aligned to this 64-bit
mask so that the cached nested_capabilities value can be used directly
without any translation.

The PAPR spec reserves bits beyond the current processor modes and
capability flags for future capabilities. As new processor generations
arrive, some of those reserved bits may become relevant for them — at
which point a 32-bit return from KVM_CHECK_EXTENSION would be
insufficient, forcing a KVM_CAP_PPC_COMPAT_CAPS2.

The struct-based ioctl avoids this problem cleanly.

Thanks,
Amit

> 
> -ritesh
> 

      reply	other threads:[~2026-08-06 14:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04 18:07 [PATCH v6 0/4] KVM: PPC: Expose CPU compatibility modes for nested guests Amit Machhiwal
2026-08-04 18:07 ` [PATCH v6 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl Amit Machhiwal
2026-08-04 18:19   ` sashiko-bot
2026-08-06 16:07     ` Amit Machhiwal
2026-08-04 18:07 ` [PATCH v6 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM Amit Machhiwal
2026-08-04 18:21   ` sashiko-bot
2026-08-06 16:09     ` Amit Machhiwal
2026-08-04 18:07 ` [PATCH v6 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV Amit Machhiwal
2026-08-04 18:27   ` sashiko-bot
2026-08-06 16:13     ` Amit Machhiwal
2026-08-04 18:07 ` [PATCH v6 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl Amit Machhiwal
2026-08-04 18:27   ` sashiko-bot
2026-08-06 16:15     ` Amit Machhiwal
2026-08-05  4:32 ` [PATCH v6 0/4] KVM: PPC: Expose CPU compatibility modes for nested guests Anushree Mathur
2026-08-05 18:39 ` Ritesh Harjani
2026-08-06  5:33   ` Amit Machhiwal
2026-08-06 13:05     ` Ritesh Harjani
2026-08-06 14:58       ` Amit Machhiwal [this message]

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=20260806195943.1fa6cacd-be-amachhiw@linux.ibm.com \
    --to=amachhiw@linux.ibm.com \
    --cc=anushree.mathur@linux.ibm.com \
    --cc=chleroy@kernel.org \
    --cc=corbet@lwn.net \
    --cc=kvm@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=ritesh.list@gmail.com \
    --cc=skhan@linuxfoundation.org \
    --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