Re: [PATCH v1 02/18] vfio/ccw: Fix FSM state if mdev probe fails

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

 



On 6/3/22 3:12 PM, Eric Farman wrote:
On Fri, 2022-06-03 at 09:21 -0400, Matthew Rosato wrote:
On 6/2/22 1:19 PM, Eric Farman wrote:
The FSM is in STANDBY state when arriving in vfio_ccw_mdev_probe(),
and this routine converts it to IDLE as part of its processing.
The error exit sets it to IDLE (again) but clears the private->mdev
pointer.

The FSM should of course be managing the state itself, but the
correct thing for vfio_ccw_mdev_probe() to do would be to put
the state back the way it found it.

The corresponding check of private->mdev in vfio_ccw_sch_io_todo()
can be removed, since the distinction is unnecessary at this point.

Fixes: 3bf1311f351ef ("vfio/ccw: Convert to use
vfio_register_emulated_iommu_dev()")
Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
---
   drivers/s390/cio/vfio_ccw_drv.c | 2 +-
   drivers/s390/cio/vfio_ccw_ops.c | 2 +-
   2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_drv.c
b/drivers/s390/cio/vfio_ccw_drv.c
index 35055eb94115..b18b4582bc8b 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -108,7 +108,7 @@ static void vfio_ccw_sch_io_todo(struct
work_struct *work)
   	 * has finished. Do not overwrite a possible processing
   	 * state if the final interrupt was for HSCH or CSCH.
   	 */
-	if (private->mdev && cp_is_finished)
+	if (cp_is_finished)
   		private->state = VFIO_CCW_STATE_IDLE;

Took me a bit to convince myself this was OK

Me too. :)

, mainly because AFAICT
despite the change below the fsm jumptable would still allow you to
reach this code when in STANDBY.  But, it should only be possible for
an
unsolicited interrupt (e.g. unsolicited implies !cp_is_finished) so
we
would still avoid a STANDBY->IDLE transition on accident.

Maybe work unsolicited interrupt into the comment block above along
with
HSCH/CSCH?

Good idea. How about:

         /*
          * Reset to IDLE only if
processing of a channel program
          * has finished. Do not
overwrite a possible processing
          * state if the interrupt was
unsolicited, or if the final
          * interrupt was for HSCH or CSCH.
*/


Sounds good to me



[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