On Thu, Apr 04, 2019 at 09:38:49AM +0000, Dave P Martin wrote:
> On Thu, Apr 04, 2019 at 10:34:15AM +0100, Andrew Jones wrote:
> > On Thu, Apr 04, 2019 at 09:35:26AM +0100, Dave Martin wrote:
> > > On Wed, Apr 03, 2019 at 10:27:46PM +0200, Andrew Jones wrote:
> > > > On Fri, Mar 29, 2019 at 01:00:51PM +0000, Dave Martin wrote:
> > > > > KVM_GET_ONE_REG and KVM_SET_ONE_REG return some error codes that
> > > > > are not documented (but hopefully not surprising either). To give
> > > > > an indication of what these may mean, this patch adds brief
> > > > > documentation.
> > > > >
> > > > > Signed-off-by: Dave Martin <dave.mar...@arm.com>
> > > > > ---
> > > > > Documentation/virtual/kvm/api.txt | 6 ++++++
> > > > > 1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/virtual/kvm/api.txt
> > > > > b/Documentation/virtual/kvm/api.txt
> > > > > index 2d4f7ce..cd920dd 100644
> > > > > --- a/Documentation/virtual/kvm/api.txt
> > > > > +++ b/Documentation/virtual/kvm/api.txt
> > > > > @@ -1871,6 +1871,9 @@ Architectures: all
> > > > > Type: vcpu ioctl
> > > > > Parameters: struct kvm_one_reg (in)
> > > > > Returns: 0 on success, negative value on failure
> > > > > +Errors:
> > > > > + ENOENT: no such register
> > > > > + EINVAL: other errors, such as bad size encoding for a known
> > > > > register
> > > > >
> > > > > struct kvm_one_reg {
> > > > > __u64 id;
> > > > > @@ -2192,6 +2195,9 @@ Architectures: all
> > > > > Type: vcpu ioctl
> > > > > Parameters: struct kvm_one_reg (in and out)
> > > > > Returns: 0 on success, negative value on failure
> > > > > +Errors:
> > > > > + ENOENT: no such register
> > > > > + EINVAL: other errors, such as bad size encoding for a known
> > > > > register
> > > > >
> > > > > This ioctl allows to receive the value of a single register
> > > > > implemented
> > > > > in a vcpu. The register to read is indicated by the "id" field of the
> > > > > --
> > > > > 2.1.4
> > > > >
> > > >
> > > > Are we sure all architectures have these, and only these errors? A quick
> > > > grep indicates not. I'm not sure we can document this easily here due to
> > > > it addressing all architectures at once. Maybe we could add
> > > > arch-specific
> > > > subsections, but I'm not sure it's worth it.
> > >
> > > Error codes are generally indicative at best, and rarely mutually
> > > exclusive in a given situation.
> > >
> > > Writing a caveat to say that you shouldn't make assumptions about
> > > precisely what error code will be returned in a given situation would
> > > look odd: that really applies widely all over the kernel/user ABI, not
> > > just here.
> > >
> > > _Maybe_ everything is exhaustively correct in this file already, but
> > > given the size of it, and the fact that many things are implemented
> > > per-arch, the chance of this seems zero.
> > >
> > > I could add "invalid register ID" for EINVAL. This allows some
> > > ambiguity about which error code applies for a nonexistent register.
> > >
> > > Alternatively, we could revert this documentation instead.
> >
> > Yeah, I vote we just drop this patch. It could add more confusion
> > when people start here to determine what to grep and then, when
> > grepping for it, can't find any occurrences, as would be the case
> > for ENOENT on at least a couple arches.
>
> I guess so. It's not like we're claiming this ioctl _can't_ fail.
> So leaving interpretation of the errno values to common sense is
> reasonable.
>
> > > It seemed appropriate to document the EPERM error for finalization
> > > ordering (added later), but since correct userspace code should never
> > > see this maybe it's reasonable to leave it undocumented(?)
>
> I assume you mean to remove this too?
I haven't reviewed that stuff yet, so I don't yet have an opinion :)
I'll get through it all today, but I'm trashing on mail and bugzilla
this morning, so progress on the review hasn't started back up since
last night yet...
drew
>
> I don't think correctly written userspace should be relying on seeing
> this error code (as opposed to e.g., KVM_GET_REG_LIST's E2BIG which you
> must observe in order to allocate the right amount of memory for the reg
> list).
>
> Cheers
> ---Dave
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
