Re: [PATCH v6 0/4] KVM: PPC: Expose CPU compatibility modes for nested guests

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

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?

-ritesh