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