On Tue, Oct 05, 2021 at 03:33:29PM -0700, Dan Williams wrote: > On Sun, Oct 3, 2021 at 10:16 PM Mika Westerberg > <mika.westerberg@xxxxxxxxxxxxxxx> wrote: > > > > Hi, > > > > On Fri, Oct 01, 2021 at 12:57:18PM -0700, Dan Williams wrote: > > > > > Ah, so are you saying that it would be sufficient for USB if the > > > > > generic authorized implementation did something like: > > > > > > > > > > dev->authorized = 1; > > > > > device_attach(dev); > > > > > > > > > > ...for the authorize case, and: > > > > > > > > > > dev->authorize = 0; > > > > > device_release_driver(dev); > > > > > > > > > > ...for the deauthorize case? > > > > > > > > Yes, I think so. But I haven't tried making this change to test and > > > > see what really happens. > > > > > > Sounds like a useful path for this effort to explore. Especially as > > > Greg seems to want the proposed "has_probe_authorization" flag in the > > > bus_type to disappear and make this all generic. It just seems that > > > Thunderbolt would need deeper surgery to move what it does in the > > > authorization toggle path into the probe and remove paths. > > > > > > Mika, do you see a path for Thunderbolt to align its authorization > > > paths behind bus ->probe() ->remove() events similar to what USB might > > > be able to support for a generic authorization path? > > > > In Thunderbolt "authorization" actually means whether there is a PCIe > > tunnel to the device or not. There is no driver bind/unbind happening > > when authorization toggles (well on Thunderbolt bus, there can be on PCI > > bus after the tunnel is established) so I'm not entirely sure how we > > could use the bus ->probe() or ->remove for that to be honest. > > Greg, per your comment: > > "... which was to move the way that busses are allowed to authorize > the devices they wish to control into a generic way instead of being > bus-specific logic." > > We have USB and TB that have already diverged on the ABI here. The USB > behavior is more in line with the "probe authorization" concept, while > TB is about tunnel establishment and not cleanly tied to probe > authorization. So while I see a path to a common authorization > implementation for USB and other buses (per the insight from Alan), TB > needs to retain the ability to record the authorization state as an > enum rather than a bool, and emit a uevent on authorization status > change. > > So how about something like the following that moves the attribute > into the core, but still calls back to TB and USB to perform their > legacy authorization work. This new authorized attribute only shows up > when devices default to not authorized, i.e. when userspace owns the > allow list past critical-boot built-in drivers, or if the bus (USB / > TB) implements ->authorize(). At quick glance, this looks better, but it would be good to see someone test it :) thanks, greg k-h
