On Mon, 7 Dec 2020 09:09:48 +0100 Vineeth Vijayan <vneethv@xxxxxxxxxxxxx> wrote: > Hi Cornelia, > > A bit more on this RFC. > I think this is a required change in the CIO layer, especially because there > are lot of validations before we call the probe, which makes sure that we > are not generating the uevent on an invalid (without a ccw-device connected) > subchannel and we dont expect a uevent-storm with the current code-base. > So, in anyway, Removing the suppression in the uevent is a cleaner way > for the > css driver. Agreed. A device being found not operational during actual ccw device setup but not earlier is probably an edge case. > > 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'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. > Second point is, there is no guarantee on the current code that the > uevent generated > will be used by udev-rules of vfio-ccw instead of io-subchannel driver. > This means, we > need to do some more modifications on the udev-rules, which can then > decide which > driver should bind with the subchannel. I think that is the only way to > avoid the > problems we are facing with the driver_override. Looking on my LPAR, it seems the driverctl rule is the second in priority; i.e., it will be called before nearly everything else. This should suffice, I hope? > I would like to get your expert-opinion on the modifications that can be > done on the > vfio-ccw udev-rules to make it sync with the current patch. I think the vfio-ccw specific rules are those we need to worry about the least. Again, from a quick browse on my LPAR, the existing rules seem pretty sane AFAICS. I cannot speak for all the different distros, though :) One thing that probably should be changed together with the removal of the uevent suppression is the de-registration of the subchannel device by the ccw bus. Likely an edge case, but might cause confusion when a subchannel is immediately gone again.
