Re: [RFC v1 1/1] vfio-ccw: Don't call cp_free if we are processing a channel program

[Cornelia Huck <cohuck@redhat.com>]

On Mon, 24 Jun 2019 12:05:14 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Mon, 24 Jun 2019 11:42:31 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
> > On Fri, 21 Jun 2019 14:34:10 -0400
> > Farhan Ali <alifm@linux.ibm.com> wrote:
> >   
> > > On 06/21/2019 01:40 PM, Eric Farman wrote:  
> > > > 
> > > > 
> > > > On 6/21/19 10:17 AM, Farhan Ali wrote:    
> > > >>
> > > >>
> > > >> On 06/20/2019 04:27 PM, Eric Farman wrote:    
> > > >>>
> > > >>>
> > > >>> On 6/20/19 3:40 PM, Farhan Ali wrote:    

> > > >>>> diff --git a/drivers/s390/cio/vfio_ccw_drv.c
> > > >>>> b/drivers/s390/cio/vfio_ccw_drv.c
> > > >>>> index 66a66ac..61ece3f 100644
> > > >>>> --- a/drivers/s390/cio/vfio_ccw_drv.c
> > > >>>> +++ b/drivers/s390/cio/vfio_ccw_drv.c
> > > >>>> @@ -88,7 +88,7 @@ static void vfio_ccw_sch_io_todo(struct work_struct
> > > >>>> *work)
> > > >>>>                 (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT));
> > > >>>>        if (scsw_is_solicited(&irb->scsw)) {
> > > >>>>            cp_update_scsw(&private->cp, &irb->scsw);    
> > > >>>
> > > >>> As I alluded earlier, do we know this irb is for this cp?  If no, what
> > > >>> does this function end up putting in the scsw?  
> > 
> > Yes, I think this also needs to check whether we have at least a prior
> > start function around. (We use the orb provided by the guest; maybe we
> > should check if that intparm is set in the irb?)  
> 
> Hrm; not so easy as we always set the intparm to the address of the
> subchannel structure... 
> 
> Maybe check if we have have one of the conditions of the large table
> 16-6 and correlate to the ccw address? Or is it enough to check the
> function control? (Don't remember when the hardware resets it.)

Nope, we cannot look at the function control, as csch clears any set
start function bit :( (see "Function Control", pg 16-13)

I think this problem mostly boils down to "csch clears pending status;
therefore, we may only get one interrupt, even though there had been a
start function going on". If we only go with what the hardware gives
us, I don't see a way to distinguish "clear with a prior start" from
"clear only". Maybe we want to track an "issued" status in the cp?