Re: [cdi-devel] Proposed Dynamic Driver Loading System

[Rich Edelman <redelman@xxxxxxxxx>]

On Sat, Jan 2, 2010 at 5:46 AM, Kevin Wolf <kevin@xxxxxxxxxx> wrote:
> On Sat, Jan 02, 2010 at 04:58:39PM +1000, Matthew Iselin wrote:
>> The idea is that the OS stores a map of device identifiers (at this stage the
>> proposal considers the use of PCI vendor and device codes) which detail the
>> name of a driver which is to be loaded if the given device is detected. This
>> list may be OS-specific, or there may be a generic list provided as part of
>> CDI. Either way, the OS is open to interpret the driver name as it sees fit:
>> for example, appending ".o" in the case of Pedigree.
>>
>> This map might look something like this:
>>
>> /** OS-specific map of PCI identifiers -> driver names */
>>
>> struct cdi_os_pci_driver_map = {
>>     0x8086, 0x1230, "piix", // Vendor, Device, Driver name
>>     ...
>> };
>
> This would be an additional top-level C file, right? I don't think it's
> a good idea to manually create it and keep it in sync (especially
> considering that many OSes don't use all drivers), but generating such a
> file looks fine to me.

I also agree that such a file should be generated. It'd not be too
hard to generate this list just from the drivers that were compiled,
rather than containing all the drivers.

>> I would like for this map to be implemented as part of CDI, in order to ensure
>> that the list is kept up-to-date with drivers as they are written for CDI.
>> Keeping the list in CDI also decreases the amount of work required to port CDI
>> to a new OS and take advantage of these kind of features.
>>
>> In this early draft of the concept, there is no support for non-PCI devices.
>> Things such as USB with each device having a class code (eg, mass storage, or
>> human interface device) may need their own list. This would be logical as the
>> method for loading drivers dynamically upon a hotplug or similar event could
>> be quite different to that of detecting static PCI devices at boot-time. Hot
>> plug PCI devices are considered to be out of the scope of this proposal.
>
> No, we need to do it right. And this definitely means considering more
> than coldplugged PCI devices. So what I would like too see in drivers is
> something like this:
>
> static struct cdi_bus_data* supported_devices[] = {
>    CDI_PCI_VENDOR_DEVICE(0x8086, 0x1230),
> };
>
> static struct cdi_driver piix_driver = {
>    .name = "piix",
>    .devices = &supported_devices,
>    ...
> }
>
> Accordingly, I would change the generated file to contain cdi_bus_data
> structs instead of a fixed vendor ID/device ID/name schema.

I'm in agreement here as well. We need to consider PCI, USB, etc all
from the start.
Also, using a separate struct like this would make it easier to scan
through the compiled drivers
and generate the mapping file as described above.

>> At a time defined by the OS implementing CDI, the list of PCI devices will be
>> iterated, and each device will be checked against the known PCI->driver list.
>> Should a driver be found in this list, the driver will be loaded (if not
>> already) and used to initialise the detected device.
>>
>> Now, this brings up a small tangent: it is my belief that driver loading could
>> be added to the CDI interface, as something like:
>>
>> /* Returns -1 if the driver could not be loaded, zero otherwise */
>> int cdi_load_driver(const char *);

I think this prototype is all wrong. Whatever it is that wants to load
another driver needs to pass a structure that describes the device a
driver is needed for. Driver's shouldn't be expected to know the name
of the driver they need for another device. Sure, they can get this
from the mapping file discussed previously, but I think that's wrong
as well. Such a file needs to really be internal to the CDI core, and
not something used by the drivers (only contributed to by the
drivers).

I've got some other issues with this as well, though...

>> However, this may be difficult to justify. After all, how many actual drivers
>> really need to load another driver? The alternative is to have the OS perform
>> driver loading itself, which effectively moves most of this subsystem to the
>> OS side (making this hardly related to CDI at all :) )
>
> Right, I'm pretty sure that it's hard to justify. Maybe I'm missing
> something, so let me ask some questions: Which driver is supposed to
> load the right other drivers? Does that mean that automatic loading is
> only done by other drivers or would the OS still load some by itself?
> And what is the benefit of having cdi_load_driver()? Can we share any
> more code with it? In fact, the implementation would still be OS
> specific.
>
> I think we're slowly crossing to border to device initialisation here,
> which is what I've been thinking about a lot. I agree that drivers need
> some way to launch other drivers - however they should do so implicitly
> rather that explicitly asking for a driver name. To be precise, what
> drivers want to do is to say "Hey CDI, I can provide this and that
> device" and the CDI implementation would load the driver and call
> init_device() there.

I've been thinking in terms of bus drivers here. At first I was
thinking bus drivers would really be the only drivers that would use
cdi_load_driver(). They would scan their bus and then tell CDI to load
drivers for any devices found. I talked to Kevin on IRC about this
though, and we pretty quickly agreed that bus drivers shouldn't load
other drivers. This is just a hotplugging step, really. Bus drivers
should just register their child devices with CDI. What is done next
is left entirely up to the CDI implementation. The event count be
ignored completely, it could automatically locate and load the proper
driver, or it could launch a userspace program to notify the user a
new device was detected. Whatever is done would be implementation
specific. We just need to make sure we provide the opportunity for CDI
to support hotplugging.

Of course, all this relies on bus drivers. It also means we need to be
able to say "stop the driver for device X" and "unregister device X".
Hotplugging isn't only about plugging in new devices, it's also about
correctly handling when a device suddenly disappears.

> Personally, I tend to consider coldplugging some kind of hotplugging
> that incidentally happens at driver startup.

+1. coldplugging is really nothing other than a device that was
hotplugged at boot time. :)

Rich