On Fri, 8 Mar 2019 17:18:22 -0500
Eric Farman <farman@xxxxxxxxxxxxx> wrote:
> On 03/01/2019 04:38 AM, Cornelia Huck wrote:
> > The flow for processing ssch requests can be improved by splitting
> > the BUSY state:
> >
> > - CP_PROCESSING: We reject any user space requests while we are in
> > the process of translating a channel program and submitting it to
> > the hardware. Use -EAGAIN to signal user space that it should
> > retry the request.
> > - CP_PENDING: We have successfully submitted a request with ssch and
> > are now expecting an interrupt. As we can't handle more than one
> > channel program being processed, reject any further requests with
> > -EBUSY. A final interrupt will move us out of this state; this also
> > fixes a latent bug where a non-final interrupt might have freed up
> > a channel program that still was in progress.
> > By making this a separate state, we make it possible to issue a
> > halt or a clear while we're still waiting for the final interrupt
> > for the ssch (in a follow-on patch).
> >
> > It also makes a lot of sense not to preemptively filter out writes to
> > the io_region if we're in an incorrect state: the state machine will
> > handle this correctly.
> >
> > Reviewed-by: Eric Farman <farman@xxxxxxxxxxxxx>
> > Signed-off-by: Cornelia Huck <cohuck@xxxxxxxxxx>
> > ---
> > drivers/s390/cio/vfio_ccw_drv.c | 8 ++++++--
> > drivers/s390/cio/vfio_ccw_fsm.c | 19 ++++++++++++++-----
> > drivers/s390/cio/vfio_ccw_ops.c | 2 --
> > drivers/s390/cio/vfio_ccw_private.h | 3 ++-
> > 4 files changed, 22 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
> > index a10cec0e86eb..0b3b9de45c60 100644
> > --- a/drivers/s390/cio/vfio_ccw_drv.c
> > +++ b/drivers/s390/cio/vfio_ccw_drv.c
> > @@ -72,20 +72,24 @@ static void vfio_ccw_sch_io_todo(struct work_struct *work)
> > {
> > struct vfio_ccw_private *private;
> > struct irb *irb;
> > + bool is_final;
> >
> > private = container_of(work, struct vfio_ccw_private, io_work);
> > irb = &private->irb;
> >
> > + is_final = !(scsw_actl(&irb->scsw) &
> > + (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT));
> > if (scsw_is_solicited(&irb->scsw)) {
> > cp_update_scsw(&private->cp, &irb->scsw);
> > - cp_free(&private->cp);
> > + if (is_final)
> > + cp_free(&private->cp);
> > }
> > memcpy(private->io_region->irb_area, irb, sizeof(*irb));
> >
> > if (private->io_trigger)
> > eventfd_signal(private->io_trigger, 1);
> >
> > - if (private->mdev)
> > + if (private->mdev && is_final)
> > private->state = VFIO_CCW_STATE_IDLE;
> > }
> >
>
> Coincidentally, I did something AWESOME last night that the chunks
> listed above actually fix. I have a large channel program, and when it
> runs my host crashes which isn't nice.
Ouch.
(...)
> Recalling the above changes, I applied JUST the above pieces (not the
> remainder of this patch), and the above channel program works fine.
Thanks for pointing that out... I'll extract a patch with only the
changes above and post it with cc:stable. A channel program submitted
by the guest being able to crash the host is... not good.
