On Wed, Dec 11, 2019 at 12:58:24AM +0800, Alex Williamson wrote:
On Mon, 9 Dec 2019 21:44:23 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:
Currently, yes, i40e has build dependency on vfio-pci. It's like this, if i40e decides to support SRIOV and compiles in vf related code who depends on vfio-pci, it will also have build dependency on vfio-pci. isn't it natural?
No, this is not natural. There are certainly i40e VF use cases that have no interest in vfio and having dependencies between the two modules is unacceptable. I think you probably want to modularize the i40e vfio support code and then perhaps register a table in vfio-pci that the vfio-pci code can perform a module request when using a compatible device. Just and idea, there might be better options. I will not accept a solution that requires unloading the i40e driver in order to unload the vfio-pci driver. It's inconvenient with just one NIC driver, imagine how poorly that scales.
what about this way: mediate driver registers a module notifier and every time when vfio_pci is loaded, register to vfio_pci its mediate ops? (Just like in below sample code) This way vfio-pci is free to unload and this registering only gives vfio-pci a name of what module to request. After that, in vfio_pci_open(), vfio-pci requests the mediate driver. (or puts the mediate driver when mediate driver does not support mediating the device) in vfio_pci_release(), vfio-pci puts the mediate driver.
static void register_mediate_ops(void) { int (*func)(struct vfio_pci_mediate_ops *ops) = NULL;
func = symbol_get(vfio_pci_register_mediate_ops);
if (func) { func(&igd_dt_ops); symbol_put(vfio_pci_register_mediate_ops); } }
static int igd_module_notify(struct notifier_block *self, unsigned long val, void *data) { struct module *mod = data; int ret = 0;
switch (val) { case MODULE_STATE_LIVE: if (!strcmp(mod->name, "vfio_pci")) register_mediate_ops(); break; case MODULE_STATE_GOING: break; default: break; } return ret; }
static struct notifier_block igd_module_nb = { .notifier_call = igd_module_notify, .priority = 0, };
static int __init igd_dt_init(void) { ... register_mediate_ops(); register_module_notifier(&igd_module_nb); ... return 0; }
No, this is bad. Please look at MODULE_ALIAS() and request_module() as used in the vfio-platform for loading reset driver modules. I think the correct approach is that vfio-pci should perform a request_module() based on the device being probed. Having the mediation provider listening for vfio-pci and registering itself regardless of whether we intend to use it assumes that we will want to use it and assumes that the mediation provider module is already loaded. We should be able to support demand loading of modules that may serve no other purpose than providing this mediation. Thanks, hi Alex Thanks for this message. So is it good to create a separate module as mediation provider driver, and alias its module name to "vfio-pci-mediate-vid-did". Then when vfio-pci probes the device, it requests module of that name ?
I think this would give us an option to have the mediator as a separate module, but not require it. Maybe rather than a request_module(), where if we follow the platform reset example we'd then expect the init code for the module to register into a list, we could do a symbol_request(). AIUI, this would give us a reference to the symbol if the module providing it is already loaded, and request a module (perhaps via an alias) if it's not already load. Thanks,
ok. got it! Thank you :) Yan