Re: [RFC 1/1] s390/cio: Remove uevent-suppress from css driver

[Halil Pasic <pasic@linux.ibm.com>]

On Tue, 8 Dec 2020 18:30:54 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> > 
> > But, the more i look at this patch and discuss on this, i think this is 
> > not complete.
> > i.e as you know, the main reason for this RFC was the the below thread.
> > https://marc.info/?l=linux-s390&m=158591045732735&w=2
> > We are still not solving the problem that was mentioned in that RFD.
> > 
> > There are couple of things which we needs to consider here. With this 
> > patch, the uevents
> > are generated before doing the initialization or before finding the 
> > ccw-device
> > connected. Which means, the udev-rules have to manage with a 
> > non-initialized setup
> > compared to the previous version (Version without this patch). As you 
> > mentioned, the
> > current user-space programs which works with this uevent, especially in 
> > case of vfio-ccw
> > will have a problem.  
> 
> IIUC, we'll get the "normal" ADD uevent when the subchannel device is
> registered (i.e. made visible). For the vfio-ccw case, we want the
> driverctl rule to match in this case, so that the driver override can
> be set before the subchannel device ends up being bound to the I/O
> subchannel driver. So I think that removing the suppression is giving
> us exactly what we want? Modulo any errors in the initialization
> sequence we might currently have in the css bus code, of course.
>

I believe, I'm the originator of these concerns, yet I find my
concern hard to recognize in the comment of Vineeth, so let me
please try to explain this in a different way.

AFAIK the uevent handling is asynchronous with regards to matching and
probing, in a sense that there is no synchronization mechanism that
ensures, the userspace has had the ADD event handled (e.g.
driver_override set_ before the kernel proceeds with matching and
probing of the device. Am I wrong about this?

If I'm, with the suppression gone we end up with race, where userspace
may or may not set driver_override in time.

The man page of driverctl
(https://manpages.debian.org/testing/driverctl/driverctl.8.en.html)
claims that: "driverctl integrates with udev to support overriding driver
selection for both cold- and hotplugged devices from the moment of discovery, ..."
and "The driver overrides created by driverctl are persistent across
system reboots by default."

Writing to the driver_override sysfs attribute does not auto-rebind. So
if we can't ensure being in time to set driver_override for the
subchannel before the io_subchannel driver binds, then the userspace
should handle this situation (by unbind and bind) to ensure the
effectiveness of 'driver override'. I couldn't find that code in
driverctl, and I assume if we had that, driver override would work
without this patch. Conny, does that sound about right?

My argument is purely speculative. I didn't try this out, but trying
stuff out is of limited value with races anyway. Vineeth did you try?
If not, I could check this out myself some time later.
 
> I'm not sure how many rules actually care about events for the
> subchannel device; the ccw device seems like the more helpful device to
> watch out for.

I tend to agree, but the problem with vfio-ccw is that (currently) we
don't have a ccw device in the host, because we pass-through the
subchannel. When we interrogate the subchannel, we do learn if there
is a device and if, what is its devno. If I were to run a system with
vfio-ccw passthrough, I would want to passthrough the subchannel that
talks to the DASD (identified by devno) I need passed through to my
guest.

Regards,
Halil