On 2026/08/07 10:05 AM, Ritesh Harjani wrote:
> Amit Machhiwal <[email protected]> writes:
>
> > Add documentation for the KVM_PPC_GET_COMPAT_CAPS ioctl to the KVM API
> > documentation.
> >
> > The ioctl exposes host processor compatibility modes supported for
> > nested KVM guests on PowerPC systems. The documentation covers error
> > code descriptions including E2BIG for forward compatibility, the
> > extensible size-based versioning contract using
> > KVM_PPC_COMPAT_CAPS_SIZE_VER0, the rationale for rejecting non-zero
> > reserved fields to prevent ABI ambiguity, bit numbering clarification
> > for IBM MSB-0 convention, and KVM-specific capability bit constants.
> >
> > Tested-by: Gautam Menghani <[email protected]>
> > Reviewed-by: Gautam Menghani <[email protected]>
> > Tested-by: Anushree Mathur <[email protected]>
> > Signed-off-by: Amit Machhiwal <[email protected]>
> > ---
> > Documentation/virt/kvm/api.rst | 79 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 79 insertions(+)
> >
> > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > index e3003a241d5b..22fedb0aa34b 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -6566,6 +6566,85 @@ KVM_S390_KEYOP_SSKE
> > Sets the storage key for the guest address ``guest_addr`` to the key
> > specified in ``key``, returning the previous value in ``key``.
> >
> > +4.145 KVM_PPC_GET_COMPAT_CAPS
> > +-----------------------------
> > +:Capability: KVM_CAP_PPC_COMPAT_CAPS
> > +:Architectures: powerpc
> > +:Type: vm ioctl
> > +:Parameters: struct kvm_ppc_compat_caps (in/out)
> > +:Returns: 0 on success, negative value on failure
> > +
> > +Errors include:
> > +
> > + ======== ============================================================
> > + EFAULT if ``struct kvm_ppc_compat_caps`` cannot be read from or
> > + written to userspace
> > + EINVAL if the ``size`` field is smaller than
> > + ``KVM_PPC_COMPAT_CAPS_SIZE_VER0``, if the ``flags`` field
> > + is non-zero, or if the backend fails to retrieve or map
> > + CPU compatibility capabilities
> > + E2BIG if ``size`` is larger than the kernel's struct size
> > + (new userspace on old kernel); the kernel writes back its
> > + own struct size into the ``size`` field so userspace can
> > + retry with the correct size
> > + ENOTTY if the backend does not implement the ``get_compat_caps``
> > + operation (e.g., on non-HV KVM implementations where the
> > + required KVM operations are not available)
>
> Amit, this may not be true anymore right after your changes in v7?
> Can we please update the documentation accordingly as well.
Agreed. After dropping the manual pre-check in patch-1 and delegating to
copy_struct_from_user(), -E2BIG is no longer unconditional when usize >
ksize — it only fires if the unknown trailing bytes are non-zero. Will
update the E2BIG entry to:
E2BIG if ``size`` is larger than the kernel's struct size and
the unknown trailing bytes are non-zero (new userspace on
old kernel with non-default fields set); the kernel writes
back its own struct size into the ``size`` field so
userspace can retry with the correct size
>
> > + ======== ============================================================
> > +
> > +IBM POWER system server-based processors provide a compatibility mode
> > feature
> > +where an Nth generation processor can operate in modes consistent with
> > earlier
> > +generations such as (N-1) and (N-2).
> > +
> > +This ioctl provides userspace with information about the CPU compatibility
> > modes
> > +supported by the current host processor for booting the nested KVM guests
> > on
> > +KVM on PowerNV (nested API v1) and KVM on PowerVM (nested API v2)
> > platforms.
> > +
> > +::
> > +
> > + struct kvm_ppc_compat_caps {
> > + __u64 size; /* Size of this structure */
> > + __u64 flags; /* Reserved for future use, must be 0 */
> > + __u64 compat_capabilities; /* Capabilities supported by the host */
> > + };
> > +
> > +Before calling this ioctl, userspace must set the ``size`` field to
> > +``sizeof(struct kvm_ppc_compat_caps)`` and zero the ``flags`` field.
> > +The kernel rejects non-zero ``flags`` with ``-EINVAL`` to prevent
> > +uninitialized stack values from being silently accepted, keeping the
> > +field available for future use without ABI ambiguity.
> > +
> > +The ioctl uses ``copy_struct_from_user()`` and ``copy_struct_to_user()``
> > +to support extensible versioning: if userspace passes a struct smaller
> > +than the current kernel version (``size >=
> > KVM_PPC_COMPAT_CAPS_SIZE_VER0``),
> > +the kernel zero-pads unknown trailing fields. If userspace passes a larger
>
> So I already requested that we should fix this. We cannot write more
> bytes than requested by the user, since that memory may not be allocated
> for this struct in userspace.
>
> On checking Sashiko comments in reply to this patch - I think that is
> also complaining of the same thing that it could cause buffer overflow.
copy_struct_to_user() itself is safe — it caps its write to min(ksize,
usize) bytes so it never writes past the user's buffer. However, the
problem is in the value written back in the size field: if usize <
sizeof(host_caps) (old userspace, new kernel), we'd write size =
sizeof(host_caps) into the first 8 bytes of the user's smaller buffer.
If userspace then reuses the struct naively, it would pass usize =
sizeof(host_caps) against its smaller allocation, which would cause an
actual overflow on the next call.
The fix is to write back usize instead:
host_caps.size = usize;
r = copy_struct_to_user(argp, usize, &host_caps,
sizeof(host_caps), NULL);
This tells userspace "I filled exactly as many bytes as you gave me",
which is the correct contract for copy_struct_to_user().
Will update both the code in patch-1 and the versioning paragraph in the
documentation accordingly in v8.
Thanks,
Amit
>
>
> > +struct (``size > sizeof(struct kvm_ppc_compat_caps)``), the kernel writes
> > +back its own struct size into the ``size`` field and returns ``-E2BIG``,
> > +allowing userspace to discover the kernel's struct size and retry.
> > +``KVM_PPC_COMPAT_CAPS_SIZE_VER0`` (24) is a frozen constant marking the
> > +size of the initial struct version.
>
> Once we update the comments in patch-1 - I think we should correct this
> documentation too accordingly. We should just simply use
> copy_to|from_user_struct() style for doing this.
>
> > +
> > +The ``compat_capabilities`` bit field describes the processor compatibility
> > +modes supported by the host. The following bits indicate support for
> > specific
> > +processor modes (using IBM's MSB-0 convention where bit 0 is the most
> > +significant bit):
> > +
> > +- ``KVM_PPC_COMPAT_CAP_POWER9`` (bit 1) -- KVM guests can run in Power9
> > processor mode
> > +- ``KVM_PPC_COMPAT_CAP_POWER10`` (bit 2) -- KVM guests can run in Power10
> > processor mode
> > +- ``KVM_PPC_COMPAT_CAP_POWER11`` (bit 3) -- KVM guests can run in Power11
> > processor mode
> > +
> > +.. note::
> > +
> > + The bit numbering above uses IBM's MSB-0 convention (bit 0 is the most
> > + significant bit). In the actual implementation, these are defined as:
> > +
> > + - ``KVM_PPC_COMPAT_CAP_POWER9`` = ``(1ULL << 62)``
> > + - ``KVM_PPC_COMPAT_CAP_POWER10`` = ``(1ULL << 61)``
> > + - ``KVM_PPC_COMPAT_CAP_POWER11`` = ``(1ULL << 60)``
> > +
> > + Userspace should use the defined constants from ``<linux/kvm.h>`` rather
> > + than hardcoding bit positions.
> > +
> > .. _kvm_run:
> >
> > 5. The kvm_run structure
> > --
> > 2.50.1 (Apple Git-155)
