On Thu, 2022-10-27 at 23:23 +0200, Peter Jin wrote:
Revert the control and flag bits in the subchannel status word in
the SSCH operation fails with non-zero CC (ditto for CSCH and HSCH).
According to POPS, the control and flag bits are only changed if
CSCH, and HSCH return CC 0, and no other action should be taken
In order to simulate that after the fact, the bits need to be
I'm okay to this point...
This change is necessary due to the fact that the pwrite() in vfio-
which triggers the SSCH can fail at any time. Previously, there was
only virtio-ccw, whose do_subchannel_work function was only able to
return CC0. However, once vfio-ccw went into the mix, it has become
necessary to handle errors in code paths that were previously assumed
to always return success.
In our case, we found that in case of pwrite() failure (which was
discovered by strace injection), the subchannel could be stuck in
pending state, which could be problematic if the pwrite() call
CC2. Experimentation shows that the guest tries to retry the SSCH
normal for CC2, but it actually continously fails due to the fact
the subchannel is stuck in start pending state even though no start
function is actually taking place.
...but the two paragraphs above are a bit cumbersome to digest. Maybe
it's just too late in the week for me. What about something like this?
While the do_subchannel_work logic for virtual (virtio) devices will
return condition code 0, passthrough (vfio) devices may encounter
errors from either the host kernel or real hardware that need to be
accounted for after this point. This includes restoring the state of
the Subchannel Status Word to reflect the subchannel, as these bits
would not be set in the event of a non-zero condition code from the
Experimentation has shown that a failure on a START SUBCHANNEL (SSCH)
to a passthrough device would leave the subchannel with the START
PENDING activity control bit set, thus blocking subsequent SSCH
operations in css_do_ssch() until some form of error recovery was
undertaken since no interrupt would be expected.
Signed-off-by: Peter Jin <firstname.lastname@example.org>
We've talked previously about clearing this within the
do_subchannel_work_passthrough routine in order to keep the _virtual
paths untouched, but this seems like a reasonable approach to me.
The commit message is probably fine either way, but as far as the code
Reviewed-by: Eric Farman <email@example.com>