Re: [RFC PATCH v2 0/4] vfio-ccw: Fix interrupt handling for HALT/CLEAR

[Cornelia Huck <cohuck@redhat.com>]

On Wed, 13 May 2020 16:29:30 +0200
Eric Farman <farman@linux.ibm.com> wrote:

> Hi Conny,
> 
> Back in January, I suggested a small patch [1] to try to clean up
> the handling of HSCH/CSCH interrupts, especially as it relates to
> concurrent SSCH interrupts. Here is a new attempt to address this.
> 
> There was some suggestion earlier about locking the FSM, but I'm not
> seeing any problems with that. Rather, what I'm noticing is that the
> flow between a synchronous START and asynchronous HALT/CLEAR have
> different impacts on the FSM state. Consider:
> 
>     CPU 1                           CPU 2
> 
>     SSCH (set state=CP_PENDING)
>     INTERRUPT (set state=IDLE)
>     CSCH (no change in state)
>                                     SSCH (set state=CP_PENDING)

This is the transition I do not understand. When we get a request via
the I/O area, we go to CP_PROCESSING and start doing translations.
However, we only transition to CP_PENDING if we actually do a SSCH with
cc 0 -- which shouldn't be possible in the flow you outline... unless
it really is something that can be taken care of with locking (state
machine transitioning due to an interrupt without locking, so we go to
IDLE without other parts noticing.)

>     INTERRUPT (set state=IDLE)
>                                     INTERRUPT (set state=IDLE)

But taking a step back (and ignoring your series and the discussion,
sorry about that):

We need to do something (creating a local translation of the guest's
channel program) that does not have any relation to the process in the
architecture at all, but is only something that needs to be done
because of what vfio-ccw is trying to do (issuing a channel program on
behalf of another entity.) Trying to sort that out by poking at actl
and fctl bits does not seem like the best way; especially as keeping
the bits up-to-date via STSCH is an exercise in futility.

What about the following (and yes, I had suggested something vaguely in
that direction before):

- Detach the cp from the subchannel (or better, remove the 1:1
  relationship). By that I mean building the cp as a separately
  allocated structure (maybe embedding a kref, but that might not be
  needed), and appending it to a list after SSCH with cc=0. Discard it
  if cc!=0.
- Remove the CP_PENDING state. The state is either IDLE after any
  successful SSCH/HSCH/CSCH, or a new state in that case. But no
  special state for SSCH.
- A successful CSCH removes the first queued request, if any.
- A final interrupt removes the first queued request, if any.

Thoughts?