All of 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>,
	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, Gautam Menghani <gautam@linux.ibm.com>
Subject: Re: [PATCH v8 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl
Date: Sat, 8 Aug 2026 21:17:01 +0530	[thread overview]
Message-ID: <20260808211615.fb580df8-72-amachhiw@linux.ibm.com> (raw)
In-Reply-To: <33wpl93b.ritesh.list@gmail.com>

On 2026/08/08 06:30 AM, Ritesh Harjani wrote:
> Amit Machhiwal <amachhiw@linux.ibm.com> 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 <vaibhav@linux.ibm.com>
> > Tested-by: Gautam Menghani <gautam@linux.ibm.com>
> > Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
> > Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
> > Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> > ---
> > 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.

Yes, correct.

> 
> 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>

Thanks for the review, Ritesh! Will carry the Reviewed-by in v9.

~Amit

  reply	other threads:[~2026-08-08 15:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-07 17:24 [PATCH v8 0/4] KVM: PPC: Expose CPU compatibility modes for nested guests Amit Machhiwal
2026-08-07 17:24 ` [PATCH v8 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl Amit Machhiwal
2026-08-07 17:34   ` sashiko-bot
2026-08-08  1:00   ` Ritesh Harjani
2026-08-08 15:47     ` Amit Machhiwal [this message]
2026-08-07 17:24 ` [PATCH v8 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM Amit Machhiwal
2026-08-07 17:39   ` sashiko-bot
2026-08-07 17:24 ` [PATCH v8 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV Amit Machhiwal
2026-08-07 17:24 ` [PATCH v8 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl Amit Machhiwal
2026-08-07 17:37   ` sashiko-bot
2026-08-08  1:15   ` Ritesh Harjani
2026-08-08 15:54     ` Amit Machhiwal

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=20260808211615.fb580df8-72-amachhiw@linux.ibm.com \
    --to=amachhiw@linux.ibm.com \
    --cc=anushree.mathur@linux.ibm.com \
    --cc=chleroy@kernel.org \
    --cc=corbet@lwn.net \
    --cc=gautam@linux.ibm.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.