linux-kernel - Re: [PATCH 8/9] vfio/pci: use x86 naming instead of igd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 2 Feb 2021 14:50:17 -0400
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Alex Williamson <alex.williamson@...hat.com>
CC:     Max Gurtovoy <mgurtovoy@...dia.com>,
        Cornelia Huck <cohuck@...hat.com>,
        Matthew Rosato <mjrosato@...ux.ibm.com>, <kvm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <liranl@...dia.com>,
        <oren@...dia.com>, <tzahio@...dia.com>, <leonro@...dia.com>,
        <yarong@...dia.com>, <aviadye@...dia.com>, <shahafs@...dia.com>,
        <artemp@...dia.com>, <kwankhede@...dia.com>, <ACurrid@...dia.com>,
        <gmataev@...dia.com>, <cjia@...dia.com>, <yishaih@...dia.com>,
        <aik@...abs.ru>
Subject: Re: [PATCH 8/9] vfio/pci: use x86 naming instead of igd

On Tue, Feb 02, 2021 at 10:54:55AM -0700, Alex Williamson wrote:

> As noted previously, if we start adding ids for vfio drivers then we
> create conflicts with the native host driver.  We cannot register a
> vfio PCI driver that automatically claims devices.

We can't do that in vfio_pci.ko, but a nvlink_vfio_pci.ko can, just
like the RFC showed with the mlx5 example. The key thing is the module
is not autoloadable and there is no modules.alias data for the PCI
IDs.

The admin must explicitly load the module, just like the admin must
explicitly do "cat > new_id". "modprobe nvlink_vfio_pci" replaces
"newid", and preloading the correct IDs into the module's driver makes
the entire admin experience much more natural and safe.

This could be improved with some simple work in the driver core:

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 2f32f38a11ed0b..dc3b088ad44d69 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -828,6 +828,9 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
 	bool async_allowed;
 	int ret;

+	if (drv->flags & DRIVER_EXPLICIT_BIND_ONLY)
+		continue;
+
 	ret = driver_match_device(drv, dev);
 	if (ret == 0) {
 		/* no match */

Thus the match table could be properly specified, but only explicit
manual bind would attach the driver. This would cleanly resolve the
duplicate ID problem, and we could even set a wildcard PCI match table
for vfio_pci and eliminate the new_id part of the sequence.

However, I'd prefer to split any driver core work from VFIO parts - so
I'd propose starting by splitting to vfio_pci_core.ko, vfio_pci.ko,
nvlink_vfio_pci.ko, and igd_vfio_pci.ko working as above.

For uAPI compatability vfio_pci.ko would need some
request_module()/symbol_get() trick to pass control over to the device
specific module.

Jason