On 10/1/21 12:00 PM, Alan Stern wrote:
On Fri, Oct 01, 2021 at 11:09:52AM -0700, Dan Williams wrote:
On Fri, Oct 1, 2021 at 9:47 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
On Fri, Oct 01, 2021 at 09:13:54AM -0700, Dan Williams wrote:
Bear with me, and perhaps it's a lack of imagination on my part, but I
don't see how to get to a globally generic "authorized" sysfs ABI
given that USB and Thunderbolt want to do bus specific actions on
authorization toggle events. Certainly a default generic authorized
attribute can be defined for all the other buses that don't have
legacy here, but Thunderbolt will still require support for '2' as an
authorized value, and USB will still want to base probe decisions on
the authorization state of both the usb_device and the usb_interface.
The USB part isn't really accurate (I can't speak for Thunderbolt).
When a usb_device is deauthorized, the device will be unconfigured,
deleting all its interfaces and removing the need for any probe
decisions about them. In other words, the probe decision for a
usb_device or usb_interface depends only on the device's/interface's
own authorization state.
True, the interface binding code does contain a test of the device's
authorization setting. That test is redundant and can be removed.
The actions that USB wants to take on authorization toggle events for
usb_devices are: for authorize, select and install a configuration;
for deauthorize, unconfigure the device. Each of these could be
handled simply enough just by binding/unbinding the device. (There
is some special code for handling wireless USB devices, but wireless
USB is now defunct.)
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.
For thunderbolt driver, it looks much more complicated. Unless you
define some callbacks in struct bus_type, we cannot easily generalize
it (but such callbacks are not recommended because it brings bus specific
operations to core layer).
sysfs_read()
-> simple read
sysfs_write()
-> tb_switch_set_authorized()
-> disapprove_switch()
-> tb_domain_disapprove_switch()
-> tb->cm_ops->disapprove_switch() (product specific call)
-> tb_domain_approve_switch_key()
-> tb->cm_ops->add_switch_key
-> tb->cm_ops->approve_switch() (product specific call)
-> tb_domain_approve_switch()
-> tb->cm_ops->approve_switch() (product specific call)
-> tb_domain_challenge_switch_key()
-> tb->cm_ops->challenge_switch_key()
-> crypto_alloc_shash()
-> crypto_shash_setkey()
-> crypto_shash_digest()
-> tb->cm_ops->approve_switch() (product specific call)
Alan Stern
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer