Messages in this thread |  | | Date | Thu, 27 Jan 2022 10:04:46 -0500 | Subject | Re: [RFC PATCH] s390: vfio-ap: Register the vfio_ap module for the "ap" parent bus | From | Tony Krowiak <> |
| |
On 12/15/21 18:02, Halil Pasic wrote: > On Wed, 15 Dec 2021 13:51:02 +0100 > Cornelia Huck <cohuck@redhat.com> wrote: > >> On Wed, Dec 15 2021, Thomas Huth <thuth@redhat.com> wrote: >> >>> On 14/12/2021 22.55, Tony Krowiak wrote: >>>> >>>> On 12/13/21 11:11, Cornelia Huck wrote: >>>>> One possibility is simply blocking autoload of the module in userspace by >>>>> default, and only allow it to be loaded automatically when e.g. qemu-kvm >>>>> is installed on the system. This is obviously something that needs to be >>>>> decided by the distros. >>>>> >>>>> (kvm might actually be autoloaded already, so autoloading vfio-ap would >>>>> not really make it worse.) >>>> Of the vfio_ccw module is automatically loaded, then the kvm >>>> module will also get loaded. I startup up a RHEL8.3 system and >>>> sure enough, the vfio_ccw module is loaded along with the >>>> kvm, vfio and mdev modules. If this is true for all distros, then >>>> it wouldn't make much difference if the vfio_ap module is >>>> autoloaded too. >>> I think I don't mind too much if we auto-load vfio-ap or not - but I think >>> we should make it consistent with vfio-ccw. So either auto-load both modules >>> (if the corresponding devices are available), or remove the >>> MODULE_DEVICE_TABLE() entries from both modules? >> I think we really need to take a step back and think about the purpose >> of adding a MODULE_DEVICE_TABLE()... basically, it declares which types >> of devices on a certain bus a driver supports, in a way that can be >> consumed by userspace (after file2alias.c worked on it). > I did a quick search to locate where this semantic was codified. But > I didn't find the place neither Documentation/ nor in the header where > MODULE_DEVICE_TABLE is defined. > >> Userspace typically uses this to match devices it is notified about to >> drivers that could possibly drive those devices. In general, the >> assumption is that you will want to have the drivers for your devices >> loaded. In some cases (drivers only used in special cases, like here), >> it might be a better idea to autoload the drivers only under certain >> circumstances (e.g. if you know you're going to run KVM guests). > Does RHEL do this, or would RHEL do this out of the box? I.e. > would we end up preserving old behavior when this fix hits the distro, > or would the end user end up with kvm and vfio_ap loaded (out of the > box)? > > What would be the mechanism of choice to implement if kvm loaded and > APs present/hotplugged load vfio_ap, otherwise don't in the userspace? > > Sorry I'm not very familiar with this whole modules auto-loading > business, so I may be asking obvious questions. But a quick google > search did not help me. > >> My main point, however, is that we're talking about policy here: whether >> a potentially useful driver should be loaded or not is a decision that >> should be made by userspace. Not providing a MODULE_DEVICE_TABLE does >> not look like the right solution, as it deprives userspace of the >> information to autoload the driver, if it actually wants to do so. >> > I'm sympathetic to this reading of the situation, but I'm not sure > it is as black and white as stated. > > I think the current state of affairs in the vfio_ap module is clearly a > bug. > > One can argue that not auto-loading vfio_ap and kvm per default out of > the box is not a bug. I mean the tooling (chzdev) seems to do fine > without this and just assuming that both kvm and vfio_ap will be needed > just because we have APs seems wrong. > > One of the more important guiding principles of Linux kernel development > is no userspace regressions. And I think suddenly getting vfio_ap and kvm > loaded just because we have AP devices can be thought of as a regression. > > So I'm sympathetic to Harald's view as well. > > Of course there is the solution that the distros should really make sure > the old behavior is preserved, or some smart behavior is introduced. But > regarding smart, I believe "if you have devices that are configured for > vfio_ap pass-through (with chzdev), then the vfio_ap module should get > loaded" is pretty much as smart as it gets. So blacklisting the module > by default in the distros looks like a viable option. If that is what > we want, we should probably ask the distros, because I don't think > it is just obviously their job to figure out that they have to do so. > > Disclaimer: I might be wrong about the current behavior, I didn't verify > my claims > > Also what does vfio-pci do?
From vfio_pci.c:
static const struct pci_device_id vfio_pci_table[] = { { PCI_DRIVER_OVERRIDE_DEVICE_VFIO(PCI_ANY_ID, PCI_ANY_ID) }, /* match all by default */ {} };
MODULE_DEVICE_TABLE(pci, vfio_pci_table);
> As far as I can tell vfio-pci does not > participate in module auto loading just because there are pci devices. > The have some smart override I don't quite understand: > https://patchwork.ozlabs.org/project/linux-pci/patch/20210826103912.128972-11-yishaih@nvidia.com/ > Before, I don't think they had a MODULE_DEVICE_TABLE: > https://elixir.bootlin.com/linux/v5.8.18/source/drivers/vfio/pci/vfio_pci.c > > Regards, > Halil
|  |