[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling
From: |
Halil Pasic |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling |
Date: |
Wed, 23 Jan 2019 14:06:01 +0100 |
On Wed, 23 Jan 2019 11:34:47 +0100
Cornelia Huck <address@hidden> wrote:
> On Tue, 22 Jan 2019 20:03:31 +0100
> Halil Pasic <address@hidden> wrote:
>
> > On Tue, 22 Jan 2019 18:26:17 +0100
> > Cornelia Huck <address@hidden> wrote:
> >
> > > On Tue, 22 Jan 2019 13:46:12 +0100
> > > Halil Pasic <address@hidden> wrote:
>
> > > > Unsolicited interrupts are another
> > > > piece of cake -- I have no idea how may of those do we get.
> > >
> > > They at least don't have the "free the cp before we got final state"
> > > bug. But I think both are reasons to get away from "use the BUSY state
> > > to ensure the right sequence".
> > >
> >
> > I'm not sure I understand you correctly. I was under the impression that
> > the whole point in having a state machine was to ensure the states are
> > traversed in the right sequence with the right stuff being done on each
> > transition. At least in theory.
>
> Sequence in user space programs, not in the state machine.
>
I'm a bit confused.
> >
> > You've probably figured out that IMHO vfio-ccw is not in a good shape
> > (to put it mildly). I have a hard time reviewing a non-holistic
> > concurrency fix. Please tell sould I get perceived as non-constructive,
> > I will try to cut back on criticism.
>
> I'm afraid this is just confusing me :(
>
> >
> > > > And because
> > > > of this the broken sequencing in userspace could actually be the kernels
> > > > fault.
> > >
> > > Here, I can't follow you at all :(
> > >
> >
> > Should we ever deliver a zeroed out IRB to the userspace, for the next
> > ioinst it would look like we have no status nor FC bit set. That is, the
> > guest could end up with stuff in parallel that was never supposed to
> > be in parallel (i.e. broken sequencing because kernel feeds false
> > information due to race with unsolicited interrupt).
> >
> > Does that help?
>
> Not at all, I'm afraid :( User space programs still need to make sure
> they poke the interfaces in the right order IMO...
>
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.
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).
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.
This in turn could lead to userspace violating the contract, as perceived
by the kernel side.
> 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.)
>
I understand. I guess you will want to send a new version because of the
stuff that got lost in the rebase, or?
Regards,
Halil
- [Qemu-devel] [PATCH v2 0/5] vfio-ccw: support hsch/csch (kernel part), Cornelia Huck, 2019/01/21
- [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Cornelia Huck, 2019/01/21
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Halil Pasic, 2019/01/21
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Cornelia Huck, 2019/01/22
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Halil Pasic, 2019/01/22
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Cornelia Huck, 2019/01/22
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Halil Pasic, 2019/01/22
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Cornelia Huck, 2019/01/22
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Halil Pasic, 2019/01/22
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Cornelia Huck, 2019/01/23
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling,
Halil Pasic <=
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Cornelia Huck, 2019/01/23
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Eric Farman, 2019/01/24
- Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Cornelia Huck, 2019/01/25
Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Halil Pasic, 2019/01/22
Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Pierre Morel, 2019/01/24
Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Cornelia Huck, 2019/01/24
Re: [Qemu-devel] [PATCH v2 2/5] vfio-ccw: concurrent I/O handling, Pierre Morel, 2019/01/24