Add support for unmanaged SR-IOV
Subsystems (7)
v6 cover Add support for unmanaged SR-IOV
This series is meant to add support for SR-IOV on devices when the VFs are not managed by the kernel. Examples of recent patches attempting to do this include:
virto - https://patchwork.kernel.org/patch/10241225/
pci-stub - https://patchwork.kernel.org/patch/10109935/
vfio - https://patchwork.kernel.org/patch/10103353/
uio - https://patchwork.kernel.org/patch/9974031/
Since this is quickly blowing up into a multi-driver problem it is probably best to implement this solution as generically as possible.
This series is an attempt to do that. What we do with this patch set is provide a generic framework to enable SR-IOV in the case that the PF driver doesn't support managing the VFs itself.
I based my patch set originally on the patch by Mark Rustad but there isn't much left after going through and cleaning out the bits that were no longer needed, and after incorporating the feedback from David Miller. At this point the only items to be fully reused was his patch description which is now present in patch 3 of the set.
This solution is limited in scope to just adding support for devices that provide no functionality for SR-IOV other than allocating the VFs by calling pci_enable_sriov. Previous sets had included patches for VFIO, but for now I am dropping that as the scope of that work is larger then I think I can take on at this time.
v2: Reduced scope back to just virtio_pci and vfio-pci
Broke into 3 patch set from single patch
Changed autoprobe behavior to always set when num_vfs is set non-zero
v3: Updated Documentation to clarify when sriov_unmanaged_autoprobe is used
Wrapped vfio_pci_sriov_configure to fix build errors w/o SR-IOV in kernel
v4: Dropped vfio-pci patch
Added ena and nvme to drivers now using pci_sriov_configure_unmanaged Dropped pci_disable_sriov call in virtio_pci to be consistent with ena
v5: Dropped sriov_unmanaged_autoprobe and pci_sriov_conifgure_unmanaged
Added new patch that enables pci_sriov_configure_simple Updated drivers to use pci_sriov_configure_simple
v6: Defined pci_sriov_configure_simple as NULL when SR-IOV is not enabled
Updated drivers to drop "#ifdef" checks for IOV
Added pci-pf-stub as place for PF-only drivers to add support
---
Alexander Duyck (5):
pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
ena: Migrate over to unmanaged SR-IOV support
nvme: Migrate over to unmanaged SR-IOV support
pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs
drivers/net/ethernet/amazon/ena/ena_netdev.c | 28 ------------ drivers/nvme/host/pci.c | 20 --------- drivers/pci/Kconfig | 12 +++++ drivers/pci/Makefile | 2 + drivers/pci/iov.c | 32 ++++++++++++++ drivers/pci/pci-pf-stub.c | 60 ++++++++++++++++++++++++++ drivers/virtio/virtio_pci_common.c | 1 include/linux/pci.h | 3 + include/linux/pci_ids.h | 2 + 9 files changed, 114 insertions(+), 46 deletions(-) create mode 100644 drivers/pci/pci-pf-stub.c
--
PATCH v6 1/5 pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
From: Alexander Duyck <[email protected]>
This patch adds a common configuration function called pci_sriov_configure_simple that will allow for managing VFs on devices where the PF is not capable of managing VF resources.
---
v5: New patch replacing pci_sriov_configure_unmanaged with
pci_sriov_configure_simple
Dropped bits related to autoprobe changes
v6: Defined pci_sriov_configure_simple as NULL if IOV is disabled
drivers/pci/iov.c | 32 ++++++++++++++++++++++++++++++++ include/linux/pci.h | 3 +++ 2 files changed, 35 insertions(+)
drivers/pci/iov.c +32 -0
@@ -807,3 +807,35 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev)return dev->sriov->total_VFs; }EXPORT_SYMBOL_GPL (pci_sriov_get_totalvfs); + +/** +* pci_sriov_configure_simple - helper to configure unmanaged SR-IOV +* @dev: the PCI device +* @nr_virtfn: number of virtual functions to enable, 0 to disable +* +* Used to provide generic enable/disable SR-IOV option for devices +* that do not manage the VFs generated by their driver +*/ +int pci_sriov_configure_simple (struct pci_dev *dev,int nr_virtfn) +{ +int err = -EINVAL ; + +might_sleep (); + +if (!dev->is_physfn) +return -ENODEV ; + +if (pci_vfs_assigned (dev)) { +pci_warn (dev, +"Cannot modify SR-IOV while VFs are assigned\n" ); + err = -EPERM ; + }else if (!nr_virtfn) { +sriov_disable (dev); + err =0 ; + }else if (!dev->sriov->num_VFs) { + err =sriov_enable (dev, nr_virtfn); + } + +return err ? err : nr_virtfn; +} +EXPORT_SYMBOL_GPL (pci_sriov_configure_simple);
include/linux/pci.h +3 -0
@@ -1953,6 +1953,7 @@ static inline void pci_mmcfg_late_init(void) { }int pci_vfs_assigned (struct pci_dev *dev);int pci_sriov_set_totalvfs (struct pci_dev *dev,u16 numvfs);int pci_sriov_get_totalvfs (struct pci_dev *dev); +int pci_sriov_configure_simple (struct pci_dev *dev,int nr_virtfn);resource_size_t pci_iov_resource_size (struct pci_dev *dev,int resno);void pci_vf_drivers_autoprobe (struct pci_dev *dev,bool probe);#else
@@ -1980,6 +1981,8 @@ static inline int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs) {return 0 ; }static inline int pci_sriov_get_totalvfs (struct pci_dev *dev) {return 0 ; } +/* since this expected to be used as a function pointer just define as NULL */ +#define pci_sriov_configure_simple NULLstatic inline resource_size_t pci_iov_resource_size (struct pci_dev *dev,int resno) {return 0 ; }static inline void pci_vf_drivers_autoprobe (struct pci_dev *dev,bool probe) { }
Re: [pci PATCH v6 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
Quoted 28 lines · Alexander Duyck
From: Alexander Duyck <[email protected]>
This patch adds a common configuration function called
pci_sriov_configure_simplethat will allow for managing VFs on devices where the PF is not capable of managing VF resources.Signed-off-by Alexander Duyck <[email protected]>---
v5: New patch replacing
pci_sriov_configure_unmanagedwith
pci_sriov_configure_simple
Dropped bits related to autoprobe changes
v6: Definedpci_sriov_configure_simpleas NULL if IOV is disableddrivers/pci/iov.c | 32 ++++++++++++++++++++++++++++++++ include/linux/pci.h | 3 +++ 2 files changed, 35 insertions(+)drivers/pci/iov.c +2
-0@@ -807,3 +807,35 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev)return dev->sriov->total_VFs; }EXPORT_SYMBOL_GPL (pci_sriov_get_totalvfs); + +/**
drivers/pci/iov.c int pci_sriov_get_totalvfs(struct pci_dev *dev) · line 821
Quoted 10 lines
drivers/pci/iov.c +10
-0+* pci_sriov_configure_simple - helper to configure unmanaged SR-IOV +* @dev: the PCI device +* @nr_virtfn: number of virtual functions to enable, 0 to disable +* +* Used to provide generic enable/disable SR-IOV option for devices +* that do not manage the VFs generated by their driver +*/ +int pci_sriov_configure_simple (struct pci_dev *dev,int nr_virtfn) +{ +int err = -EINVAL ;
This assignment seems like it is never used..
drivers/pci/iov.c int pci_sriov_get_totalvfs(struct pci_dev *dev) · line 831
Quoted 10 lines
drivers/pci/iov.c +10
-0+ +might_sleep (); + +if (!dev->is_physfn) +return -ENODEV ; + +if (pci_vfs_assigned (dev)) { +pci_warn (dev, +"Cannot modify SR-IOV while VFs are assigned\n" ); + err = -EPERM ;
Why not:
if (pci_vfs_assigned(dev)) {
pci_warn(dev,
"Cannot modify SR-IOV while VFs are assigned\n");
return -EPERM;
}Otherwise looks good:
Re: [pci PATCH v6 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
Quoted 33 lines · Christoph Hellwig
Quoted 32 lines · Alexander Duyck
From: Alexander Duyck <[email protected]>
This patch adds a common configuration function called
pci_sriov_configure_simplethat will allow for managing VFs on devices where the PF is not capable of managing VF resources.Signed-off-by Alexander Duyck <[email protected]>---
v5: New patch replacing
pci_sriov_configure_unmanagedwith
pci_sriov_configure_simple
Dropped bits related to autoprobe changes
v6: Definedpci_sriov_configure_simpleas NULL if IOV is disableddrivers/pci/iov.c | 32 ++++++++++++++++++++++++++++++++ include/linux/pci.h | 3 +++ 2 files changed, 35 insertions(+)drivers/pci/iov.c +6
-0@@ -807,3 +807,35 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev)return dev->sriov->total_VFs; }EXPORT_SYMBOL_GPL (pci_sriov_get_totalvfs); + +/** +* pci_sriov_configure_simple - helper to configure unmanaged SR-IOV +* @dev: the PCI device +* @nr_virtfn: number of virtual functions to enable, 0 to disable +*
Quoted 8 lines
drivers/pci/iov.c int pci_sriov_get_totalvfs(struct pci_dev *dev) · line 821
Quoted 6 lines
drivers/pci/iov.c +6
-0+* Used to provide generic enable/disable SR-IOV option for devices +* that do not manage the VFs generated by their driver +*/ +int pci_sriov_configure_simple (struct pci_dev *dev,int nr_virtfn) +{ +int err = -EINVAL ;This assignment seems like it is never used..
It applies in the case where we are setting a number of VFs when VFs are already allocated. It was needed since the last statement ended in an if.
Quoted 13 lines
drivers/pci/iov.c int pci_sriov_get_totalvfs(struct pci_dev *dev) · line 831
Quoted 10 lines
drivers/pci/iov.c +10
-0+ +might_sleep (); + +if (!dev->is_physfn) +return -ENODEV ; + +if (pci_vfs_assigned (dev)) { +pci_warn (dev, +"Cannot modify SR-IOV while VFs are assigned\n" ); + err = -EPERM ;Why not:
Quoted 5 lines
if (pci_vfs_assigned(dev)) {
pci_warn(dev,
"Cannot modify SR-IOV while VFs are assigned\n");
return -EPERM;
}Symmetry. I had this as one of 3 blocks and just did it this way to be consistent with the next two statements. I could probably just return from each if block if that is preferred.
Quoted 3 lines
Otherwise looks good:
Reviewed-by Christoph Hellwig <[email protected]>
For v7 I can probably drop the err parameter and just return the necessary error values directly.
PATCH v6 2/5 virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
Hardware-realized virtio_pci devices can implement SR-IOV, so this patch enables its use. The device in question is an upcoming Intel NIC that implements both a virtio_net PF and virtio_net VFs. These are hardware realizations of what has been up to now been a software interface.
The device in question has the following 4-part PCI IDs:
PF: vendor: 1af4 device: 1041 subvendor: 8086 subdevice: 15fe
VF: vendor: 1af4 device: 1041 subvendor: 8086 subdevice: 05fe
The patch currently needs no check for device ID, because the callback will never be made for devices that do not assert the capability or when run on a platform incapable of SR-IOV.
One reason for this patch is because the hardware requires the vendor ID of a VF to be the same as the vendor ID of the PF that created it. So it seemed logical to simply have a fully-functioning virtio_net PF create the VFs. This patch makes that possible.
---
v4: Dropped call to pci_disable_sriov in virtio_pci_remove function
v5: Replaced call to pci_sriov_configure_unmanaged with
pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition
drivers/virtio/virtio_pci_common.c | 1 + 1 file changed, 1 insertion(+)
drivers/virtio/virtio_pci_common.c +1 -0
@@ -596,6 +596,7 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)#ifdef CONFIG_PM_SLEEP .driver.pm = &virtio_pci_pm_ops, #endif + .sriov_configure = pci_sriov_configure_simple, };module_pci_driver (virtio_pci_driver);
Re: [pci PATCH v6 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
Looks good,
PATCH v6 3/5 ena: Migrate over to unmanaged SR-IOV support
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
Instead of implementing our own version of a SR-IOV configuration stub in the ena driver we can just reuse the existing pci_sriov_configure_simple function.
---
v5: Replaced call to pci_sriov_configure_unmanaged with
pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition
drivers/net/ethernet/amazon/ena/ena_netdev.c | 28 +------------------------- 1 file changed, 1 insertion(+), 27 deletions(-)
drivers/net/ethernet/amazon/ena/ena_netdev.c +1 -27
@@ -3385,32 +3385,6 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent) }/*****************************************************************************/ -static int ena_sriov_configure (struct pci_dev *dev,int numvfs) -{ -int rc; - -if (numvfs >0 ) { - rc =pci_enable_sriov (dev, numvfs); -if (rc !=0 ) { -dev_err (&dev->dev, -"pci_enable_sriov failed to enable: %d vfs with the error: %d\n" , - numvfs, rc); -return rc; - } - -return numvfs; - } - -if (numvfs ==0 ) { -pci_disable_sriov (dev); -return 0 ; - } - -return -EINVAL ; -} - -/*****************************************************************************/ -/*****************************************************************************/ /* ena_remove - Device Removal Routine * @pdev: PCI device information struct
@@ -3525,7 +3499,7 @@ static int ena_resume(struct pci_dev *pdev) .suspend = ena_suspend, .resume = ena_resume, #endif - .sriov_configure = ena_sriov_configure, + .sriov_configure = pci_sriov_configure_simple, };static int __init ena_init (void )
PATCH v6 4/5 nvme: Migrate over to unmanaged SR-IOV support
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
Instead of implementing our own version of a SR-IOV configuration stub in the nvme driver we can just reuse the existing pci_sriov_configure_simple function.
---
v5: Replaced call to pci_sriov_configure_unmanaged with
pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition
drivers/nvme/host/pci.c | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-)
drivers/nvme/host/pci.c +1 -19
@@ -2580,24 +2580,6 @@ static void nvme_remove(struct pci_dev *pdev)nvme_put_ctrl (&dev->ctrl); } -static int nvme_pci_sriov_configure (struct pci_dev *pdev,int numvfs) -{ -int ret =0 ; - -if (numvfs ==0 ) { -if (pci_vfs_assigned (pdev)) { -dev_warn (&pdev->dev, -"Cannot disable SR-IOV VFs while assigned\n" ); -return -EPERM ; - } -pci_disable_sriov (pdev); -return 0 ; - } - - ret =pci_enable_sriov (pdev, numvfs); -return ret ? ret : numvfs; -} -#ifdef CONFIG_PM_SLEEP static int nvme_suspend (struct device *dev) {
@@ -2716,7 +2698,7 @@ static void nvme_error_resume(struct pci_dev *pdev) .driver = { .pm = &nvme_dev_pm_ops, }, - .sriov_configure = nvme_pci_sriov_configure, + .sriov_configure = pci_sriov_configure_simple, .err_handler = &nvme_err_handler, };
Re: [pci PATCH v6 4/5] nvme: Migrate over to unmanaged SR-IOV support
Looks good,
PATCH v6 5/5 pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
Add a new driver called "pci-pf-stub" to act as a "white-list" for PF devices that provide no other functionality other then acting as a means of allocating a set of VFs. For now I only have one example ID provided by Amazon in terms of devices that require this functionality. The general idea is that in the future we will see other devices added as vendors come up with devices where the PF is more or less just a lightweight shim used to allocate VFs.
---
v6: New driver to address concerns about Amazon devices left unsupported
drivers/pci/Kconfig | 12 +++++++++ drivers/pci/Makefile | 2 ++ drivers/pci/pci-pf-stub.c | 60 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci_ids.h | 2 ++ 4 files changed, 76 insertions(+) create mode 100644 drivers/pci/pci-pf-stub.c
drivers/pci/Kconfig +12 -0
@@ -71,6 +71,18 @@ config PCI_STUB When in doubt, say N. +config PCI_PF_STUB + tristate "PCI PF Stub driver" + depends on PCI + depends on PCI_IOV + help + Say Y or M here if you want to enable support for devices that + require SR-IOV support, while at the same time the PF itself is + not providing any actual services on the host itself such as + storage or networking. + + When in doubt, say N. + config XEN_PCIDEV_FRONTEND tristate "Xen PCI Frontend" depends on PCI && X86 && XEN
drivers/pci/Makefile +2 -0
@@ -43,6 +43,8 @@ obj-$(CONFIG_PCI_SYSCALL) += syscall.o obj-$(CONFIG_PCI_STUB) += pci-stub.o +obj-$(CONFIG_PCI_PF_STUB) += pci-pf-stub.o + obj-$(CONFIG_PCI_ECAM) += ecam.o obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
drivers/pci/pci-pf-stub.c new file +60 -0
60 lines not on this page · raw message
include/linux/pci_ids.h +2 -0
@@ -2548,6 +2548,8 @@#define PCI_VENDOR_ID_CIRCUITCO 0x1cc8#define PCI_SUBSYSTEM_ID_CIRCUITCO_MINNOWBOARD 0x0001 +#define PCI_VENDOR_ID_AMAZON 0x1d0f +#define PCI_VENDOR_ID_TEKRAM 0x1de1#define PCI_DEVICE_ID_TEKRAM_DC290 0xdc29
Re: [pci PATCH v6 5/5] pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs
Quoted 5 lines
drivers/pci/pci-pf-stub.c new file +5
-0+ +/** +* pci_pf_stub_white_list - White list of devices to bind pci-pf-stub onto +* +* This table provides the list of IDs this driver is supposed to bind
drivers/pci/pci-pf-stub.c line 25
Quoted 10 lines
drivers/pci/pci-pf-stub.c new file +10
-0+* onto. You could think of this as a list of "quirked" devices where we +* are adding support for SR-IOV here since there are no other drivers +* that they would be running under. +* +* Layout of the table below is as follows: +* { Vendor ID, Device ID, +* SubVendor ID, SubDevice ID, +* Class, Class Mask, +* private data (not used) } +*/
No need to document the PCI device table format in a random driver.
Otherwise looks fine:
Re: [pci PATCH v6 5/5] pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs
Quoted 9 lines · Christoph Hellwig
Quoted 9 lines
drivers/pci/pci-pf-stub.c new file +9
-0+ +/** +* pci_pf_stub_white_list - White list of devices to bind pci-pf-stub onto +* +* This table provides the list of IDs this driver is supposed to bind +* onto. You could think of this as a list of "quirked" devices where we +* are adding support for SR-IOV here since there are no other drivers +* that they would be running under. +*
Quoted 8 lines
Quoted 6 lines
+ * Layout of the table below is as follows:
+ * { Vendor ID, Device ID,+ * SubVendor ID, SubDevice ID, + * Class, Class Mask, + * private data (not used) }+ */
No need to document the PCI device table format in a random driver.
I realize this probably isn't necessary, but I wanted to document it to make it clear in terms of layout so that it is't confused with the quirk structure layout since this has been referred to as a "quirk" driver a few times.
Quoted 3 lines
Otherwise looks fine:
Reviewed-by Christoph Hellwig <[email protected]>