[PATCH] vfio-ccw: document possible errors

[PATCH] vfio-ccw: document possible errors

[Cornelia Huck]

Interacting with the I/O and the async regions can yield a number
of errors, which had been undocumented so far. These are part of
the api, so remedy that.

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 Documentation/s390/vfio-ccw.rst | 54 ++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/Documentation/s390/vfio-ccw.rst b/Documentation/s390/vfio-ccw.rst
index fca9c4f5bd9c..4538215a362c 100644
--- a/Documentation/s390/vfio-ccw.rst
+++ b/Documentation/s390/vfio-ccw.rst
@@ -210,7 +210,36 @@ Subchannel.
 
 irb_area stores the I/O result.
 
-ret_code stores a return code for each access of the region.
+ret_code stores a return code for each access of the region. The following
+values may occur:
+
+``0``
+  The operation was successful.
+
+``-EOPNOTSUPP``
+  The orb specified transport mode or an unidentified IDAW format, did not
+  specify prefetch mode, or the scsw specified a function other than the
+  start function.
+
+``-EIO``
+  A request was issued while the device was not in a state ready to accept
+  requests, or an internal error occurred.
+
+``-EBUSY``
+  The subchannel was status pending or busy, or a request is already active.
+
+``-EAGAIN``
+  A request was being processed, and the caller should retry.
+
+``-EACCES``
+  The channel path(s) used for the I/O were found to be not operational.
+
+``-ENODEV``
+  The device was found to be not operational.
+
+``-EINVAL``
+  The orb specified a chain longer than 255 ccws, or an internal error
+  occurred.
 
 This region is always available.
 
@@ -231,6 +260,29 @@ This region is exposed via region type VFIO_REGION_SUBTYPE_CCW_ASYNC_CMD.
 
 Currently, CLEAR SUBCHANNEL and HALT SUBCHANNEL use this region.
 
+command specifies the command to be issued; ret_code stores a return code
+for each access of the region. The following values may occur:
+
+``0``
+  The operation was successful.
+
+``-ENODEV``
+  The device was found to be not operational.
+
+``-EINVAL``
+  A command other than halt or clear was specified.
+
+``-EIO``
+  A request was issued while the device was not in a state ready to accept
+  requests.
+
+``-EAGAIN``
+  A request was being processed, and the caller should retry.
+
+``-EBUSY``
+  The subchannel was status pending or busy while processing a halt request.
+
+
 vfio-ccw operation details
 --------------------------
 
-- 
2.21.1

Re: [PATCH] vfio-ccw: document possible errors

[Eric Farman]

On 4/7/20 7:16 AM, Cornelia Huck wrote:
> Interacting with the I/O and the async regions can yield a number
> of errors, which had been undocumented so far. These are part of
> the api, so remedy that.

(Makes a note to myself, to do the same for the schib/crw regions we're
adding for channel path handling.)

> 
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  Documentation/s390/vfio-ccw.rst | 54 ++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/s390/vfio-ccw.rst b/Documentation/s390/vfio-ccw.rst
> index fca9c4f5bd9c..4538215a362c 100644
> --- a/Documentation/s390/vfio-ccw.rst
> +++ b/Documentation/s390/vfio-ccw.rst
> @@ -210,7 +210,36 @@ Subchannel.
>  
>  irb_area stores the I/O result.
>  
> -ret_code stores a return code for each access of the region.
> +ret_code stores a return code for each access of the region. The following
> +values may occur:
> +
> +``0``
> +  The operation was successful.
> +
> +``-EOPNOTSUPP``
> +  The orb specified transport mode or an unidentified IDAW format, did not
> +  specify prefetch mode, or the scsw specified a function other than the
> +  start function.
> +
> +``-EIO``
> +  A request was issued while the device was not in a state ready to accept
> +  requests, or an internal error occurred.
> +
> +``-EBUSY``
> +  The subchannel was status pending or busy, or a request is already active.
> +
> +``-EAGAIN``
> +  A request was being processed, and the caller should retry.
> +
> +``-EACCES``
> +  The channel path(s) used for the I/O were found to be not operational.
> +
> +``-ENODEV``
> +  The device was found to be not operational.
> +
> +``-EINVAL``
> +  The orb specified a chain longer than 255 ccws, or an internal error
> +  occurred.
>  
>  This region is always available.

