On 01/23/2019 08:34 AM, Cornelia Huck wrote:
On Wed, 23 Jan 2019 14:06:01 +0100
Halil Pasic <pasic@xxxxxxxxxxxxx> wrote:
On Wed, 23 Jan 2019 11:34:47 +0100
Cornelia Huck <cohuck@xxxxxxxxxx> wrote:
Yes, one can usually think of interfaces as contracts: both sides need
to keep their end for things to work as intended. Unfortunately the
vfio-ccw iterface is not a very well specified one, and that makes
reasoning about right order so much harder.
That's probably where our disconnect comes from.
I was under the impression that the right ordering is dictated by the
SCSW in userspace. E.g. if there is an FC bit set there userspace is not
ought to issue a SSCH request (write to the io_region). The kernel part
however may say 'userspace read the actual SCSW' by signaling
the io_trigger eventfd. Userspace is supposed to read the IRB from the
region and update it's SCSW.
Now if userspace reads a broken SCSW from the IRB, because of a race
(due to poorly written kernel part -- userspace not at fault), it is
going to make wrong assumptions about currently legal and illegal
operations (ordering).
My understanding of the interface was that writing to the I/O region
triggers a ssch (unless rejected with error) and that reading it just
gets whatever the kernel wrote there the last time it updated its
internal structures. The eventfd simply triggers to say "the region has
been updated with an IRB", not to say "userspace, read this".
Previously I described a scenario where IRB can break without userspace
being at fault (race between unsolicited interrupt -- can happen at any
time -- and a legit io request). I was under the impression we agreed on
this.
There is a bug in there (clearing the cp for non-final interrupts), and
it needs to be fixed. I'm not so sure if the unsolicited interrupt
thing is a bug (beyond that the internal state machine is confused).
This in turn could lead to userspace violating the contract, as perceived
by the kernel side.
Which contract? ;)
Also, I'm not sure if we'd rather get a deferred cc 1?
As I'm encountering dcc=1 quite regularly lately, it's a nice error.
But we don't have a good way of recovering from it, and so my test tends
to go down in a heap quite quickly. This patch set will probably help;
I should really get it applied and try it out.
At this point, I'm mostly confused... I'd prefer to simply fix things
as they come up so that we can finally move forward with the halt/clear
handling (and probably rework the state machine on top of that.)
+1 for fixing things as we go. I hear the complaints about this code
(and probably say them too), but remain convinced that a large rewrite
is unnecessary. Lots of opportunities for improvement, with lots of
willing and motivated participants, means it can only get better!
I understand. I guess you will want to send a new version because of the
stuff that got lost in the rebase, or?
Yes, I'll send a new version; but I'll wait for more feedback for a bit.
I'll try to provide some now. Still digging through the emails marked
"todo" :)
- Eric
