On Thu, 2021-04-15 at 12:51 +0200, Cornelia Huck wrote:
> On Tue, 13 Apr 2021 20:24:08 +0200
> Eric Farman <farman@xxxxxxxxxxxxx> wrote:
>
> > When an interrupt is received via the IRQ, the bulk of the work
> > is stacked on a workqueue for later processing. Which means that
> > a concurrent START or HALT/CLEAR operation (via the async_region)
> > will race with this process and require some serialization.
> >
> > Once we have all our locks acquired, let's just look to see if
> > we're
> > in a window where the process has been started from the IRQ, but
> > not
> > yet picked up by vfio-ccw to clean up an I/O. If there is, mark the
> > request as BUSY so it can be redriven.
> >
> > Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
> > ---
> > drivers/s390/cio/vfio_ccw_fsm.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/s390/cio/vfio_ccw_fsm.c
> > b/drivers/s390/cio/vfio_ccw_fsm.c
> > index 23e61aa638e4..92d638f10b27 100644
> > --- a/drivers/s390/cio/vfio_ccw_fsm.c
> > +++ b/drivers/s390/cio/vfio_ccw_fsm.c
> > @@ -28,6 +28,11 @@ static int fsm_io_helper(struct vfio_ccw_private
> > *private)
> >
> > spin_lock_irqsave(sch->lock, flags);
> >
> > + if (work_pending(&private->io_work)) {
> > + ret = -EBUSY;
> > + goto out;
> > + }
> > +
> > orb = cp_get_orb(&private->cp, (u32)(addr_t)sch, sch->lpm);
> > if (!orb) {
> > ret = -EIO;
>
> I'm wondering what condition we can consider this situation
> equivalent
> to. I'd say that the virtual version of the subchannel is basically
> status pending already, even though userspace may not have retrieved
> that information yet; so probably cc 1?
Yes, I guess cc1 is a more natural fit, since there is status pending
rather than an active start/halt/clear that would expect get the cc2.
>
> Following the code path further along, it seems we return -EBUSY both
> for cc 1 and cc 2 conditions we receive from the device (i.e. they
> are
> not distinguishable from userspace).
Yeah. :/
> I don't think we can change that,
> as it is an existing API (QEMU maps -EBUSY to cc 2.) So this change
> looks fine so far.
>
> I'm wondering what we should do for hsch. We probably want to return
> -EBUSY for a pending condition as well, if I read the PoP
> correctly...
Ah, yes... I agree that to maintain parity with ssch and pops, the
same cc1/-EBUSY would be applicable here. Will make that change in next
version.
> the only problem is that QEMU seems to match everything to 0; but
> that
> is arguably not the kernel's problem.
>
> For clear, we obviously don't have busy conditions. Should we clean
> up
> any pending conditions?
By doing anything other than issuing the csch to the subchannel? I
don't think so, that should be more than enough to get the css and
vfio-ccw in sync with each other.
>
> [It feels like we have discussed this before, but any information has
> vanished from my cache :/]
>
It has vanished from mine too, and looking over the old threads and
notes doesn't page anything useful in, so here we are. Sorry. :(
Eric
