Re: [PATCH v4 9/9] s390/vfio_ccw: implement a crw lock

[Eric Farman]

On 7/25/26 1:03 PM, Matthew Rosato wrote:
> On 7/25/26 11:27 AM, Eric Farman wrote:
> > Unlike the channel_program struct, which covers synchronous I/O
> > submissions and asynchronous interrupts, the CRW region relies
> > exclusively on asynchronous events coming from hardware.
> >
> > Implement a lock to manage the list of those payloads, to ensure
> > they are read cohesively.
> >
> > Fixes: 3f02cb2fd9d2 ("vfio-ccw: Wire up the CRW irq and CRW region")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Cc: Farhan Ali <alifm@xxxxxxxxxxxxx>
> > Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
>
> ...
>
> > --- a/drivers/s390/cio/vfio_ccw_private.h
> > +++ b/drivers/s390/cio/vfio_ccw_private.h
> > @@ -98,6 +98,8 @@ struct vfio_ccw_parent {
> >    * @cp: channel program for the current I/O operation
> >    * @irb: irb info received from interrupt
> >    * @scsw: scsw info
> > + * @crw_lock: serialization of CRW information
>
> I would re-word this to say 'CRW list', to make it a bit more clear that
> the lock does not protect crw_trigger at this time (as discussed in v3).

Yeah, good call.

> With that:
>
> Reviewed-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>

Thanks,

> > + * @crw: list of Channel Report Word elements
> >    * @io_trigger: eventfd ctx for signaling userspace I/O results
> >    * @crw_trigger: eventfd ctx for signaling userspace CRW information
> >    * @req_trigger: eventfd ctx for signaling userspace to return device
> > @@ -122,6 +124,8 @@ struct vfio_ccw_private {
> >   
> >   	struct irb		irb;
> >   	union scsw		scsw;
> > +
> > +	spinlock_t		crw_lock;
> >   	struct list_head	crw;
> >   
> >   	struct eventfd_ctx	*io_trigger;