Re: [PATCH v4 2/6] vfio-ccw: rework ssch state handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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. First, the callback:

[  547.821235] Call Trace:
[  547.821236] ([<0000000000000000>]           (null))
[  547.821244]  [<000003ff808d8b4a>] cp_prefetch+0x422/0x750 [vfio_ccw]
[  547.821247]  [<000003ff808d9a90>] fsm_io_request+0x1a0/0x2f0 [vfio_ccw]
[ 547.821250] [<000003ff808d90c4>] vfio_ccw_mdev_write+0xc4/0x1d8 [vfio_ccw]
[  547.821255]  [<0000000000358d8c>] __vfs_write+0x34/0x1a8
[  547.821256]  [<00000000003590d0>] vfs_write+0xa0/0x1d8
[  547.821259]  [<0000000000359572>] ksys_pwrite64+0x8a/0xa8
[  547.821264]  [<0000000000866cf0>] system_call+0x270/0x290
[  547.821264] Last Breaking-Event-Address:
[  547.821267]  [<00000000003325b2>] __kmalloc+0x1c2/0x288

The channel program in question looks like this:

x01 cmd=0b flags=44 count=0006
x02 cmd=02 flags=64 count=07bf
x03 cmd=47 flags=44 count=0010
x04 cmd=49 flags=64 count=049b
x05 cmd=08 flags=00 count=0000 TIC to x04
x06 cmd=0b flags=64 count=0007
x07 cmd=23 flags=44 count=0001
x08 cmd=e4 flags=44 count=0018
x09 cmd=07 flags=44 count=0006
x0a cmd=e4 flags=44 count=0018
x0b cmd=47 flags=64 count=001b
x0c cmd=8e flags=64 count=013a
x0d cmd=9a flags=64 count=0009
x0e cmd=31 flags=4c count=0005
x0f cmd=08 flags=00 count=0000 TIC to x0e
x10 cmd=0d flags=64 count=061b
x11 cmd=07 flags=64 count=000b
x12 cmd=96 flags=64 count=0144
x13 cmd=a9 flags=64 count=0025
x14 cmd=08 flags=00 count=0000 TIC to x13
x15 cmd=05 flags=64 count=0387
x16 cmd=a4 flags=64 count=003e
x17 cmd=e4 flags=44 count=0018
x18 cmd=0b flags=64 count=000a
x19 cmd=96 flags=64 count=0497
x1a cmd=8e flags=64 count=02c3
x1b cmd=29 flags=64 count=01bf
x1c cmd=08 flags=00 count=0000 TIC to x1b
x1d cmd=1b flags=24 count=000a

Debugging it today, I found that we get an intermediate interrupt on CCW 0x0e, and a final interrupt (well, unit check) on CCW 0x11. But because of the intermediate interrupt, rewinding in cp_prefetch() at label out_err fails and we crash. Whoops!

Recalling the above changes, I applied JUST the above pieces (not the remainder of this patch), and the above channel program works fine. Now to figure out why I get a unit check. :)

 - Eric




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux