Re: [PATCH v8 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl

Amit Machhiwal [off-list ref] writes:

Introduce a new capability and ioctl to expose CPU compatibility modes
supported by the host processor for nested guests.

On IBM POWER systems, newer processor generations (N) can operate in
compatibility modes corresponding to earlier generations, like (N-1) and
(N-2). This is particularly relevant for nested virtualization, where
nested KVM guests may need to run with a specific processor compatibility
level.

Introduce KVM_CAP_PPC_COMPAT_CAPS capability and the corresponding
KVM_PPC_GET_COMPAT_CAPS vm ioctl. The ioctl returns a bitmap describing
the compatibility modes supported by the host in respective bit numbers,
allowing userspace (e.g., QEMU) to select an appropriate compatibility
level when configuring nested KVM guests.

The ioctl handling is added in kvm_arch_vm_ioctl() and retrieves host
CPU compatibility capabilities via a PowerPC-specific backend
implementation when available.

The struct kvm_ppc_compat_caps places the 'size' field first so it can
be read alone via get_user() before copy_struct_from_user() is called,
avoiding pointer arithmetic to locate the size field.

The ioctl is defined using _IO so the ioctl number remains stable even if
the struct grows in future versions. It uses copy_struct_from_user() and
copy_struct_to_user() to provide forward- and backward-compatible
extensibility: older userspace passing a smaller struct to a newer kernel
gets zero-padded trailing fields. Newer userspace passing a larger struct to
an older kernel (usize > ksize) succeeds if trailing bytes are zero (the
kernel reports back min(usize, ksize) as the filled size); if trailing bytes
are non-zero, the kernel writes back ksize into host_caps.size and returns
-E2BIG so userspace can retry with the correct size.

KVM_PPC_COMPAT_CAPS_SIZE_VER0 is defined as a frozen integer constant
(24) marking the size of the initial struct version, used as the
minimum floor for size field validation, similar to other versioned
struct interfaces in the kernel.

The 'flags' field is reserved for future use. The kernel rejects any
call where flags is non-zero with -EINVAL, preventing garbage values
from being baked into ABI permanently.

The ioctl returns appropriate error codes: E2BIG if usize exceeds
PAGE_SIZE, or if new userspace provides a larger struct with non-zero
trailing bytes (with ksize written back into host_caps.size for the
retry); EINVAL for an invalid size or non-zero reserved fields; EFAULT
for failed copy operations; and ENOTTY if the backend doesn't implement
get_compat_caps.

Suggested-by: Vaibhav Jain <redacted>
Tested-by: Gautam Menghani <redacted>
Reviewed-by: Gautam Menghani <redacted>
Tested-by: Anushree Mathur <redacted>
Signed-off-by: Amit Machhiwal <redacted>
---
Changes in this version:
  - Add PAGE_SIZE guard after get_user() to bound the check_zeroed_user()
    scan in the usize > ksize path [Ritesh]
  - Drop manual usize > sizeof(host_caps) pre-check; delegate entirely to
    copy_struct_from_user() which succeeds on zero trailing bytes and
    returns -E2BIG only on non-zero trailing bytes; handle -E2BIG with
    ksize writeback and -EFAULT escalation if put_user() fails [Ritesh]
  - Fix host_caps.size on success path: use min_t(u64, usize,
    sizeof(host_caps)) so new userspace with zero trailing bytes gets back
    the number of bytes the kernel actually populated, not usize [Ritesh]

Thanks for addressing them!
The only remaining comments from Sashiko now are because, it cannot find
the implementation of ->get_compat_caps() since it is in the next patch.

So as for this patch, the changes looks good to me. Please feel free to
add:

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

 arch/powerpc/include/asm/kvm_ppc.h  |  1 +
 arch/powerpc/include/uapi/asm/kvm.h |  8 +++
 arch/powerpc/kvm/powerpc.c          | 78 +++++++++++++++++++++++++++++
 include/uapi/linux/kvm.h            |  3 ++
 4 files changed, 90 insertions(+)