Maybe move this little line up between the struct layout and "While
starting an I/O request, orb_area ..." instead of being lost way down here?

But other than that suggestion, everything looks fine.

Reviewed-by: Eric Farman <farman@linux.ibm.com>

>  
> @@ -231,6 +260,29 @@ This region is exposed via region type VFIO_REGION_SUBTYPE_CCW_ASYNC_CMD.
>  
>  Currently, CLEAR SUBCHANNEL and HALT SUBCHANNEL use this region.
>  
> +command specifies the command to be issued; ret_code stores a return code
> +for each access of the region. The following values may occur:
> +
> +``0``
> +  The operation was successful.
> +
> +``-ENODEV``
> +  The device was found to be not operational.
> +
> +``-EINVAL``
> +  A command other than halt or clear was specified.
> +
> +``-EIO``
> +  A request was issued while the device was not in a state ready to accept
> +  requests.
> +
> +``-EAGAIN``
> +  A request was being processed, and the caller should retry.
> +
> +``-EBUSY``
> +  The subchannel was status pending or busy while processing a halt request.
> +
> +
>  vfio-ccw operation details
>  --------------------------
>  
> 

Re: [PATCH] vfio-ccw: document possible errors

[Cornelia Huck]

On Fri, 17 Apr 2020 12:33:18 -0400
Eric Farman <farman@linux.ibm.com> wrote:

> On 4/7/20 7:16 AM, Cornelia Huck wrote:
> > Interacting with the I/O and the async regions can yield a number
> > of errors, which had been undocumented so far. These are part of
> > the api, so remedy that.  
> 
> (Makes a note to myself, to do the same for the schib/crw regions we're
> adding for channel path handling.)

Yes, please :) I plan to merge this today, so you can add a patch on
top.

> 
> > 
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >  Documentation/s390/vfio-ccw.rst | 54 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 53 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/s390/vfio-ccw.rst b/Documentation/s390/vfio-ccw.rst
> > index fca9c4f5bd9c..4538215a362c 100644
> > --- a/Documentation/s390/vfio-ccw.rst
> > +++ b/Documentation/s390/vfio-ccw.rst
> > @@ -210,7 +210,36 @@ Subchannel.
> >  
> >  irb_area stores the I/O result.
> >  
> > -ret_code stores a return code for each access of the region.
> > +ret_code stores a return code for each access of the region. The following
> > +values may occur:
> > +
> > +``0``
> > +  The operation was successful.
> > +
> > +``-EOPNOTSUPP``
> > +  The orb specified transport mode or an unidentified IDAW format, did not
> > +  specify prefetch mode, or the scsw specified a function other than the

'did not specify prefetch mode' needs to be dropped now, will do so
before queuing.

> > +  start function.
> > +
> > +``-EIO``
> > +  A request was issued while the device was not in a state ready to accept
> > +  requests, or an internal error occurred.
> > +
> > +``-EBUSY``
> > +  The subchannel was status pending or busy, or a request is already active.
> > +
> > +``-EAGAIN``
> > +  A request was being processed, and the caller should retry.
> > +
> > +``-EACCES``
> > +  The channel path(s) used for the I/O were found to be not operational.
> > +
> > +``-ENODEV``
> > +  The device was found to be not operational.
> > +
> > +``-EINVAL``
> > +  The orb specified a chain longer than 255 ccws, or an internal error
> > +  occurred.
> >  
> >  This region is always available.  
> 
> Maybe move this little line up between the struct layout and "While
> starting an I/O request, orb_area ..." instead of being lost way down here?

Good idea, that also would match the documentation for the async region.

> 
> But other than that suggestion, everything looks fine.
> 
> Reviewed-by: Eric Farman <farman@linux.ibm.com>

Thanks!

