Re: [PATCH v8 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl

Amit Machhiwal <[email protected]>
Newsgroups org.ozlabs.lists.linuxppc-dev,org.kernel.vger.kvm,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On 2026/08/08 06:45 AM, Ritesh Harjani wrote:
> Amit Machhiwal <[email protected]> writes:
> 
> > Add documentation for the KVM_PPC_GET_COMPAT_CAPS ioctl to the KVM API
> > documentation.

<snip>

> > +
> > +The ioctl uses ``copy_struct_from_user()`` and ``copy_struct_to_user()``
> > +to support extensible versioning across three cases:
> > +
> > +- If ``size`` is smaller than the kernel's struct size (old userspace,
> > +  new kernel), the kernel zero-pads the unknown trailing fields before
> > +  returning, and writes back ``size`` unchanged so userspace knows how
> > +  many bytes were filled.
> 
> I agree with Sashiko comment here. This para is slightly misleading.
> This sounds like we are zero padding to userspace struct before
> returning. Whereas what we intend to say here is, when we copy user
> struct into kernel (copy_struct_from_user()), we zero pad the trailing
> bytes in kernel's struct.
> 
> > +- If ``size`` equals the kernel's struct size, the struct is copied
> > +  verbatim.
> > +- If ``size`` is larger than the kernel's struct size (new userspace,
> > +  old kernel) and the unknown trailing bytes are all zero, the call
> > +  succeeds as if the sizes matched. If any trailing bytes are non-zero,
> > +  the kernel returns ``-E2BIG`` and writes back its own struct size into
> > +  the ``size`` field so userspace can retry with the correct size.
> >
> 
> BTW - I anyway feel this is too much. We can get rid of all 3 points
> which explains how struct copying is working. We have more than enough
> documentation around how copy_struct_{from|to}_user() works and we have
> also added the comments around the code. So I think this is just
> unnecessary.

Agreed on both counts. I'll drop the three-bullet block in the next
version - v9 is on the way!

> 
> We can just say:
>    +The ioctl uses ``copy_struct_from_user()`` and ``copy_struct_to_user()``
>    +to support extensible versioning.

Sure, makes sense.

> 
> 
> With that taken care, please feel free to add:
> Reviewed-by: Ritesh Harjani (IBM) <[email protected]>

Thanks again for the detailed reviews on the series. Will carry the R-b
in v9.

~Amit
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.