> 
> >  
> > @@ -231,6 +260,29 @@ This region is exposed via region type VFIO_REGION_SUBTYPE_CCW_ASYNC_CMD.
> >  
> >  Currently, CLEAR SUBCHANNEL and HALT SUBCHANNEL use this region.
> >  
> > +command specifies the command to be issued; ret_code stores a return code
> > +for each access of the region. The following values may occur:
> > +
> > +``0``
> > +  The operation was successful.
> > +
> > +``-ENODEV``
> > +  The device was found to be not operational.
> > +
> > +``-EINVAL``
> > +  A command other than halt or clear was specified.
> > +
> > +``-EIO``
> > +  A request was issued while the device was not in a state ready to accept
> > +  requests.
> > +
> > +``-EAGAIN``
> > +  A request was being processed, and the caller should retry.
> > +
> > +``-EBUSY``
> > +  The subchannel was status pending or busy while processing a halt request.
> > +
> > +
> >  vfio-ccw operation details
> >  --------------------------
> >  
> >   
> 

Re: [PATCH] vfio-ccw: document possible errors

[Eric Farman]

On 5/8/20 6:55 AM, Cornelia Huck wrote:
> On Fri, 17 Apr 2020 12:33:18 -0400
> Eric Farman <farman@linux.ibm.com> wrote:
> 
>> On 4/7/20 7:16 AM, Cornelia Huck wrote:
>>> Interacting with the I/O and the async regions can yield a number
>>> of errors, which had been undocumented so far. These are part of
>>> the api, so remedy that.  
>>
>> (Makes a note to myself, to do the same for the schib/crw regions we're
>> adding for channel path handling.)
> 
> Yes, please :) I plan to merge this today, so you can add a patch on
> top.

I finally picked this up and realized that the io and async regions both
document the return codes that would be stored in a field within their
respective regions. The schib/crw regions don't have any such field, so
the only values to be documented are the ones that the .read callback
itself returns. What obvious thing am I missing?

> 
>>
>>>
>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>> ---
>>>  Documentation/s390/vfio-ccw.rst | 54 ++++++++++++++++++++++++++++++++-
>>>  1 file changed, 53 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/s390/vfio-ccw.rst b/Documentation/s390/vfio-ccw.rst
>>> index fca9c4f5bd9c..4538215a362c 100644
>>> --- a/Documentation/s390/vfio-ccw.rst
>>> +++ b/Documentation/s390/vfio-ccw.rst
>>> @@ -210,7 +210,36 @@ Subchannel.
>>>  
>>>  irb_area stores the I/O result.
>>>  
>>> -ret_code stores a return code for each access of the region.
>>> +ret_code stores a return code for each access of the region. The following
>>> +values may occur:
>>> +
>>> +``0``
>>> +  The operation was successful.
>>> +
>>> +``-EOPNOTSUPP``
>>> +  The orb specified transport mode or an unidentified IDAW format, did not
>>> +  specify prefetch mode, or the scsw specified a function other than the
> 
> 'did not specify prefetch mode' needs to be dropped now, will do so
> before queuing.
> 
>>> +  start function.
>>> +
>>> +``-EIO``
>>> +  A request was issued while the device was not in a state ready to accept
>>> +  requests, or an internal error occurred.
>>> +
>>> +``-EBUSY``
>>> +  The subchannel was status pending or busy, or a request is already active.
>>> +
>>> +``-EAGAIN``
>>> +  A request was being processed, and the caller should retry.
>>> +
>>> +``-EACCES``
>>> +  The channel path(s) used for the I/O were found to be not operational.
>>> +
>>> +``-ENODEV``
>>> +  The device was found to be not operational.
>>> +
>>> +``-EINVAL``
>>> +  The orb specified a chain longer than 255 ccws, or an internal error
>>> +  occurred.
>>>  
>>>  This region is always available.  
>>
>> Maybe move this little line up between the struct layout and "While
>> starting an I/O request, orb_area ..." instead of being lost way down here?
> 
> Good idea, that also would match the documentation for the async region.
> 
>>
>> But other than that suggestion, everything looks fine.
>>
>> Reviewed-by: Eric Farman <farman@linux.ibm.com>
> 
> Thanks!
> 
>>
>>>  
>>> @@ -231,6 +260,29 @@ This region is exposed via region type VFIO_REGION_SUBTYPE_CCW_ASYNC_CMD.
>>>  
>>>  Currently, CLEAR SUBCHANNEL and HALT SUBCHANNEL use this region.
>>>  
>>> +command specifies the command to be issued; ret_code stores a return code
>>> +for each access of the region. The following values may occur:
>>> +
>>> +``0``
>>> +  The operation was successful.
>>> +
>>> +``-ENODEV``
>>> +  The device was found to be not operational.
>>> +
>>> +``-EINVAL``
>>> +  A command other than halt or clear was specified.
>>> +
>>> +``-EIO``
>>> +  A request was issued while the device was not in a state ready to accept
>>> +  requests.
>>> +
>>> +``-EAGAIN``
>>> +  A request was being processed, and the caller should retry.
>>> +
>>> +``-EBUSY``
>>> +  The subchannel was status pending or busy while processing a halt request.
>>> +
>>> +
>>>  vfio-ccw operation details
>>>  --------------------------
>>>  
>>>   
>>
> 

Re: [PATCH] vfio-ccw: document possible errors

[Cornelia Huck]

On Tue, 26 May 2020 15:39:22 -0400
Eric Farman <farman@linux.ibm.com> wrote:

> On 5/8/20 6:55 AM, Cornelia Huck wrote:
> > On Fri, 17 Apr 2020 12:33:18 -0400
> > Eric Farman <farman@linux.ibm.com> wrote:
> >   
> >> On 4/7/20 7:16 AM, Cornelia Huck wrote:  
> >>> Interacting with the I/O and the async regions can yield a number
> >>> of errors, which had been undocumented so far. These are part of
> >>> the api, so remedy that.    
> >>
> >> (Makes a note to myself, to do the same for the schib/crw regions we're
> >> adding for channel path handling.)  
> > 
> > Yes, please :) I plan to merge this today, so you can add a patch on
> > top.  
> 
> I finally picked this up and realized that the io and async regions both
> document the return codes that would be stored in a field within their
> respective regions. The schib/crw regions don't have any such field, so
> the only values to be documented are the ones that the .read callback
> itself returns. What obvious thing am I missing?

The fact that you are right :)

No need to do anything, I might have spread my own confusion here ;)

Re: [PATCH] vfio-ccw: document possible errors

[Eric Farman]

On 5/27/20 2:19 AM, Cornelia Huck wrote:
> On Tue, 26 May 2020 15:39:22 -0400
> Eric Farman <farman@linux.ibm.com> wrote:
> 
>> On 5/8/20 6:55 AM, Cornelia Huck wrote:
>>> On Fri, 17 Apr 2020 12:33:18 -0400
>>> Eric Farman <farman@linux.ibm.com> wrote:
>>>   
>>>> On 4/7/20 7:16 AM, Cornelia Huck wrote:  
>>>>> Interacting with the I/O and the async regions can yield a number
>>>>> of errors, which had been undocumented so far. These are part of
>>>>> the api, so remedy that.    
>>>>
>>>> (Makes a note to myself, to do the same for the schib/crw regions we're
>>>> adding for channel path handling.)  
>>>
>>> Yes, please :) I plan to merge this today, so you can add a patch on
>>> top.  
>>
>> I finally picked this up and realized that the io and async regions both
>> document the return codes that would be stored in a field within their
>> respective regions. The schib/crw regions don't have any such field, so
>> the only values to be documented are the ones that the .read callback
>> itself returns. What obvious thing am I missing?
> 
> The fact that you are right :)
> 
> No need to do anything, I might have spread my own confusion here ;)
> 

Oh, good. I was worried vacation was too long/short. :)

Thanks for confirming; will get back to the other things on the list
instead.

Re: [PATCH] vfio-ccw: document possible errors

[Cornelia Huck]

On Tue,  7 Apr 2020 13:16:05 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> Interacting with the I/O and the async regions can yield a number
> of errors, which had been undocumented so far. These are part of
> the api, so remedy that.
> 
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  Documentation/s390/vfio-ccw.rst | 54 ++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)

Queued with the mentioned minor changes.