All of lore.kernel.org
 help / color / mirror / Atom feed
* [pci PATCH v7 0/5] Add support for unmanaged SR-IOV
@ 2018-03-15 18:40 ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:40 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

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
v7: Dropped pci_id table explanation from pci-pf-stub driver
    Updated pci_sriov_configure_simple to drop need for err value
    Fixed comment explaining why pci_sriov_configure_simple is NULL

Cc: Mark Rustad <mark.d.rustad@intel.com>
Cc: Maximilian Heyne <mheyne@amazon.de>
Cc: Liang-Min Wang <liang-min.wang@intel.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>

---

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                            |   31 +++++++++++++++
 drivers/pci/pci-pf-stub.c                    |   54 ++++++++++++++++++++++++++
 drivers/virtio/virtio_pci_common.c           |    1 
 include/linux/pci.h                          |    3 +
 include/linux/pci_ids.h                      |    2 +
 9 files changed, 107 insertions(+), 46 deletions(-)
 create mode 100644 drivers/pci/pci-pf-stub.c

--

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [pci PATCH v7 0/5] Add support for unmanaged SR-IOV
@ 2018-03-15 18:40 ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:40 UTC (permalink / raw)


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
v7: Dropped pci_id table explanation from pci-pf-stub driver
    Updated pci_sriov_configure_simple to drop need for err value
    Fixed comment explaining why pci_sriov_configure_simple is NULL

Cc: Mark Rustad <mark.d.rustad at intel.com>
Cc: Maximilian Heyne <mheyne at amazon.de>
Cc: Liang-Min Wang <liang-min.wang at intel.com>
Cc: David Woodhouse <dwmw at amazon.co.uk>

---

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                            |   31 +++++++++++++++
 drivers/pci/pci-pf-stub.c                    |   54 ++++++++++++++++++++++++++
 drivers/virtio/virtio_pci_common.c           |    1 
 include/linux/pci.h                          |    3 +
 include/linux/pci_ids.h                      |    2 +
 9 files changed, 107 insertions(+), 46 deletions(-)
 create mode 100644 drivers/pci/pci-pf-stub.c

--

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 0/5] Add support for unmanaged SR-IOV
@ 2018-03-15 18:40 ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:40 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

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
v7: Dropped pci_id table explanation from pci-pf-stub driver
    Updated pci_sriov_configure_simple to drop need for err value
    Fixed comment explaining why pci_sriov_configure_simple is NULL

Cc: Mark Rustad <mark.d.rustad@intel.com>
Cc: Maximilian Heyne <mheyne@amazon.de>
Cc: Liang-Min Wang <liang-min.wang@intel.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>

---

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                            |   31 +++++++++++++++
 drivers/pci/pci-pf-stub.c                    |   54 ++++++++++++++++++++++++++
 drivers/virtio/virtio_pci_common.c           |    1 
 include/linux/pci.h                          |    3 +
 include/linux/pci_ids.h                      |    2 +
 9 files changed, 107 insertions(+), 46 deletions(-)
 create mode 100644 drivers/pci/pci-pf-stub.c

--

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* [pci PATCH v7 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
  2018-03-15 18:40 ` Alexander Duyck
  (?)
@ 2018-03-15 18:41   ` Alexander Duyck
  -1 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:41 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

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
v7: Updated pci_sriov_configure_simple to drop need for err value
    Fixed comment explaining why pci_sriov_configure_simple is NULL

 drivers/pci/iov.c   |   31 +++++++++++++++++++++++++++++++
 include/linux/pci.h |    3 +++
 2 files changed, 34 insertions(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 677924ae0350..3e0a7fdff3e9 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -807,3 +807,34 @@ 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. Return value
+ * is negative on error, or number of VFs allocated on success.
+ */
+int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn)
+{
+	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");
+		return -EPERM;
+	}
+
+	if (!nr_virtfn) {
+		sriov_disable(dev);
+		return 0;
+	}
+
+	return sriov_enable(dev, nr_virtfn) ? : nr_virtfn;
+}
+EXPORT_SYMBOL_GPL(pci_sriov_configure_simple);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 024a1beda008..992202449829 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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; }
+/* this is expected to be used as a function pointer, just define as NULL */
+#define pci_sriov_configure_simple NULL
 static 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) { }

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [pci PATCH v7 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
@ 2018-03-15 18:41   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:41 UTC (permalink / raw)


From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
---

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
v7: Updated pci_sriov_configure_simple to drop need for err value
    Fixed comment explaining why pci_sriov_configure_simple is NULL

 drivers/pci/iov.c   |   31 +++++++++++++++++++++++++++++++
 include/linux/pci.h |    3 +++
 2 files changed, 34 insertions(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 677924ae0350..3e0a7fdff3e9 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -807,3 +807,34 @@ 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. Return value
+ * is negative on error, or number of VFs allocated on success.
+ */
+int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn)
+{
+	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");
+		return -EPERM;
+	}
+
+	if (!nr_virtfn) {
+		sriov_disable(dev);
+		return 0;
+	}
+
+	return sriov_enable(dev, nr_virtfn) ? : nr_virtfn;
+}
+EXPORT_SYMBOL_GPL(pci_sriov_configure_simple);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 024a1beda008..992202449829 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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; }
+/* this is expected to be used as a function pointer, just define as NULL */
+#define pci_sriov_configure_simple NULL
 static 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) { }

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
@ 2018-03-15 18:41   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:41 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

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
v7: Updated pci_sriov_configure_simple to drop need for err value
    Fixed comment explaining why pci_sriov_configure_simple is NULL

 drivers/pci/iov.c   |   31 +++++++++++++++++++++++++++++++
 include/linux/pci.h |    3 +++
 2 files changed, 34 insertions(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 677924ae0350..3e0a7fdff3e9 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -807,3 +807,34 @@ 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. Return value
+ * is negative on error, or number of VFs allocated on success.
+ */
+int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn)
+{
+	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");
+		return -EPERM;
+	}
+
+	if (!nr_virtfn) {
+		sriov_disable(dev);
+		return 0;
+	}
+
+	return sriov_enable(dev, nr_virtfn) ? : nr_virtfn;
+}
+EXPORT_SYMBOL_GPL(pci_sriov_configure_simple);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 024a1beda008..992202449829 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -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; }
+/* this is expected to be used as a function pointer, just define as NULL */
+#define pci_sriov_configure_simple NULL
 static 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) { }


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-03-15 18:40 ` Alexander Duyck
  (?)
@ 2018-03-15 18:42   ` Alexander Duyck
  -1 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:42 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

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
v7: No code change, added Reviewed-by

 drivers/virtio/virtio_pci_common.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 48d4d1cf1cb6..67a227fd7aa0 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -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);

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-03-15 18:42   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:42 UTC (permalink / raw)


From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Reviewed-by: Christoph Hellwig <hch at lst.de>
Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
---

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
v7: No code change, added Reviewed-by

 drivers/virtio/virtio_pci_common.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 48d4d1cf1cb6..67a227fd7aa0 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -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);

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-03-15 18:42   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:42 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

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
v7: No code change, added Reviewed-by

 drivers/virtio/virtio_pci_common.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 48d4d1cf1cb6..67a227fd7aa0 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -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);


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [pci PATCH v7 3/5] ena: Migrate over to unmanaged SR-IOV support
  2018-03-15 18:40 ` Alexander Duyck
  (?)
@ 2018-03-15 18:43   ` Alexander Duyck
  -1 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:43 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

v5: Replaced call to pci_sriov_configure_unmanaged with
        pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition
v7: No change

 drivers/net/ethernet/amazon/ena/ena_netdev.c |   28 +-------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 6975150d144e..6054deb1e6aa 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -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)

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [pci PATCH v7 3/5] ena: Migrate over to unmanaged SR-IOV support
@ 2018-03-15 18:43   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:43 UTC (permalink / raw)


From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
---

v5: Replaced call to pci_sriov_configure_unmanaged with
        pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition
v7: No change

 drivers/net/ethernet/amazon/ena/ena_netdev.c |   28 +-------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 6975150d144e..6054deb1e6aa 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -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)

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 3/5] ena: Migrate over to unmanaged SR-IOV support
@ 2018-03-15 18:43   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:43 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

v5: Replaced call to pci_sriov_configure_unmanaged with
        pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition
v7: No change

 drivers/net/ethernet/amazon/ena/ena_netdev.c |   28 +-------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 6975150d144e..6054deb1e6aa 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -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)


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [pci PATCH v7 4/5] nvme: Migrate over to unmanaged SR-IOV support
  2018-03-15 18:40 ` Alexander Duyck
  (?)
@ 2018-03-15 18:43   ` Alexander Duyck
  -1 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:43 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

v5: Replaced call to pci_sriov_configure_unmanaged with
        pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition
v7: No code change, added Reviewed-by

 drivers/nvme/host/pci.c |   20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 5933a5c732e8..5e963058882a 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -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,
 };
 

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [pci PATCH v7 4/5] nvme: Migrate over to unmanaged SR-IOV support
@ 2018-03-15 18:43   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:43 UTC (permalink / raw)


From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Reviewed-by: Christoph Hellwig <hch at lst.de>
Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
---

v5: Replaced call to pci_sriov_configure_unmanaged with
        pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition
v7: No code change, added Reviewed-by

 drivers/nvme/host/pci.c |   20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 5933a5c732e8..5e963058882a 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -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,
 };
 

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 4/5] nvme: Migrate over to unmanaged SR-IOV support
@ 2018-03-15 18:43   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:43 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

v5: Replaced call to pci_sriov_configure_unmanaged with
        pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition
v7: No code change, added Reviewed-by

 drivers/nvme/host/pci.c |   20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 5933a5c732e8..5e963058882a 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -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,
 };
 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [pci PATCH v7 5/5] pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs
  2018-03-15 18:40 ` Alexander Duyck
  (?)
@ 2018-03-15 18:44   ` Alexander Duyck
  -1 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:44 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

v6: New driver to address concerns about Amazon devices left unsupported
v7: Dropped pci_id table explanation from pci-pf-stub driver

 drivers/pci/Kconfig       |   12 ++++++++++
 drivers/pci/Makefile      |    2 ++
 drivers/pci/pci-pf-stub.c |   54 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h   |    2 ++
 4 files changed, 70 insertions(+)
 create mode 100644 drivers/pci/pci-pf-stub.c

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 34b56a8f8480..cdef2a2a9bc5 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -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
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 941970936840..4e133d3df403 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -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
diff --git a/drivers/pci/pci-pf-stub.c b/drivers/pci/pci-pf-stub.c
new file mode 100644
index 000000000000..9d5fdf20d485
--- /dev/null
+++ b/drivers/pci/pci-pf-stub.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+/* pci-pf-stub - simple stub driver for PCI SR-IOV PF device
+ *
+ * This driver is meant to act as a "white-list" for devices that provde
+ * SR-IOV functionality while at the same time not actually needing a
+ * driver of their own.
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+
+/**
+ * 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.
+ */
+static const struct pci_device_id pci_pf_stub_white_list[] = {
+	{ PCI_VDEVICE(AMAZON, 0x0053) },
+	/* required last entry */
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(pci, pci_pf_stub_white_list);
+
+static int pci_pf_stub_probe(struct pci_dev *dev,
+			     const struct pci_device_id *id)
+{
+	pci_info(dev, "claimed by pci-pf-stub\n");
+	return 0;
+}
+
+static struct pci_driver pf_stub_driver = {
+	.name			= "pci-pf-stub",
+	.id_table		= pci_pf_stub_white_list,
+	.probe			= pci_pf_stub_probe,
+	.sriov_configure	= pci_sriov_configure_simple,
+};
+
+static int __init pci_pf_stub_init(void)
+{
+	return pci_register_driver(&pf_stub_driver);
+}
+
+static void __exit pci_pf_stub_exit(void)
+{
+	pci_unregister_driver(&pf_stub_driver);
+}
+
+module_init(pci_pf_stub_init);
+module_exit(pci_pf_stub_exit);
+
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index a6b30667a331..b10621896017 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -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
 

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [pci PATCH v7 5/5] pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs
@ 2018-03-15 18:44   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:44 UTC (permalink / raw)


From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
---

v6: New driver to address concerns about Amazon devices left unsupported
v7: Dropped pci_id table explanation from pci-pf-stub driver

 drivers/pci/Kconfig       |   12 ++++++++++
 drivers/pci/Makefile      |    2 ++
 drivers/pci/pci-pf-stub.c |   54 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h   |    2 ++
 4 files changed, 70 insertions(+)
 create mode 100644 drivers/pci/pci-pf-stub.c

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 34b56a8f8480..cdef2a2a9bc5 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -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
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 941970936840..4e133d3df403 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -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
diff --git a/drivers/pci/pci-pf-stub.c b/drivers/pci/pci-pf-stub.c
new file mode 100644
index 000000000000..9d5fdf20d485
--- /dev/null
+++ b/drivers/pci/pci-pf-stub.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+/* pci-pf-stub - simple stub driver for PCI SR-IOV PF device
+ *
+ * This driver is meant to act as a "white-list" for devices that provde
+ * SR-IOV functionality while at the same time not actually needing a
+ * driver of their own.
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+
+/**
+ * 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.
+ */
+static const struct pci_device_id pci_pf_stub_white_list[] = {
+	{ PCI_VDEVICE(AMAZON, 0x0053) },
+	/* required last entry */
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(pci, pci_pf_stub_white_list);
+
+static int pci_pf_stub_probe(struct pci_dev *dev,
+			     const struct pci_device_id *id)
+{
+	pci_info(dev, "claimed by pci-pf-stub\n");
+	return 0;
+}
+
+static struct pci_driver pf_stub_driver = {
+	.name			= "pci-pf-stub",
+	.id_table		= pci_pf_stub_white_list,
+	.probe			= pci_pf_stub_probe,
+	.sriov_configure	= pci_sriov_configure_simple,
+};
+
+static int __init pci_pf_stub_init(void)
+{
+	return pci_register_driver(&pf_stub_driver);
+}
+
+static void __exit pci_pf_stub_exit(void)
+{
+	pci_unregister_driver(&pf_stub_driver);
+}
+
+module_init(pci_pf_stub_init);
+module_exit(pci_pf_stub_exit);
+
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index a6b30667a331..b10621896017 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -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
 

^ permalink raw reply related	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 5/5] pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs
@ 2018-03-15 18:44   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-15 18:44 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, ddutile, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, hch, dwmw

From: Alexander Duyck <alexander.h.duyck@intel.com>

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.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

v6: New driver to address concerns about Amazon devices left unsupported
v7: Dropped pci_id table explanation from pci-pf-stub driver

 drivers/pci/Kconfig       |   12 ++++++++++
 drivers/pci/Makefile      |    2 ++
 drivers/pci/pci-pf-stub.c |   54 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h   |    2 ++
 4 files changed, 70 insertions(+)
 create mode 100644 drivers/pci/pci-pf-stub.c

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 34b56a8f8480..cdef2a2a9bc5 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -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
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 941970936840..4e133d3df403 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -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
diff --git a/drivers/pci/pci-pf-stub.c b/drivers/pci/pci-pf-stub.c
new file mode 100644
index 000000000000..9d5fdf20d485
--- /dev/null
+++ b/drivers/pci/pci-pf-stub.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+/* pci-pf-stub - simple stub driver for PCI SR-IOV PF device
+ *
+ * This driver is meant to act as a "white-list" for devices that provde
+ * SR-IOV functionality while at the same time not actually needing a
+ * driver of their own.
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+
+/**
+ * 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.
+ */
+static const struct pci_device_id pci_pf_stub_white_list[] = {
+	{ PCI_VDEVICE(AMAZON, 0x0053) },
+	/* required last entry */
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(pci, pci_pf_stub_white_list);
+
+static int pci_pf_stub_probe(struct pci_dev *dev,
+			     const struct pci_device_id *id)
+{
+	pci_info(dev, "claimed by pci-pf-stub\n");
+	return 0;
+}
+
+static struct pci_driver pf_stub_driver = {
+	.name			= "pci-pf-stub",
+	.id_table		= pci_pf_stub_white_list,
+	.probe			= pci_pf_stub_probe,
+	.sriov_configure	= pci_sriov_configure_simple,
+};
+
+static int __init pci_pf_stub_init(void)
+{
+	return pci_register_driver(&pf_stub_driver);
+}
+
+static void __exit pci_pf_stub_exit(void)
+{
+	pci_unregister_driver(&pf_stub_driver);
+}
+
+module_init(pci_pf_stub_init);
+module_exit(pci_pf_stub_exit);
+
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index a6b30667a331..b10621896017 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -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
 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply related	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-03-15 18:42   ` Alexander Duyck
  (?)
@ 2018-03-16 16:34     ` Michael S. Tsirkin
  -1 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-03-16 16:34 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: bhelgaas, alexander.h.duyck, linux-pci, virtio-dev, kvm, netdev,
	dan.daly, linux-kernel, linux-nvme, keith.busch, netanel,
	ddutile, mheyne, liang-min.wang, mark.d.rustad, dwmw2, hch, dwmw

On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>

So if and when virtio PFs can manage the VFs, then we can
add a feature bit for that?
Seems reasonable.

Also, I am guessing that hardware implementations will want
to add things like stong memory barriers - I guess we
will add new feature bits for that too down the road?


> ---
> 
> 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
> v7: No code change, added Reviewed-by
> 
>  drivers/virtio/virtio_pci_common.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..67a227fd7aa0 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -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);
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-03-16 16:34     ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-03-16 16:34 UTC (permalink / raw)


On Thu, Mar 15, 2018@11:42:41AM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck at intel.com>
> 
> 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.
> 
> Reviewed-by: Christoph Hellwig <hch at lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>

So if and when virtio PFs can manage the VFs, then we can
add a feature bit for that?
Seems reasonable.

Also, I am guessing that hardware implementations will want
to add things like stong memory barriers - I guess we
will add new feature bits for that too down the road?


> ---
> 
> 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
> v7: No code change, added Reviewed-by
> 
>  drivers/virtio/virtio_pci_common.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..67a227fd7aa0 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -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);
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe at lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help at lists.oasis-open.org

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-03-16 16:34     ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-03-16 16:34 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: bhelgaas, alexander.h.duyck, linux-pci, virtio-dev, kvm, netdev,
	dan.daly, linux-kernel, linux-nvme, keith.busch, netanel,
	ddutile, mheyne, liang-min.wang, mark.d.rustad, dwmw2, hch, dwmw

On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>

So if and when virtio PFs can manage the VFs, then we can
add a feature bit for that?
Seems reasonable.

Also, I am guessing that hardware implementations will want
to add things like stong memory barriers - I guess we
will add new feature bits for that too down the road?


> ---
> 
> 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
> v7: No code change, added Reviewed-by
> 
>  drivers/virtio/virtio_pci_common.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..67a227fd7aa0 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -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);
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-03-16 16:34     ` Michael S. Tsirkin
  (?)
@ 2018-03-16 16:40       ` Alexander Duyck
  -1 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-16 16:40 UTC (permalink / raw)
  To: Michael S. Tsirkin, Daly, Dan
  Cc: Bjorn Helgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm,
	Netdev, LKML, linux-nvme, Keith Busch, netanel, Don Dutile,
	Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
>> From: Alexander Duyck <alexander.h.duyck@intel.com>
>>
>> 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.
>>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>
> So if and when virtio PFs can manage the VFs, then we can
> add a feature bit for that?
> Seems reasonable.

Yes. If nothing else you may not even need a feature bit depending on
how things go. One of the reasons why Mark called out the
subvendor/subdevice was because that might be able to be used to
identify the specific hardware that is providing the SR-IOV feature so
in the future if it is added to virtio itself then you could exclude
devices like this by just limiting things based on subvendor/subdevice
IDs.

> Also, I am guessing that hardware implementations will want
> to add things like stong memory barriers - I guess we
> will add new feature bits for that too down the road?

That piece I don't have visibility into at this time. Perhaps Dan
might have more visibility into future plans on what this might need.

Thanks.

- Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-03-16 16:40       ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-16 16:40 UTC (permalink / raw)


On Fri, Mar 16, 2018@9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Mar 15, 2018@11:42:41AM -0700, Alexander Duyck wrote:
>> From: Alexander Duyck <alexander.h.duyck at intel.com>
>>
>> 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.
>>
>> Reviewed-by: Christoph Hellwig <hch at lst.de>
>> Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
>
> So if and when virtio PFs can manage the VFs, then we can
> add a feature bit for that?
> Seems reasonable.

Yes. If nothing else you may not even need a feature bit depending on
how things go. One of the reasons why Mark called out the
subvendor/subdevice was because that might be able to be used to
identify the specific hardware that is providing the SR-IOV feature so
in the future if it is added to virtio itself then you could exclude
devices like this by just limiting things based on subvendor/subdevice
IDs.

> Also, I am guessing that hardware implementations will want
> to add things like stong memory barriers - I guess we
> will add new feature bits for that too down the road?

That piece I don't have visibility into at this time. Perhaps Dan
might have more visibility into future plans on what this might need.

Thanks.

- Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-03-16 16:40       ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-03-16 16:40 UTC (permalink / raw)
  To: Michael S. Tsirkin, Daly, Dan
  Cc: Bjorn Helgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm,
	Netdev, LKML, linux-nvme, Keith Busch, netanel, Don Dutile,
	Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
>> From: Alexander Duyck <alexander.h.duyck@intel.com>
>>
>> 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.
>>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>
> So if and when virtio PFs can manage the VFs, then we can
> add a feature bit for that?
> Seems reasonable.

Yes. If nothing else you may not even need a feature bit depending on
how things go. One of the reasons why Mark called out the
subvendor/subdevice was because that might be able to be used to
identify the specific hardware that is providing the SR-IOV feature so
in the future if it is added to virtio itself then you could exclude
devices like this by just limiting things based on subvendor/subdevice
IDs.

> Also, I am guessing that hardware implementations will want
> to add things like stong memory barriers - I guess we
> will add new feature bits for that too down the road?

That piece I don't have visibility into at this time. Perhaps Dan
might have more visibility into future plans on what this might need.

Thanks.

- Alex

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [pci PATCH v7 0/5] Add support for unmanaged SR-IOV
  2018-03-15 18:40 ` Alexander Duyck
@ 2018-03-16 21:42   ` Don Dutile
  -1 siblings, 0 replies; 78+ messages in thread
From: Don Dutile @ 2018-03-16 21:42 UTC (permalink / raw)
  To: Alexander Duyck, bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, linux-nvme,
	keith.busch, netanel, mheyne, liang-min.wang, mark.d.rustad,
	dwmw2, hch, dwmw

On 03/15/2018 02:40 PM, Alexander Duyck wrote:
> 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
> v7: Dropped pci_id table explanation from pci-pf-stub driver
>      Updated pci_sriov_configure_simple to drop need for err value
>      Fixed comment explaining why pci_sriov_configure_simple is NULL
> 
> Cc: Mark Rustad <mark.d.rustad@intel.com>
> Cc: Maximilian Heyne <mheyne@amazon.de>
> Cc: Liang-Min Wang <liang-min.wang@intel.com>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> 
> ---
> 
> 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                            |   31 +++++++++++++++
>   drivers/pci/pci-pf-stub.c                    |   54 ++++++++++++++++++++++++++
>   drivers/virtio/virtio_pci_common.c           |    1
>   include/linux/pci.h                          |    3 +
>   include/linux/pci_ids.h                      |    2 +
>   9 files changed, 107 insertions(+), 46 deletions(-)
>   create mode 100644 drivers/pci/pci-pf-stub.c
> 
> --
> 
For what it's worth.

Good, simpler start for this type of support/effort.
Thanks for the multiple versions to get to this point.

Reviewed-by: Donald Dutile <ddutile@redhat.com>

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [pci PATCH v7 0/5] Add support for unmanaged SR-IOV
@ 2018-03-16 21:42   ` Don Dutile
  0 siblings, 0 replies; 78+ messages in thread
From: Don Dutile @ 2018-03-16 21:42 UTC (permalink / raw)


On 03/15/2018 02:40 PM, Alexander Duyck wrote:
> 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
> v7: Dropped pci_id table explanation from pci-pf-stub driver
>      Updated pci_sriov_configure_simple to drop need for err value
>      Fixed comment explaining why pci_sriov_configure_simple is NULL
> 
> Cc: Mark Rustad <mark.d.rustad at intel.com>
> Cc: Maximilian Heyne <mheyne at amazon.de>
> Cc: Liang-Min Wang <liang-min.wang at intel.com>
> Cc: David Woodhouse <dwmw at amazon.co.uk>
> 
> ---
> 
> 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                            |   31 +++++++++++++++
>   drivers/pci/pci-pf-stub.c                    |   54 ++++++++++++++++++++++++++
>   drivers/virtio/virtio_pci_common.c           |    1
>   include/linux/pci.h                          |    3 +
>   include/linux/pci_ids.h                      |    2 +
>   9 files changed, 107 insertions(+), 46 deletions(-)
>   create mode 100644 drivers/pci/pci-pf-stub.c
> 
> --
> 
For what it's worth.

Good, simpler start for this type of support/effort.
Thanks for the multiple versions to get to this point.

Reviewed-by: Donald Dutile <ddutile at redhat.com>

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
  2018-03-15 18:41   ` Alexander Duyck
  (?)
@ 2018-03-28 21:30     ` Rustad, Mark D
  -1 siblings, 0 replies; 78+ messages in thread
From: Rustad, Mark D @ 2018-03-28 21:30 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: bhelgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm, Netdev,
	Daly, Dan, LKML, linux-nvme, Busch, Keith, netanel, ddutile,
	mheyne, Wang, Liang-min, dwmw2, hch, dwmw

[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]

On Mar 15, 2018, at 11:41 AM, Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> 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.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> 
> 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
> v7: Updated pci_sriov_configure_simple to drop need for err value
>     Fixed comment explaining why pci_sriov_configure_simple is NULL
> 
>  drivers/pci/iov.c   |   31 +++++++++++++++++++++++++++++++
>  include/linux/pci.h |    3 +++
>  2 files changed, 34 insertions(+)

Tested with the device identified in patch #2.

Tested-by: Mark Rustad <mark.d.rustad@intel.com>

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [pci PATCH v7 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
@ 2018-03-28 21:30     ` Rustad, Mark D
  0 siblings, 0 replies; 78+ messages in thread
From: Rustad, Mark D @ 2018-03-28 21:30 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: bhelgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm, Netdev,
	Daly, Dan, LKML, linux-nvme, Busch, Keith, netanel, ddutile,
	mheyne, Wang, Liang-min, dwmw2, hch, dwmw

[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]

On Mar 15, 2018, at 11:41 AM, Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> 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.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> 
> 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
> v7: Updated pci_sriov_configure_simple to drop need for err value
>     Fixed comment explaining why pci_sriov_configure_simple is NULL
> 
>  drivers/pci/iov.c   |   31 +++++++++++++++++++++++++++++++
>  include/linux/pci.h |    3 +++
>  2 files changed, 34 insertions(+)

Tested with the device identified in patch #2.

Tested-by: Mark Rustad <mark.d.rustad@intel.com>

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
@ 2018-03-28 21:30     ` Rustad, Mark D
  0 siblings, 0 replies; 78+ messages in thread
From: Rustad, Mark D @ 2018-03-28 21:30 UTC (permalink / raw)


On Mar 15, 2018,@11:41 AM, Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck at intel.com>
> 
> 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.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
> ---
> 
> 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
> v7: Updated pci_sriov_configure_simple to drop need for err value
>     Fixed comment explaining why pci_sriov_configure_simple is NULL
> 
>  drivers/pci/iov.c   |   31 +++++++++++++++++++++++++++++++
>  include/linux/pci.h |    3 +++
>  2 files changed, 34 insertions(+)

Tested with the device identified in patch #2.

Tested-by: Mark Rustad <mark.d.rustad at intel.com>

--
Mark Rustad, Networking Division, Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 873 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20180328/2014b39a/attachment.sig>

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-03-15 18:42   ` Alexander Duyck
  (?)
  (?)
@ 2018-03-28 21:31     ` Rustad, Mark D
  -1 siblings, 0 replies; 78+ messages in thread
From: Rustad, Mark D @ 2018-03-28 21:31 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: bhelgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm, netdev,
	Daly, Dan, linux-kernel, linux-nvme, Busch, Keith, netanel,
	ddutile, mheyne, Wang, Liang-min, dwmw2, hch, dwmw

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

On Mar 15, 2018, at 11:42 AM, Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> 
> 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
> v7: No code change, added Reviewed-by
> 
>  drivers/virtio/virtio_pci_common.c |    1 +
>  1 file changed, 1 insertion(+)

Tested with the identified device.

Tested-by: Mark Rustad <mark.d.rustad@intel.com>

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-03-28 21:31     ` Rustad, Mark D
  0 siblings, 0 replies; 78+ messages in thread
From: Rustad, Mark D @ 2018-03-28 21:31 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: bhelgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm, netdev,
	Daly, Dan, linux-kernel, linux-nvme, Busch, Keith, netanel,
	ddutile, mheyne, Wang, Liang-min, dwmw2, hch,

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

On Mar 15, 2018, at 11:42 AM, Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> 
> 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
> v7: No code change, added Reviewed-by
> 
>  drivers/virtio/virtio_pci_common.c |    1 +
>  1 file changed, 1 insertion(+)

Tested with the identified device.

Tested-by: Mark Rustad <mark.d.rustad@intel.com>

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-03-28 21:31     ` Rustad, Mark D
  0 siblings, 0 replies; 78+ messages in thread
From: Rustad, Mark D @ 2018-03-28 21:31 UTC (permalink / raw)


On Mar 15, 2018,@11:42 AM, Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck at intel.com>
> 
> 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.
> 
> Reviewed-by: Christoph Hellwig <hch at lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
> ---
> 
> 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
> v7: No code change, added Reviewed-by
> 
>  drivers/virtio/virtio_pci_common.c |    1 +
>  1 file changed, 1 insertion(+)

Tested with the identified device.

Tested-by: Mark Rustad <mark.d.rustad at intel.com>

--
Mark Rustad, Networking Division, Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 873 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20180328/3b039aae/attachment.sig>

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] Re: [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-03-28 21:31     ` Rustad, Mark D
  0 siblings, 0 replies; 78+ messages in thread
From: Rustad, Mark D @ 2018-03-28 21:31 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: bhelgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm, netdev,
	Daly, Dan, linux-kernel, linux-nvme, Busch, Keith, netanel,
	ddutile, mheyne, Wang, Liang-min, dwmw2, hch, dwmw

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

On Mar 15, 2018, at 11:42 AM, Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> 
> 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
> v7: No code change, added Reviewed-by
> 
>  drivers/virtio/virtio_pci_common.c |    1 +
>  1 file changed, 1 insertion(+)

Tested with the identified device.

Tested-by: Mark Rustad <mark.d.rustad@intel.com>

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-03-15 18:42   ` Alexander Duyck
  (?)
  (?)
@ 2018-04-03 13:11     ` Michael S. Tsirkin
  -1 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-03 13:11 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: bhelgaas, alexander.h.duyck, linux-pci, virtio-dev, kvm, netdev,
	dan.daly, linux-kernel, linux-nvme, keith.busch, netanel,
	ddutile, mheyne, liang-min.wang, mark.d.rustad, dwmw2, hch, dwmw

On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>

I thought hard about this, and I think we need a feature
bit for this. This way host can detect support,
and we can also change our minds later if we need
to modify the interface and manage VFs after all.

It seems PCI specific so non pci transports would disable the feature
for now.

> ---
> 
> 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
> v7: No code change, added Reviewed-by
> 
>  drivers/virtio/virtio_pci_common.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..67a227fd7aa0 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -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);
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 13:11     ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-03 13:11 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: bhelgaas, alexander.h.duyck, linux-pci, virtio-dev, kvm, netdev,
	dan.daly, linux-kernel, linux-nvme, keith.busch, netanel,
	ddutile, mheyne, liang-min.wang, mark.d.rustad, dwmw2, hch, dwmw

On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>

I thought hard about this, and I think we need a feature
bit for this. This way host can detect support,
and we can also change our minds later if we need
to modify the interface and manage VFs after all.

It seems PCI specific so non pci transports would disable the feature
for now.

> ---
> 
> 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
> v7: No code change, added Reviewed-by
> 
>  drivers/virtio/virtio_pci_common.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..67a227fd7aa0 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -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);
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 13:11     ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-03 13:11 UTC (permalink / raw)


On Thu, Mar 15, 2018@11:42:41AM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck at intel.com>
> 
> 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.
> 
> Reviewed-by: Christoph Hellwig <hch at lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>

I thought hard about this, and I think we need a feature
bit for this. This way host can detect support,
and we can also change our minds later if we need
to modify the interface and manage VFs after all.

It seems PCI specific so non pci transports would disable the feature
for now.

> ---
> 
> 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
> v7: No code change, added Reviewed-by
> 
>  drivers/virtio/virtio_pci_common.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..67a227fd7aa0 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -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);
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe at lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help at lists.oasis-open.org

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 13:11     ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-03 13:11 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: bhelgaas, alexander.h.duyck, linux-pci, virtio-dev, kvm, netdev,
	dan.daly, linux-kernel, linux-nvme, keith.busch, netanel,
	ddutile, mheyne, liang-min.wang, mark.d.rustad, dwmw2, hch, dwmw

On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>

I thought hard about this, and I think we need a feature
bit for this. This way host can detect support,
and we can also change our minds later if we need
to modify the interface and manage VFs after all.

It seems PCI specific so non pci transports would disable the feature
for now.

> ---
> 
> 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
> v7: No code change, added Reviewed-by
> 
>  drivers/virtio/virtio_pci_common.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..67a227fd7aa0 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -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);
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-03-16 16:40       ` Alexander Duyck
  (?)
@ 2018-04-03 13:12         ` Michael S. Tsirkin
  -1 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-03 13:12 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
> >>
> >> 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.
> >>
> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> >
> > So if and when virtio PFs can manage the VFs, then we can
> > add a feature bit for that?
> > Seems reasonable.
> 
> Yes. If nothing else you may not even need a feature bit depending on
> how things go.

OTOH if the interface is changed in an incompatible way,
and old Linux will attempt to drive the new device
since there is no check.

I think we should add a feature bit right away.


> One of the reasons why Mark called out the
> subvendor/subdevice was because that might be able to be used to
> identify the specific hardware that is providing the SR-IOV feature so
> in the future if it is added to virtio itself then you could exclude
> devices like this by just limiting things based on subvendor/subdevice
> IDs.
> 
> > Also, I am guessing that hardware implementations will want
> > to add things like stong memory barriers - I guess we
> > will add new feature bits for that too down the road?
> 
> That piece I don't have visibility into at this time. Perhaps Dan
> might have more visibility into future plans on what this might need.
> 
> Thanks.
> 
> - Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 13:12         ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-03 13:12 UTC (permalink / raw)


On Fri, Mar 16, 2018@09:40:34AM -0700, Alexander Duyck wrote:
> On Fri, Mar 16, 2018@9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Thu, Mar 15, 2018@11:42:41AM -0700, Alexander Duyck wrote:
> >> From: Alexander Duyck <alexander.h.duyck at intel.com>
> >>
> >> 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.
> >>
> >> Reviewed-by: Christoph Hellwig <hch at lst.de>
> >> Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
> >> Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
> >
> > So if and when virtio PFs can manage the VFs, then we can
> > add a feature bit for that?
> > Seems reasonable.
> 
> Yes. If nothing else you may not even need a feature bit depending on
> how things go.

OTOH if the interface is changed in an incompatible way,
and old Linux will attempt to drive the new device
since there is no check.

I think we should add a feature bit right away.


> One of the reasons why Mark called out the
> subvendor/subdevice was because that might be able to be used to
> identify the specific hardware that is providing the SR-IOV feature so
> in the future if it is added to virtio itself then you could exclude
> devices like this by just limiting things based on subvendor/subdevice
> IDs.
> 
> > Also, I am guessing that hardware implementations will want
> > to add things like stong memory barriers - I guess we
> > will add new feature bits for that too down the road?
> 
> That piece I don't have visibility into at this time. Perhaps Dan
> might have more visibility into future plans on what this might need.
> 
> Thanks.
> 
> - Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 13:12         ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-03 13:12 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
> >>
> >> 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.
> >>
> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> >
> > So if and when virtio PFs can manage the VFs, then we can
> > add a feature bit for that?
> > Seems reasonable.
> 
> Yes. If nothing else you may not even need a feature bit depending on
> how things go.

OTOH if the interface is changed in an incompatible way,
and old Linux will attempt to drive the new device
since there is no check.

I think we should add a feature bit right away.


> One of the reasons why Mark called out the
> subvendor/subdevice was because that might be able to be used to
> identify the specific hardware that is providing the SR-IOV feature so
> in the future if it is added to virtio itself then you could exclude
> devices like this by just limiting things based on subvendor/subdevice
> IDs.
> 
> > Also, I am guessing that hardware implementations will want
> > to add things like stong memory barriers - I guess we
> > will add new feature bits for that too down the road?
> 
> That piece I don't have visibility into at this time. Perhaps Dan
> might have more visibility into future plans on what this might need.
> 
> Thanks.
> 
> - Alex

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-04-03 13:12         ` Michael S. Tsirkin
  (?)
  (?)
@ 2018-04-03 17:32           ` Alexander Duyck
  -1 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-03 17:32 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
>> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
>> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
>> >>
>> >> 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.
>> >>
>> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> >
>> > So if and when virtio PFs can manage the VFs, then we can
>> > add a feature bit for that?
>> > Seems reasonable.
>>
>> Yes. If nothing else you may not even need a feature bit depending on
>> how things go.
>
> OTOH if the interface is changed in an incompatible way,
> and old Linux will attempt to drive the new device
> since there is no check.
>
> I think we should add a feature bit right away.

I'm not sure why you would need a feature bit. The capability is
controlled via PCI configuration space. If it is present the device
has the capability. If it is not then it does not.

Basically if the PCI configuration space is not present then the sysfs
entries will not be spawned and nothing will attempt to use this
function.

- ALex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 17:32           ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-03 17:32 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
>> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
>> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
>> >>
>> >> 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.
>> >>
>> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> >
>> > So if and when virtio PFs can manage the VFs, then we can
>> > add a feature bit for that?
>> > Seems reasonable.
>>
>> Yes. If nothing else you may not even need a feature bit depending on
>> how things go.
>
> OTOH if the interface is changed in an incompatible way,
> and old Linux will attempt to drive the new device
> since there is no check.
>
> I think we should add a feature bit right away.

I'm not sure why you would need a feature bit. The capability is
controlled via PCI configuration space. If it is present the device
has the capability. If it is not then it does not.

Basically if the PCI configuration space is not present then the sysfs
entries will not be spawned and nothing will attempt to use this
function.

- ALex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 17:32           ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-03 17:32 UTC (permalink / raw)


On Tue, Apr 3, 2018@6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Mar 16, 2018@09:40:34AM -0700, Alexander Duyck wrote:
>> On Fri, Mar 16, 2018@9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Thu, Mar 15, 2018@11:42:41AM -0700, Alexander Duyck wrote:
>> >> From: Alexander Duyck <alexander.h.duyck at intel.com>
>> >>
>> >> 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.
>> >>
>> >> Reviewed-by: Christoph Hellwig <hch at lst.de>
>> >> Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
>> >> Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
>> >
>> > So if and when virtio PFs can manage the VFs, then we can
>> > add a feature bit for that?
>> > Seems reasonable.
>>
>> Yes. If nothing else you may not even need a feature bit depending on
>> how things go.
>
> OTOH if the interface is changed in an incompatible way,
> and old Linux will attempt to drive the new device
> since there is no check.
>
> I think we should add a feature bit right away.

I'm not sure why you would need a feature bit. The capability is
controlled via PCI configuration space. If it is present the device
has the capability. If it is not then it does not.

Basically if the PCI configuration space is not present then the sysfs
entries will not be spawned and nothing will attempt to use this
function.

- ALex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 17:32           ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-03 17:32 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
>> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
>> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
>> >>
>> >> 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.
>> >>
>> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> >
>> > So if and when virtio PFs can manage the VFs, then we can
>> > add a feature bit for that?
>> > Seems reasonable.
>>
>> Yes. If nothing else you may not even need a feature bit depending on
>> how things go.
>
> OTOH if the interface is changed in an incompatible way,
> and old Linux will attempt to drive the new device
> since there is no check.
>
> I think we should add a feature bit right away.

I'm not sure why you would need a feature bit. The capability is
controlled via PCI configuration space. If it is present the device
has the capability. If it is not then it does not.

Basically if the PCI configuration space is not present then the sysfs
entries will not be spawned and nothing will attempt to use this
function.

- ALex

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-04-03 17:32           ` Alexander Duyck
  (?)
@ 2018-04-03 18:27             ` Michael S. Tsirkin
  -1 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-03 18:27 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Tue, Apr 03, 2018 at 10:32:00AM -0700, Alexander Duyck wrote:
> On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
> >> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> >> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >>
> >> >> 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.
> >> >>
> >> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
> >> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >
> >> > So if and when virtio PFs can manage the VFs, then we can
> >> > add a feature bit for that?
> >> > Seems reasonable.
> >>
> >> Yes. If nothing else you may not even need a feature bit depending on
> >> how things go.
> >
> > OTOH if the interface is changed in an incompatible way,
> > and old Linux will attempt to drive the new device
> > since there is no check.
> >
> > I think we should add a feature bit right away.
> 
> I'm not sure why you would need a feature bit. The capability is
> controlled via PCI configuration space. If it is present the device
> has the capability. If it is not then it does not.
> 
> Basically if the PCI configuration space is not present then the sysfs
> entries will not be spawned and nothing will attempt to use this
> function.
> 
> - ALex

It's about compability with older guests which ignore the
capability.

The feature is thus helpful so host knows whether guest supports VFs.


-- 
MSR

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 18:27             ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-03 18:27 UTC (permalink / raw)


On Tue, Apr 03, 2018@10:32:00AM -0700, Alexander Duyck wrote:
> On Tue, Apr 3, 2018@6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Mar 16, 2018@09:40:34AM -0700, Alexander Duyck wrote:
> >> On Fri, Mar 16, 2018@9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Thu, Mar 15, 2018@11:42:41AM -0700, Alexander Duyck wrote:
> >> >> From: Alexander Duyck <alexander.h.duyck at intel.com>
> >> >>
> >> >> 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.
> >> >>
> >> >> Reviewed-by: Christoph Hellwig <hch at lst.de>
> >> >> Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
> >> >
> >> > So if and when virtio PFs can manage the VFs, then we can
> >> > add a feature bit for that?
> >> > Seems reasonable.
> >>
> >> Yes. If nothing else you may not even need a feature bit depending on
> >> how things go.
> >
> > OTOH if the interface is changed in an incompatible way,
> > and old Linux will attempt to drive the new device
> > since there is no check.
> >
> > I think we should add a feature bit right away.
> 
> I'm not sure why you would need a feature bit. The capability is
> controlled via PCI configuration space. If it is present the device
> has the capability. If it is not then it does not.
> 
> Basically if the PCI configuration space is not present then the sysfs
> entries will not be spawned and nothing will attempt to use this
> function.
> 
> - ALex

It's about compability with older guests which ignore the
capability.

The feature is thus helpful so host knows whether guest supports VFs.


-- 
MSR

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 18:27             ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-03 18:27 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Tue, Apr 03, 2018 at 10:32:00AM -0700, Alexander Duyck wrote:
> On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
> >> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> >> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >>
> >> >> 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.
> >> >>
> >> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
> >> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >
> >> > So if and when virtio PFs can manage the VFs, then we can
> >> > add a feature bit for that?
> >> > Seems reasonable.
> >>
> >> Yes. If nothing else you may not even need a feature bit depending on
> >> how things go.
> >
> > OTOH if the interface is changed in an incompatible way,
> > and old Linux will attempt to drive the new device
> > since there is no check.
> >
> > I think we should add a feature bit right away.
> 
> I'm not sure why you would need a feature bit. The capability is
> controlled via PCI configuration space. If it is present the device
> has the capability. If it is not then it does not.
> 
> Basically if the PCI configuration space is not present then the sysfs
> entries will not be spawned and nothing will attempt to use this
> function.
> 
> - ALex

It's about compability with older guests which ignore the
capability.

The feature is thus helpful so host knows whether guest supports VFs.


-- 
MSR

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-04-03 18:27             ` Michael S. Tsirkin
  (?)
@ 2018-04-03 19:06               ` Alexander Duyck
  -1 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-03 19:06 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Tue, Apr 3, 2018 at 11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Apr 03, 2018 at 10:32:00AM -0700, Alexander Duyck wrote:
>> On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
>> >> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
>> >> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
>> >> >>
>> >> >> 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.
>> >> >>
>> >> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> >> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> >> >
>> >> > So if and when virtio PFs can manage the VFs, then we can
>> >> > add a feature bit for that?
>> >> > Seems reasonable.
>> >>
>> >> Yes. If nothing else you may not even need a feature bit depending on
>> >> how things go.
>> >
>> > OTOH if the interface is changed in an incompatible way,
>> > and old Linux will attempt to drive the new device
>> > since there is no check.
>> >
>> > I think we should add a feature bit right away.
>>
>> I'm not sure why you would need a feature bit. The capability is
>> controlled via PCI configuration space. If it is present the device
>> has the capability. If it is not then it does not.
>>
>> Basically if the PCI configuration space is not present then the sysfs
>> entries will not be spawned and nothing will attempt to use this
>> function.
>>
>> - ALex
>
> It's about compability with older guests which ignore the
> capability.
>
> The feature is thus helpful so host knows whether guest supports VFs.

The thing is if the capability is ignored then the feature isn't used.
So for SR-IOV it isn't an uncommon thing for there to be drivers for
the PF floating around that do not support SR-IOV. In such cases
SR-IOV just isn't used while the hardware could support it.

I would think in the case of virtio it would be the same kind of
thing. Basically if SR-IOV is supported by the host then the
capability would be present. If SR-IOV is supported by the guest then
it would make use of the capability to spawn VFs. If either the
capability isn't present, or the driver doesn't use it then you won't
be able to spawn VFs in the guest.

Maybe I am missing something. Do you support dynamically changing the
PCI configuration space for Virtio devices based on the presence of
feature bits provided by the guest?

Also are you saying this patch set should wait on the feature bit to
be added, or are you talking about doing this as some sort of
follow-up?

- Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 19:06               ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-03 19:06 UTC (permalink / raw)


On Tue, Apr 3, 2018@11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Apr 03, 2018@10:32:00AM -0700, Alexander Duyck wrote:
>> On Tue, Apr 3, 2018@6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Fri, Mar 16, 2018@09:40:34AM -0700, Alexander Duyck wrote:
>> >> On Fri, Mar 16, 2018@9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> > On Thu, Mar 15, 2018@11:42:41AM -0700, Alexander Duyck wrote:
>> >> >> From: Alexander Duyck <alexander.h.duyck at intel.com>
>> >> >>
>> >> >> 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.
>> >> >>
>> >> >> Reviewed-by: Christoph Hellwig <hch at lst.de>
>> >> >> Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
>> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
>> >> >
>> >> > So if and when virtio PFs can manage the VFs, then we can
>> >> > add a feature bit for that?
>> >> > Seems reasonable.
>> >>
>> >> Yes. If nothing else you may not even need a feature bit depending on
>> >> how things go.
>> >
>> > OTOH if the interface is changed in an incompatible way,
>> > and old Linux will attempt to drive the new device
>> > since there is no check.
>> >
>> > I think we should add a feature bit right away.
>>
>> I'm not sure why you would need a feature bit. The capability is
>> controlled via PCI configuration space. If it is present the device
>> has the capability. If it is not then it does not.
>>
>> Basically if the PCI configuration space is not present then the sysfs
>> entries will not be spawned and nothing will attempt to use this
>> function.
>>
>> - ALex
>
> It's about compability with older guests which ignore the
> capability.
>
> The feature is thus helpful so host knows whether guest supports VFs.

The thing is if the capability is ignored then the feature isn't used.
So for SR-IOV it isn't an uncommon thing for there to be drivers for
the PF floating around that do not support SR-IOV. In such cases
SR-IOV just isn't used while the hardware could support it.

I would think in the case of virtio it would be the same kind of
thing. Basically if SR-IOV is supported by the host then the
capability would be present. If SR-IOV is supported by the guest then
it would make use of the capability to spawn VFs. If either the
capability isn't present, or the driver doesn't use it then you won't
be able to spawn VFs in the guest.

Maybe I am missing something. Do you support dynamically changing the
PCI configuration space for Virtio devices based on the presence of
feature bits provided by the guest?

Also are you saying this patch set should wait on the feature bit to
be added, or are you talking about doing this as some sort of
follow-up?

- Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 19:06               ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-03 19:06 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Tue, Apr 3, 2018 at 11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Apr 03, 2018 at 10:32:00AM -0700, Alexander Duyck wrote:
>> On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
>> >> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
>> >> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
>> >> >>
>> >> >> 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.
>> >> >>
>> >> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> >> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> >> >
>> >> > So if and when virtio PFs can manage the VFs, then we can
>> >> > add a feature bit for that?
>> >> > Seems reasonable.
>> >>
>> >> Yes. If nothing else you may not even need a feature bit depending on
>> >> how things go.
>> >
>> > OTOH if the interface is changed in an incompatible way,
>> > and old Linux will attempt to drive the new device
>> > since there is no check.
>> >
>> > I think we should add a feature bit right away.
>>
>> I'm not sure why you would need a feature bit. The capability is
>> controlled via PCI configuration space. If it is present the device
>> has the capability. If it is not then it does not.
>>
>> Basically if the PCI configuration space is not present then the sysfs
>> entries will not be spawned and nothing will attempt to use this
>> function.
>>
>> - ALex
>
> It's about compability with older guests which ignore the
> capability.
>
> The feature is thus helpful so host knows whether guest supports VFs.

The thing is if the capability is ignored then the feature isn't used.
So for SR-IOV it isn't an uncommon thing for there to be drivers for
the PF floating around that do not support SR-IOV. In such cases
SR-IOV just isn't used while the hardware could support it.

I would think in the case of virtio it would be the same kind of
thing. Basically if SR-IOV is supported by the host then the
capability would be present. If SR-IOV is supported by the guest then
it would make use of the capability to spawn VFs. If either the
capability isn't present, or the driver doesn't use it then you won't
be able to spawn VFs in the guest.

Maybe I am missing something. Do you support dynamically changing the
PCI configuration space for Virtio devices based on the presence of
feature bits provided by the guest?

Also are you saying this patch set should wait on the feature bit to
be added, or are you talking about doing this as some sort of
follow-up?

- Alex

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-04-03 18:27             ` Michael S. Tsirkin
  (?)
@ 2018-04-03 19:18               ` Rustad, Mark D
  -1 siblings, 0 replies; 78+ messages in thread
From: Rustad, Mark D @ 2018-04-03 19:18 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Alexander Duyck, Daly, Dan, Bjorn Helgaas, Duyck, Alexander H,
	linux-pci, virtio-dev, kvm-devel, Netdev, LKML, linux-nvme,
	Busch, Keith, netanel, Don Dutile, Maximilian Heyne, Wang,
	Liang-min, David Woodhouse, Christoph Hellwig, dwmw

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]

On Apr 3, 2018, at 11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:

>> I'm not sure why you would need a feature bit. The capability is
>> controlled via PCI configuration space. If it is present the device
>> has the capability. If it is not then it does not.
>>
>> Basically if the PCI configuration space is not present then the sysfs
>> entries will not be spawned and nothing will attempt to use this
>> function.
>>
>> - ALex
>
> It's about compability with older guests which ignore the
> capability.
>
> The feature is thus helpful so host knows whether guest supports VFs.

This is not about a guest creating its own VFs. This is about a host PF  
that happens to have a virtio interface to be able to create virtio VFs  
that can be assigned to guests. Nothing changes at all from a guest  
perspective. Or maybe I am not understanding what you mean by "whether  
guest supports VFs".

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 19:18               ` Rustad, Mark D
  0 siblings, 0 replies; 78+ messages in thread
From: Rustad, Mark D @ 2018-04-03 19:18 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Alexander Duyck, Daly, Dan, Bjorn Helgaas, Duyck, Alexander H,
	linux-pci, virtio-dev, kvm-devel, Netdev, LKML, linux-nvme,
	Busch, Keith, netanel, Don Dutile, Maximilian Heyne, Wang,
	Liang-min, David Woodhouse, Christoph Hellwig, dwmw@amazon.co.uk

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]

On Apr 3, 2018, at 11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:

>> I'm not sure why you would need a feature bit. The capability is
>> controlled via PCI configuration space. If it is present the device
>> has the capability. If it is not then it does not.
>>
>> Basically if the PCI configuration space is not present then the sysfs
>> entries will not be spawned and nothing will attempt to use this
>> function.
>>
>> - ALex
>
> It's about compability with older guests which ignore the
> capability.
>
> The feature is thus helpful so host knows whether guest supports VFs.

This is not about a guest creating its own VFs. This is about a host PF  
that happens to have a virtio interface to be able to create virtio VFs  
that can be assigned to guests. Nothing changes at all from a guest  
perspective. Or maybe I am not understanding what you mean by "whether  
guest supports VFs".

--
Mark Rustad, Networking Division, Intel Corporation

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-03 19:18               ` Rustad, Mark D
  0 siblings, 0 replies; 78+ messages in thread
From: Rustad, Mark D @ 2018-04-03 19:18 UTC (permalink / raw)


On Apr 3, 2018,@11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:

>> I'm not sure why you would need a feature bit. The capability is
>> controlled via PCI configuration space. If it is present the device
>> has the capability. If it is not then it does not.
>>
>> Basically if the PCI configuration space is not present then the sysfs
>> entries will not be spawned and nothing will attempt to use this
>> function.
>>
>> - ALex
>
> It's about compability with older guests which ignore the
> capability.
>
> The feature is thus helpful so host knows whether guest supports VFs.

This is not about a guest creating its own VFs. This is about a host PF  
that happens to have a virtio interface to be able to create virtio VFs  
that can be assigned to guests. Nothing changes at all from a guest  
perspective. Or maybe I am not understanding what you mean by "whether  
guest supports VFs".

--
Mark Rustad, Networking Division, Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 873 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20180403/fe140733/attachment.sig>

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [pci PATCH v7 0/5] Add support for unmanaged SR-IOV
  2018-03-15 18:40 ` Alexander Duyck
  (?)
@ 2018-04-19 22:54   ` Alexander Duyck
  -1 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-19 22:54 UTC (permalink / raw)
  To: Bjorn Helgaas, Duyck, Alexander H, linux-pci
  Cc: virtio-dev, kvm, Netdev, Daly, Dan, LKML, linux-nvme,
	Keith Busch, netanel, Don Dutile, Maximilian Heyne, Wang,
	Liang-min, Rustad, Mark D, David Woodhouse, Christoph Hellwig,
	dwmw, Michael S. Tsirkin

On Thu, Mar 15, 2018 at 11:40 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> 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
> v7: Dropped pci_id table explanation from pci-pf-stub driver
>     Updated pci_sriov_configure_simple to drop need for err value
>     Fixed comment explaining why pci_sriov_configure_simple is NULL
>

Just following up since this has been sitting in patchwork for just
over a month now
(https://patchwork.ozlabs.org/project/linux-pci/list/?series=34034).
I'm just wondering what the expectation is on getting these pulled
into the pci tree? I'm assuming that is the best place for these
patches. Are there any concerns I still need to address or are these
going to be pulled in at some point, and if so is there any ETA on
when that will be?

Thanks.

- Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [pci PATCH v7 0/5] Add support for unmanaged SR-IOV
@ 2018-04-19 22:54   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-19 22:54 UTC (permalink / raw)


On Thu, Mar 15, 2018 at 11:40 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> 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
> v7: Dropped pci_id table explanation from pci-pf-stub driver
>     Updated pci_sriov_configure_simple to drop need for err value
>     Fixed comment explaining why pci_sriov_configure_simple is NULL
>

Just following up since this has been sitting in patchwork for just
over a month now
(https://patchwork.ozlabs.org/project/linux-pci/list/?series=34034).
I'm just wondering what the expectation is on getting these pulled
into the pci tree? I'm assuming that is the best place for these
patches. Are there any concerns I still need to address or are these
going to be pulled in at some point, and if so is there any ETA on
when that will be?

Thanks.

- Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] Re: [pci PATCH v7 0/5] Add support for unmanaged SR-IOV
@ 2018-04-19 22:54   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-19 22:54 UTC (permalink / raw)
  To: Bjorn Helgaas, Duyck, Alexander H, linux-pci
  Cc: virtio-dev, kvm, Netdev, Daly, Dan, LKML, linux-nvme,
	Keith Busch, netanel, Don Dutile, Maximilian Heyne, Wang,
	Liang-min, Rustad, Mark D, David Woodhouse, Christoph Hellwig,
	dwmw, Michael S. Tsirkin

On Thu, Mar 15, 2018 at 11:40 AM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> 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
> v7: Dropped pci_id table explanation from pci-pf-stub driver
>     Updated pci_sriov_configure_simple to drop need for err value
>     Fixed comment explaining why pci_sriov_configure_simple is NULL
>

Just following up since this has been sitting in patchwork for just
over a month now
(https://patchwork.ozlabs.org/project/linux-pci/list/?series=34034).
I'm just wondering what the expectation is on getting these pulled
into the pci tree? I'm assuming that is the best place for these
patches. Are there any concerns I still need to address or are these
going to be pulled in at some point, and if so is there any ETA on
when that will be?

Thanks.

- Alex

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-04-03 19:06               ` Alexander Duyck
  (?)
  (?)
@ 2018-04-20  0:40                 ` Michael S. Tsirkin
  -1 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20  0:40 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Tue, Apr 03, 2018 at 12:06:03PM -0700, Alexander Duyck wrote:
> On Tue, Apr 3, 2018 at 11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Apr 03, 2018 at 10:32:00AM -0700, Alexander Duyck wrote:
> >> On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
> >> >> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> >> >> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >> >>
> >> >> >> 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.
> >> >> >>
> >> >> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
> >> >> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> >> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >> >
> >> >> > So if and when virtio PFs can manage the VFs, then we can
> >> >> > add a feature bit for that?
> >> >> > Seems reasonable.
> >> >>
> >> >> Yes. If nothing else you may not even need a feature bit depending on
> >> >> how things go.
> >> >
> >> > OTOH if the interface is changed in an incompatible way,
> >> > and old Linux will attempt to drive the new device
> >> > since there is no check.
> >> >
> >> > I think we should add a feature bit right away.
> >>
> >> I'm not sure why you would need a feature bit. The capability is
> >> controlled via PCI configuration space. If it is present the device
> >> has the capability. If it is not then it does not.
> >>
> >> Basically if the PCI configuration space is not present then the sysfs
> >> entries will not be spawned and nothing will attempt to use this
> >> function.
> >>
> >> - ALex
> >
> > It's about compability with older guests which ignore the
> > capability.
> >
> > The feature is thus helpful so host knows whether guest supports VFs.
> 
> The thing is if the capability is ignored then the feature isn't used.
> So for SR-IOV it isn't an uncommon thing for there to be drivers for
> the PF floating around that do not support SR-IOV. In such cases
> SR-IOV just isn't used while the hardware could support it.

Right but how come there are VF drivers but PF driver does not
know about these?

And are there PF drivers that intentially do not enable SRIOV
because it's known to be broken in some way?

Case in point I do think virtio want to limit this
depending on a feature bit on general principles
(the principle being that all extensions have feature bits).

There are security implications here - we previously relied on
whitelisting after all.

Wouldn't it be safer to be a bit more careful and update the
actual PF drivers? It's just one line per driver, but it
can be done with an ack by driver maintainer.
If/once we find out all drivers do have it, we can then
change the default.

> I would think in the case of virtio it would be the same kind of
> thing. Basically if SR-IOV is supported by the host then the
> capability would be present. If SR-IOV is supported by the guest then
> it would make use of the capability to spawn VFs. If either the
> capability isn't present, or the driver doesn't use it then you won't
> be able to spawn VFs in the guest.

> Maybe I am missing something. Do you support dynamically changing the
> PCI configuration space for Virtio devices based on the presence of
> feature bits provided by the guest?

No. The point is that IMHO at least virtio - in absence of feature bit -
to ignore VFs rather than assume they are safe to drive
in an unmanaged way.

> Also are you saying this patch set should wait on the feature bit to
> be added, or are you talking about doing this as some sort of
> follow-up?
> 
> - Alex

I think for virtio it should include the feature bit, yes.
Adding feature bit is very easy - post a patch to the virtio TC mailing
list, wait about a week to give people time to respond (two weeks if it
is around holidays and such).

-- 
MST

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20  0:40                 ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20  0:40 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Tue, Apr 03, 2018 at 12:06:03PM -0700, Alexander Duyck wrote:
> On Tue, Apr 3, 2018 at 11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Apr 03, 2018 at 10:32:00AM -0700, Alexander Duyck wrote:
> >> On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
> >> >> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> >> >> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >> >>
> >> >> >> 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.
> >> >> >>
> >> >> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
> >> >> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> >> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >> >
> >> >> > So if and when virtio PFs can manage the VFs, then we can
> >> >> > add a feature bit for that?
> >> >> > Seems reasonable.
> >> >>
> >> >> Yes. If nothing else you may not even need a feature bit depending on
> >> >> how things go.
> >> >
> >> > OTOH if the interface is changed in an incompatible way,
> >> > and old Linux will attempt to drive the new device
> >> > since there is no check.
> >> >
> >> > I think we should add a feature bit right away.
> >>
> >> I'm not sure why you would need a feature bit. The capability is
> >> controlled via PCI configuration space. If it is present the device
> >> has the capability. If it is not then it does not.
> >>
> >> Basically if the PCI configuration space is not present then the sysfs
> >> entries will not be spawned and nothing will attempt to use this
> >> function.
> >>
> >> - ALex
> >
> > It's about compability with older guests which ignore the
> > capability.
> >
> > The feature is thus helpful so host knows whether guest supports VFs.
> 
> The thing is if the capability is ignored then the feature isn't used.
> So for SR-IOV it isn't an uncommon thing for there to be drivers for
> the PF floating around that do not support SR-IOV. In such cases
> SR-IOV just isn't used while the hardware could support it.

Right but how come there are VF drivers but PF driver does not
know about these?

And are there PF drivers that intentially do not enable SRIOV
because it's known to be broken in some way?

Case in point I do think virtio want to limit this
depending on a feature bit on general principles
(the principle being that all extensions have feature bits).

There are security implications here - we previously relied on
whitelisting after all.

Wouldn't it be safer to be a bit more careful and update the
actual PF drivers? It's just one line per driver, but it
can be done with an ack by driver maintainer.
If/once we find out all drivers do have it, we can then
change the default.

> I would think in the case of virtio it would be the same kind of
> thing. Basically if SR-IOV is supported by the host then the
> capability would be present. If SR-IOV is supported by the guest then
> it would make use of the capability to spawn VFs. If either the
> capability isn't present, or the driver doesn't use it then you won't
> be able to spawn VFs in the guest.

> Maybe I am missing something. Do you support dynamically changing the
> PCI configuration space for Virtio devices based on the presence of
> feature bits provided by the guest?

No. The point is that IMHO at least virtio - in absence of feature bit -
to ignore VFs rather than assume they are safe to drive
in an unmanaged way.

> Also are you saying this patch set should wait on the feature bit to
> be added, or are you talking about doing this as some sort of
> follow-up?
> 
> - Alex

I think for virtio it should include the feature bit, yes.
Adding feature bit is very easy - post a patch to the virtio TC mailing
list, wait about a week to give people time to respond (two weeks if it
is around holidays and such).

-- 
MST

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20  0:40                 ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20  0:40 UTC (permalink / raw)


On Tue, Apr 03, 2018@12:06:03PM -0700, Alexander Duyck wrote:
> On Tue, Apr 3, 2018@11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Apr 03, 2018@10:32:00AM -0700, Alexander Duyck wrote:
> >> On Tue, Apr 3, 2018@6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Fri, Mar 16, 2018@09:40:34AM -0700, Alexander Duyck wrote:
> >> >> On Fri, Mar 16, 2018@9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >> > On Thu, Mar 15, 2018@11:42:41AM -0700, Alexander Duyck wrote:
> >> >> >> From: Alexander Duyck <alexander.h.duyck at intel.com>
> >> >> >>
> >> >> >> 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.
> >> >> >>
> >> >> >> Reviewed-by: Christoph Hellwig <hch at lst.de>
> >> >> >> Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
> >> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
> >> >> >
> >> >> > So if and when virtio PFs can manage the VFs, then we can
> >> >> > add a feature bit for that?
> >> >> > Seems reasonable.
> >> >>
> >> >> Yes. If nothing else you may not even need a feature bit depending on
> >> >> how things go.
> >> >
> >> > OTOH if the interface is changed in an incompatible way,
> >> > and old Linux will attempt to drive the new device
> >> > since there is no check.
> >> >
> >> > I think we should add a feature bit right away.
> >>
> >> I'm not sure why you would need a feature bit. The capability is
> >> controlled via PCI configuration space. If it is present the device
> >> has the capability. If it is not then it does not.
> >>
> >> Basically if the PCI configuration space is not present then the sysfs
> >> entries will not be spawned and nothing will attempt to use this
> >> function.
> >>
> >> - ALex
> >
> > It's about compability with older guests which ignore the
> > capability.
> >
> > The feature is thus helpful so host knows whether guest supports VFs.
> 
> The thing is if the capability is ignored then the feature isn't used.
> So for SR-IOV it isn't an uncommon thing for there to be drivers for
> the PF floating around that do not support SR-IOV. In such cases
> SR-IOV just isn't used while the hardware could support it.

Right but how come there are VF drivers but PF driver does not
know about these?

And are there PF drivers that intentially do not enable SRIOV
because it's known to be broken in some way?

Case in point I do think virtio want to limit this
depending on a feature bit on general principles
(the principle being that all extensions have feature bits).

There are security implications here - we previously relied on
whitelisting after all.

Wouldn't it be safer to be a bit more careful and update the
actual PF drivers? It's just one line per driver, but it
can be done with an ack by driver maintainer.
If/once we find out all drivers do have it, we can then
change the default.

> I would think in the case of virtio it would be the same kind of
> thing. Basically if SR-IOV is supported by the host then the
> capability would be present. If SR-IOV is supported by the guest then
> it would make use of the capability to spawn VFs. If either the
> capability isn't present, or the driver doesn't use it then you won't
> be able to spawn VFs in the guest.

> Maybe I am missing something. Do you support dynamically changing the
> PCI configuration space for Virtio devices based on the presence of
> feature bits provided by the guest?

No. The point is that IMHO at least virtio - in absence of feature bit -
to ignore VFs rather than assume they are safe to drive
in an unmanaged way.

> Also are you saying this patch set should wait on the feature bit to
> be added, or are you talking about doing this as some sort of
> follow-up?
> 
> - Alex

I think for virtio it should include the feature bit, yes.
Adding feature bit is very easy - post a patch to the virtio TC mailing
list, wait about a week to give people time to respond (two weeks if it
is around holidays and such).

-- 
MST

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20  0:40                 ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20  0:40 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Tue, Apr 03, 2018 at 12:06:03PM -0700, Alexander Duyck wrote:
> On Tue, Apr 3, 2018 at 11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Apr 03, 2018 at 10:32:00AM -0700, Alexander Duyck wrote:
> >> On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
> >> >> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
> >> >> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >> >>
> >> >> >> 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.
> >> >> >>
> >> >> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
> >> >> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
> >> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> >> >> >
> >> >> > So if and when virtio PFs can manage the VFs, then we can
> >> >> > add a feature bit for that?
> >> >> > Seems reasonable.
> >> >>
> >> >> Yes. If nothing else you may not even need a feature bit depending on
> >> >> how things go.
> >> >
> >> > OTOH if the interface is changed in an incompatible way,
> >> > and old Linux will attempt to drive the new device
> >> > since there is no check.
> >> >
> >> > I think we should add a feature bit right away.
> >>
> >> I'm not sure why you would need a feature bit. The capability is
> >> controlled via PCI configuration space. If it is present the device
> >> has the capability. If it is not then it does not.
> >>
> >> Basically if the PCI configuration space is not present then the sysfs
> >> entries will not be spawned and nothing will attempt to use this
> >> function.
> >>
> >> - ALex
> >
> > It's about compability with older guests which ignore the
> > capability.
> >
> > The feature is thus helpful so host knows whether guest supports VFs.
> 
> The thing is if the capability is ignored then the feature isn't used.
> So for SR-IOV it isn't an uncommon thing for there to be drivers for
> the PF floating around that do not support SR-IOV. In such cases
> SR-IOV just isn't used while the hardware could support it.

Right but how come there are VF drivers but PF driver does not
know about these?

And are there PF drivers that intentially do not enable SRIOV
because it's known to be broken in some way?

Case in point I do think virtio want to limit this
depending on a feature bit on general principles
(the principle being that all extensions have feature bits).

There are security implications here - we previously relied on
whitelisting after all.

Wouldn't it be safer to be a bit more careful and update the
actual PF drivers? It's just one line per driver, but it
can be done with an ack by driver maintainer.
If/once we find out all drivers do have it, we can then
change the default.

> I would think in the case of virtio it would be the same kind of
> thing. Basically if SR-IOV is supported by the host then the
> capability would be present. If SR-IOV is supported by the guest then
> it would make use of the capability to spawn VFs. If either the
> capability isn't present, or the driver doesn't use it then you won't
> be able to spawn VFs in the guest.

> Maybe I am missing something. Do you support dynamically changing the
> PCI configuration space for Virtio devices based on the presence of
> feature bits provided by the guest?

No. The point is that IMHO at least virtio - in absence of feature bit -
to ignore VFs rather than assume they are safe to drive
in an unmanaged way.

> Also are you saying this patch set should wait on the feature bit to
> be added, or are you talking about doing this as some sort of
> follow-up?
> 
> - Alex

I think for virtio it should include the feature bit, yes.
Adding feature bit is very easy - post a patch to the virtio TC mailing
list, wait about a week to give people time to respond (two weeks if it
is around holidays and such).

-- 
MST

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [pci PATCH v7 0/5] Add support for unmanaged SR-IOV
  2018-04-19 22:54   ` Alexander Duyck
  (?)
@ 2018-04-20  0:46     ` Michael S. Tsirkin
  -1 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20  0:46 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Bjorn Helgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm,
	Netdev, Daly, Dan, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Thu, Apr 19, 2018 at 03:54:49PM -0700, Alexander Duyck wrote:
> On Thu, Mar 15, 2018 at 11:40 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
> > 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
> > v7: Dropped pci_id table explanation from pci-pf-stub driver
> >     Updated pci_sriov_configure_simple to drop need for err value
> >     Fixed comment explaining why pci_sriov_configure_simple is NULL
> >
> 
> Just following up since this has been sitting in patchwork for just
> over a month now
> (https://patchwork.ozlabs.org/project/linux-pci/list/?series=34034).
> I'm just wondering what the expectation is on getting these pulled
> into the pci tree? I'm assuming that is the best place for these
> patches. Are there any concerns I still need to address or are these
> going to be pulled in at some point, and if so is there any ETA on
> when that will be?
> 
> Thanks.
> 
> - Alex

Sorry I didn't notice you had more questions. I have responded
hopefully explaining my concerns. Summary:

- For virtio we should add this with a feature bit.
- I am worried about security of this for the stub, but I am
  not the maintainer there.

-- 
MST

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [pci PATCH v7 0/5] Add support for unmanaged SR-IOV
@ 2018-04-20  0:46     ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20  0:46 UTC (permalink / raw)


On Thu, Apr 19, 2018@03:54:49PM -0700, Alexander Duyck wrote:
> On Thu, Mar 15, 2018 at 11:40 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
> > 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
> > v7: Dropped pci_id table explanation from pci-pf-stub driver
> >     Updated pci_sriov_configure_simple to drop need for err value
> >     Fixed comment explaining why pci_sriov_configure_simple is NULL
> >
> 
> Just following up since this has been sitting in patchwork for just
> over a month now
> (https://patchwork.ozlabs.org/project/linux-pci/list/?series=34034).
> I'm just wondering what the expectation is on getting these pulled
> into the pci tree? I'm assuming that is the best place for these
> patches. Are there any concerns I still need to address or are these
> going to be pulled in at some point, and if so is there any ETA on
> when that will be?
> 
> Thanks.
> 
> - Alex

Sorry I didn't notice you had more questions. I have responded
hopefully explaining my concerns. Summary:

- For virtio we should add this with a feature bit.
- I am worried about security of this for the stub, but I am
  not the maintainer there.

-- 
MST

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] Re: [pci PATCH v7 0/5] Add support for unmanaged SR-IOV
@ 2018-04-20  0:46     ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20  0:46 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Bjorn Helgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm,
	Netdev, Daly, Dan, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Thu, Apr 19, 2018 at 03:54:49PM -0700, Alexander Duyck wrote:
> On Thu, Mar 15, 2018 at 11:40 AM, Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
> > 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
> > v7: Dropped pci_id table explanation from pci-pf-stub driver
> >     Updated pci_sriov_configure_simple to drop need for err value
> >     Fixed comment explaining why pci_sriov_configure_simple is NULL
> >
> 
> Just following up since this has been sitting in patchwork for just
> over a month now
> (https://patchwork.ozlabs.org/project/linux-pci/list/?series=34034).
> I'm just wondering what the expectation is on getting these pulled
> into the pci tree? I'm assuming that is the best place for these
> patches. Are there any concerns I still need to address or are these
> going to be pulled in at some point, and if so is there any ETA on
> when that will be?
> 
> Thanks.
> 
> - Alex

Sorry I didn't notice you had more questions. I have responded
hopefully explaining my concerns. Summary:

- For virtio we should add this with a feature bit.
- I am worried about security of this for the stub, but I am
  not the maintainer there.

-- 
MST

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-04-20  0:40                 ` Michael S. Tsirkin
  (?)
@ 2018-04-20 14:56                   ` Alexander Duyck
  -1 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-20 14:56 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Thu, Apr 19, 2018 at 5:40 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Apr 03, 2018 at 12:06:03PM -0700, Alexander Duyck wrote:
>> On Tue, Apr 3, 2018 at 11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Tue, Apr 03, 2018 at 10:32:00AM -0700, Alexander Duyck wrote:
>> >> On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> > On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
>> >> >> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> >> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
>> >> >> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
>> >> >> >>
>> >> >> >> 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.
>> >> >> >>
>> >> >> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> >> >> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> >> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> >> >> >
>> >> >> > So if and when virtio PFs can manage the VFs, then we can
>> >> >> > add a feature bit for that?
>> >> >> > Seems reasonable.
>> >> >>
>> >> >> Yes. If nothing else you may not even need a feature bit depending on
>> >> >> how things go.
>> >> >
>> >> > OTOH if the interface is changed in an incompatible way,
>> >> > and old Linux will attempt to drive the new device
>> >> > since there is no check.
>> >> >
>> >> > I think we should add a feature bit right away.
>> >>
>> >> I'm not sure why you would need a feature bit. The capability is
>> >> controlled via PCI configuration space. If it is present the device
>> >> has the capability. If it is not then it does not.
>> >>
>> >> Basically if the PCI configuration space is not present then the sysfs
>> >> entries will not be spawned and nothing will attempt to use this
>> >> function.
>> >>
>> >> - ALex
>> >
>> > It's about compability with older guests which ignore the
>> > capability.
>> >
>> > The feature is thus helpful so host knows whether guest supports VFs.
>>
>> The thing is if the capability is ignored then the feature isn't used.
>> So for SR-IOV it isn't an uncommon thing for there to be drivers for
>> the PF floating around that do not support SR-IOV. In such cases
>> SR-IOV just isn't used while the hardware could support it.
>
> Right but how come there are VF drivers but PF driver does not
> know about these?

I'm not sure what you mean here. The VF and PF drivers are the same
driver. The only difference is that the PF has the extra SR-IOV
configuration space.

What this code is meant to enable is a form of SR-IOV where the VFs
are essentially pre-allocated resources. So for example in our case
the MMIO space is identical for a PF versus any of the VFs. It doesn't
have any special controls in place to allow the PF to manipulate any
of the resources belonging to the VFs.

> And are there PF drivers that intentially do not enable SRIOV
> because it's known to be broken in some way?

In the Virtio IO case right now are there any devices that support
SR-IOV? For now this is just an add-on bit to a function that is
already emulating the Virtio in hardware.

> Case in point I do think virtio want to limit this
> depending on a feature bit on general principles
> (the principle being that all extensions have feature bits).

This part has me kind of scratching my head. In our setup the "PF" is
really nothing more than a "VF" with the SR-IOV configuration space
attached to it. There are already examples of similar designs for NVMe
and the Amazon ENA devices. Giving the "PF" any functionality in MMIO
space that controls the SR-IOV kind of defeats the whole point of
allowing this function in the first place. Basically the PF isn't
really controlling things, it is the kernel that is doing it.

> There are security implications here - we previously relied on
> whitelisting after all.

Yes and no. The original patch set had issues as you could have a PF
assigned to user space and the VFs managed by the host. When I changed
things so that the function had to be in a kernel driver that issue
went away.

> Wouldn't it be safer to be a bit more careful and update the
> actual PF drivers? It's just one line per driver, but it
> can be done with an ack by driver maintainer.
> If/once we find out all drivers do have it, we can then
> change the default.

I have no clue what you are talking about here. This is the more
careful approach. Are you sure you are reviewing the v7 of the
patches?

My understanding is that no paravirtual interfaces currently expose
SR-IOV. What we are looking at is hardware will want to emulate
Virtio, specifically virtio_net in the future and as a part of that
the PF ends up emulating it as well. What we would need to watch for
going forward is that any device that enables SR-IOV support would
need to also provide a 4 tuple ID so that if something goes wrong with
it we could disable SR-IOV on the device via a PCI quirk later.

>> I would think in the case of virtio it would be the same kind of
>> thing. Basically if SR-IOV is supported by the host then the
>> capability would be present. If SR-IOV is supported by the guest then
>> it would make use of the capability to spawn VFs. If either the
>> capability isn't present, or the driver doesn't use it then you won't
>> be able to spawn VFs in the guest.
>
>> Maybe I am missing something. Do you support dynamically changing the
>> PCI configuration space for Virtio devices based on the presence of
>> feature bits provided by the guest?
>
> No. The point is that IMHO at least virtio - in absence of feature bit -
> to ignore VFs rather than assume they are safe to drive
> in an unmanaged way.
>
>> Also are you saying this patch set should wait on the feature bit to
>> be added, or are you talking about doing this as some sort of
>> follow-up?
>>
>> - Alex
>
> I think for virtio it should include the feature bit, yes.
> Adding feature bit is very easy - post a patch to the virtio TC mailing
> list, wait about a week to give people time to respond (two weeks if it
> is around holidays and such).

The problem is we are talking about hardware/FPGA, not software.
Adding a feature bit means going back and updating RTL. The software
side of things is easy, re-validating things after a hardware/FPGA
change not so much.

If this is a hard requirement I may just drop the virtio patch, push
what I have, and leave it to Mark/Dan to deal with the necessary RTL
and code changes needed to support Virtio as I don't expect the
turnaround to be as easy as just a patch.

Thanks.

- Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20 14:56                   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-20 14:56 UTC (permalink / raw)


On Thu, Apr 19, 2018@5:40 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Apr 03, 2018@12:06:03PM -0700, Alexander Duyck wrote:
>> On Tue, Apr 3, 2018@11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Tue, Apr 03, 2018@10:32:00AM -0700, Alexander Duyck wrote:
>> >> On Tue, Apr 3, 2018@6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> > On Fri, Mar 16, 2018@09:40:34AM -0700, Alexander Duyck wrote:
>> >> >> On Fri, Mar 16, 2018@9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> >> > On Thu, Mar 15, 2018@11:42:41AM -0700, Alexander Duyck wrote:
>> >> >> >> From: Alexander Duyck <alexander.h.duyck at intel.com>
>> >> >> >>
>> >> >> >> 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.
>> >> >> >>
>> >> >> >> Reviewed-by: Christoph Hellwig <hch at lst.de>
>> >> >> >> Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
>> >> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck at intel.com>
>> >> >> >
>> >> >> > So if and when virtio PFs can manage the VFs, then we can
>> >> >> > add a feature bit for that?
>> >> >> > Seems reasonable.
>> >> >>
>> >> >> Yes. If nothing else you may not even need a feature bit depending on
>> >> >> how things go.
>> >> >
>> >> > OTOH if the interface is changed in an incompatible way,
>> >> > and old Linux will attempt to drive the new device
>> >> > since there is no check.
>> >> >
>> >> > I think we should add a feature bit right away.
>> >>
>> >> I'm not sure why you would need a feature bit. The capability is
>> >> controlled via PCI configuration space. If it is present the device
>> >> has the capability. If it is not then it does not.
>> >>
>> >> Basically if the PCI configuration space is not present then the sysfs
>> >> entries will not be spawned and nothing will attempt to use this
>> >> function.
>> >>
>> >> - ALex
>> >
>> > It's about compability with older guests which ignore the
>> > capability.
>> >
>> > The feature is thus helpful so host knows whether guest supports VFs.
>>
>> The thing is if the capability is ignored then the feature isn't used.
>> So for SR-IOV it isn't an uncommon thing for there to be drivers for
>> the PF floating around that do not support SR-IOV. In such cases
>> SR-IOV just isn't used while the hardware could support it.
>
> Right but how come there are VF drivers but PF driver does not
> know about these?

I'm not sure what you mean here. The VF and PF drivers are the same
driver. The only difference is that the PF has the extra SR-IOV
configuration space.

What this code is meant to enable is a form of SR-IOV where the VFs
are essentially pre-allocated resources. So for example in our case
the MMIO space is identical for a PF versus any of the VFs. It doesn't
have any special controls in place to allow the PF to manipulate any
of the resources belonging to the VFs.

> And are there PF drivers that intentially do not enable SRIOV
> because it's known to be broken in some way?

In the Virtio IO case right now are there any devices that support
SR-IOV? For now this is just an add-on bit to a function that is
already emulating the Virtio in hardware.

> Case in point I do think virtio want to limit this
> depending on a feature bit on general principles
> (the principle being that all extensions have feature bits).

This part has me kind of scratching my head. In our setup the "PF" is
really nothing more than a "VF" with the SR-IOV configuration space
attached to it. There are already examples of similar designs for NVMe
and the Amazon ENA devices. Giving the "PF" any functionality in MMIO
space that controls the SR-IOV kind of defeats the whole point of
allowing this function in the first place. Basically the PF isn't
really controlling things, it is the kernel that is doing it.

> There are security implications here - we previously relied on
> whitelisting after all.

Yes and no. The original patch set had issues as you could have a PF
assigned to user space and the VFs managed by the host. When I changed
things so that the function had to be in a kernel driver that issue
went away.

> Wouldn't it be safer to be a bit more careful and update the
> actual PF drivers? It's just one line per driver, but it
> can be done with an ack by driver maintainer.
> If/once we find out all drivers do have it, we can then
> change the default.

I have no clue what you are talking about here. This is the more
careful approach. Are you sure you are reviewing the v7 of the
patches?

My understanding is that no paravirtual interfaces currently expose
SR-IOV. What we are looking at is hardware will want to emulate
Virtio, specifically virtio_net in the future and as a part of that
the PF ends up emulating it as well. What we would need to watch for
going forward is that any device that enables SR-IOV support would
need to also provide a 4 tuple ID so that if something goes wrong with
it we could disable SR-IOV on the device via a PCI quirk later.

>> I would think in the case of virtio it would be the same kind of
>> thing. Basically if SR-IOV is supported by the host then the
>> capability would be present. If SR-IOV is supported by the guest then
>> it would make use of the capability to spawn VFs. If either the
>> capability isn't present, or the driver doesn't use it then you won't
>> be able to spawn VFs in the guest.
>
>> Maybe I am missing something. Do you support dynamically changing the
>> PCI configuration space for Virtio devices based on the presence of
>> feature bits provided by the guest?
>
> No. The point is that IMHO at least virtio - in absence of feature bit -
> to ignore VFs rather than assume they are safe to drive
> in an unmanaged way.
>
>> Also are you saying this patch set should wait on the feature bit to
>> be added, or are you talking about doing this as some sort of
>> follow-up?
>>
>> - Alex
>
> I think for virtio it should include the feature bit, yes.
> Adding feature bit is very easy - post a patch to the virtio TC mailing
> list, wait about a week to give people time to respond (two weeks if it
> is around holidays and such).

The problem is we are talking about hardware/FPGA, not software.
Adding a feature bit means going back and updating RTL. The software
side of things is easy, re-validating things after a hardware/FPGA
change not so much.

If this is a hard requirement I may just drop the virtio patch, push
what I have, and leave it to Mark/Dan to deal with the necessary RTL
and code changes needed to support Virtio as I don't expect the
turnaround to be as easy as just a patch.

Thanks.

- Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20 14:56                   ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-20 14:56 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Thu, Apr 19, 2018 at 5:40 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Apr 03, 2018 at 12:06:03PM -0700, Alexander Duyck wrote:
>> On Tue, Apr 3, 2018 at 11:27 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Tue, Apr 03, 2018 at 10:32:00AM -0700, Alexander Duyck wrote:
>> >> On Tue, Apr 3, 2018 at 6:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> > On Fri, Mar 16, 2018 at 09:40:34AM -0700, Alexander Duyck wrote:
>> >> >> On Fri, Mar 16, 2018 at 9:34 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> >> > On Thu, Mar 15, 2018 at 11:42:41AM -0700, Alexander Duyck wrote:
>> >> >> >> From: Alexander Duyck <alexander.h.duyck@intel.com>
>> >> >> >>
>> >> >> >> 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.
>> >> >> >>
>> >> >> >> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> >> >> >> Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
>> >> >> >> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> >> >> >
>> >> >> > So if and when virtio PFs can manage the VFs, then we can
>> >> >> > add a feature bit for that?
>> >> >> > Seems reasonable.
>> >> >>
>> >> >> Yes. If nothing else you may not even need a feature bit depending on
>> >> >> how things go.
>> >> >
>> >> > OTOH if the interface is changed in an incompatible way,
>> >> > and old Linux will attempt to drive the new device
>> >> > since there is no check.
>> >> >
>> >> > I think we should add a feature bit right away.
>> >>
>> >> I'm not sure why you would need a feature bit. The capability is
>> >> controlled via PCI configuration space. If it is present the device
>> >> has the capability. If it is not then it does not.
>> >>
>> >> Basically if the PCI configuration space is not present then the sysfs
>> >> entries will not be spawned and nothing will attempt to use this
>> >> function.
>> >>
>> >> - ALex
>> >
>> > It's about compability with older guests which ignore the
>> > capability.
>> >
>> > The feature is thus helpful so host knows whether guest supports VFs.
>>
>> The thing is if the capability is ignored then the feature isn't used.
>> So for SR-IOV it isn't an uncommon thing for there to be drivers for
>> the PF floating around that do not support SR-IOV. In such cases
>> SR-IOV just isn't used while the hardware could support it.
>
> Right but how come there are VF drivers but PF driver does not
> know about these?

I'm not sure what you mean here. The VF and PF drivers are the same
driver. The only difference is that the PF has the extra SR-IOV
configuration space.

What this code is meant to enable is a form of SR-IOV where the VFs
are essentially pre-allocated resources. So for example in our case
the MMIO space is identical for a PF versus any of the VFs. It doesn't
have any special controls in place to allow the PF to manipulate any
of the resources belonging to the VFs.

> And are there PF drivers that intentially do not enable SRIOV
> because it's known to be broken in some way?

In the Virtio IO case right now are there any devices that support
SR-IOV? For now this is just an add-on bit to a function that is
already emulating the Virtio in hardware.

> Case in point I do think virtio want to limit this
> depending on a feature bit on general principles
> (the principle being that all extensions have feature bits).

This part has me kind of scratching my head. In our setup the "PF" is
really nothing more than a "VF" with the SR-IOV configuration space
attached to it. There are already examples of similar designs for NVMe
and the Amazon ENA devices. Giving the "PF" any functionality in MMIO
space that controls the SR-IOV kind of defeats the whole point of
allowing this function in the first place. Basically the PF isn't
really controlling things, it is the kernel that is doing it.

> There are security implications here - we previously relied on
> whitelisting after all.

Yes and no. The original patch set had issues as you could have a PF
assigned to user space and the VFs managed by the host. When I changed
things so that the function had to be in a kernel driver that issue
went away.

> Wouldn't it be safer to be a bit more careful and update the
> actual PF drivers? It's just one line per driver, but it
> can be done with an ack by driver maintainer.
> If/once we find out all drivers do have it, we can then
> change the default.

I have no clue what you are talking about here. This is the more
careful approach. Are you sure you are reviewing the v7 of the
patches?

My understanding is that no paravirtual interfaces currently expose
SR-IOV. What we are looking at is hardware will want to emulate
Virtio, specifically virtio_net in the future and as a part of that
the PF ends up emulating it as well. What we would need to watch for
going forward is that any device that enables SR-IOV support would
need to also provide a 4 tuple ID so that if something goes wrong with
it we could disable SR-IOV on the device via a PCI quirk later.

>> I would think in the case of virtio it would be the same kind of
>> thing. Basically if SR-IOV is supported by the host then the
>> capability would be present. If SR-IOV is supported by the guest then
>> it would make use of the capability to spawn VFs. If either the
>> capability isn't present, or the driver doesn't use it then you won't
>> be able to spawn VFs in the guest.
>
>> Maybe I am missing something. Do you support dynamically changing the
>> PCI configuration space for Virtio devices based on the presence of
>> feature bits provided by the guest?
>
> No. The point is that IMHO at least virtio - in absence of feature bit -
> to ignore VFs rather than assume they are safe to drive
> in an unmanaged way.
>
>> Also are you saying this patch set should wait on the feature bit to
>> be added, or are you talking about doing this as some sort of
>> follow-up?
>>
>> - Alex
>
> I think for virtio it should include the feature bit, yes.
> Adding feature bit is very easy - post a patch to the virtio TC mailing
> list, wait about a week to give people time to respond (two weeks if it
> is around holidays and such).

The problem is we are talking about hardware/FPGA, not software.
Adding a feature bit means going back and updating RTL. The software
side of things is easy, re-validating things after a hardware/FPGA
change not so much.

If this is a hard requirement I may just drop the virtio patch, push
what I have, and leave it to Mark/Dan to deal with the necessary RTL
and code changes needed to support Virtio as I don't expect the
turnaround to be as easy as just a patch.

Thanks.

- Alex

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-04-20 14:56                   ` Alexander Duyck
  (?)
  (?)
@ 2018-04-20 15:28                     ` Michael S. Tsirkin
  -1 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20 15:28 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Fri, Apr 20, 2018 at 07:56:14AM -0700, Alexander Duyck wrote:
> > I think for virtio it should include the feature bit, yes.
> > Adding feature bit is very easy - post a patch to the virtio TC mailing
> > list, wait about a week to give people time to respond (two weeks if it
> > is around holidays and such).
> 
> The problem is we are talking about hardware/FPGA, not software.
> Adding a feature bit means going back and updating RTL. The software
> side of things is easy, re-validating things after a hardware/FPGA
> change not so much.
> 
> If this is a hard requirement I may just drop the virtio patch, push
> what I have, and leave it to Mark/Dan to deal with the necessary RTL
> and code changes needed to support Virtio as I don't expect the
> turnaround to be as easy as just a patch.
> 
> Thanks.
> 
> - Alex

Let's focus on virtio in this thread.

Involving the virtio TC in host/guest interface changes is a
hard requirement. It's just too easy to create conflicts otherwise.

So you guys should have just sent the proposal to the TC when you
were doing your RTL and you would have been in the clear.

Generally adding a feature bit with any extension is a good idea:
this way you merely reserve a feature bit for your feature through
the TC and are more or less sure of forward and backward compatibility.
It's incredibly easy.

But maybe it's not needed here.  I am not making the decisions myself.
Not too late: post to the TC list and let's see what the response is.
Without a feature bit you are making a change affecting all future
implementations without exception so the bar is a bit higher: you need
to actually post a spec text proposal not just a patch showing how to
use the feature, and TC needs to vote on it. Voting takes a week,
review a week or two depending on change complexity.

Hope this helps,

-- 
MST

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20 15:28                     ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20 15:28 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Fri, Apr 20, 2018 at 07:56:14AM -0700, Alexander Duyck wrote:
> > I think for virtio it should include the feature bit, yes.
> > Adding feature bit is very easy - post a patch to the virtio TC mailing
> > list, wait about a week to give people time to respond (two weeks if it
> > is around holidays and such).
> 
> The problem is we are talking about hardware/FPGA, not software.
> Adding a feature bit means going back and updating RTL. The software
> side of things is easy, re-validating things after a hardware/FPGA
> change not so much.
> 
> If this is a hard requirement I may just drop the virtio patch, push
> what I have, and leave it to Mark/Dan to deal with the necessary RTL
> and code changes needed to support Virtio as I don't expect the
> turnaround to be as easy as just a patch.
> 
> Thanks.
> 
> - Alex

Let's focus on virtio in this thread.

Involving the virtio TC in host/guest interface changes is a
hard requirement. It's just too easy to create conflicts otherwise.

So you guys should have just sent the proposal to the TC when you
were doing your RTL and you would have been in the clear.

Generally adding a feature bit with any extension is a good idea:
this way you merely reserve a feature bit for your feature through
the TC and are more or less sure of forward and backward compatibility.
It's incredibly easy.

But maybe it's not needed here.  I am not making the decisions myself.
Not too late: post to the TC list and let's see what the response is.
Without a feature bit you are making a change affecting all future
implementations without exception so the bar is a bit higher: you need
to actually post a spec text proposal not just a patch showing how to
use the feature, and TC needs to vote on it. Voting takes a week,
review a week or two depending on change complexity.

Hope this helps,

-- 
MST

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20 15:28                     ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20 15:28 UTC (permalink / raw)


On Fri, Apr 20, 2018@07:56:14AM -0700, Alexander Duyck wrote:
> > I think for virtio it should include the feature bit, yes.
> > Adding feature bit is very easy - post a patch to the virtio TC mailing
> > list, wait about a week to give people time to respond (two weeks if it
> > is around holidays and such).
> 
> The problem is we are talking about hardware/FPGA, not software.
> Adding a feature bit means going back and updating RTL. The software
> side of things is easy, re-validating things after a hardware/FPGA
> change not so much.
> 
> If this is a hard requirement I may just drop the virtio patch, push
> what I have, and leave it to Mark/Dan to deal with the necessary RTL
> and code changes needed to support Virtio as I don't expect the
> turnaround to be as easy as just a patch.
> 
> Thanks.
> 
> - Alex

Let's focus on virtio in this thread.

Involving the virtio TC in host/guest interface changes is a
hard requirement. It's just too easy to create conflicts otherwise.

So you guys should have just sent the proposal to the TC when you
were doing your RTL and you would have been in the clear.

Generally adding a feature bit with any extension is a good idea:
this way you merely reserve a feature bit for your feature through
the TC and are more or less sure of forward and backward compatibility.
It's incredibly easy.

But maybe it's not needed here.  I am not making the decisions myself.
Not too late: post to the TC list and let's see what the response is.
Without a feature bit you are making a change affecting all future
implementations without exception so the bar is a bit higher: you need
to actually post a spec text proposal not just a patch showing how to
use the feature, and TC needs to vote on it. Voting takes a week,
review a week or two depending on change complexity.

Hope this helps,

-- 
MST

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20 15:28                     ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20 15:28 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Bjorn Helgaas, Duyck, Alexander H, linux-pci,
	virtio-dev, kvm, Netdev, LKML, linux-nvme, Keith Busch, netanel,
	Don Dutile, Maximilian Heyne, Wang, Liang-min, Rustad, Mark D,
	David Woodhouse, Christoph Hellwig, dwmw

On Fri, Apr 20, 2018 at 07:56:14AM -0700, Alexander Duyck wrote:
> > I think for virtio it should include the feature bit, yes.
> > Adding feature bit is very easy - post a patch to the virtio TC mailing
> > list, wait about a week to give people time to respond (two weeks if it
> > is around holidays and such).
> 
> The problem is we are talking about hardware/FPGA, not software.
> Adding a feature bit means going back and updating RTL. The software
> side of things is easy, re-validating things after a hardware/FPGA
> change not so much.
> 
> If this is a hard requirement I may just drop the virtio patch, push
> what I have, and leave it to Mark/Dan to deal with the necessary RTL
> and code changes needed to support Virtio as I don't expect the
> turnaround to be as easy as just a patch.
> 
> Thanks.
> 
> - Alex

Let's focus on virtio in this thread.

Involving the virtio TC in host/guest interface changes is a
hard requirement. It's just too easy to create conflicts otherwise.

So you guys should have just sent the proposal to the TC when you
were doing your RTL and you would have been in the clear.

Generally adding a feature bit with any extension is a good idea:
this way you merely reserve a feature bit for your feature through
the TC and are more or less sure of forward and backward compatibility.
It's incredibly easy.

But maybe it's not needed here.  I am not making the decisions myself.
Not too late: post to the TC list and let's see what the response is.
Without a feature bit you are making a change affecting all future
implementations without exception so the bar is a bit higher: you need
to actually post a spec text proposal not just a patch showing how to
use the feature, and TC needs to vote on it. Voting takes a week,
review a week or two depending on change complexity.

Hope this helps,

-- 
MST

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-04-20 15:28                     ` Michael S. Tsirkin
  (?)
@ 2018-04-20 16:08                       ` Alexander Duyck
  -1 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-20 16:08 UTC (permalink / raw)
  To: Michael S. Tsirkin, Daly, Dan, Rustad, Mark D
  Cc: Bjorn Helgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm,
	Netdev, LKML, linux-nvme, Keith Busch, netanel, Don Dutile,
	Maximilian Heyne, Wang, Liang-min, David Woodhouse,
	Christoph Hellwig, dwmw

On Fri, Apr 20, 2018 at 8:28 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Apr 20, 2018 at 07:56:14AM -0700, Alexander Duyck wrote:
>> > I think for virtio it should include the feature bit, yes.
>> > Adding feature bit is very easy - post a patch to the virtio TC mailing
>> > list, wait about a week to give people time to respond (two weeks if it
>> > is around holidays and such).
>>
>> The problem is we are talking about hardware/FPGA, not software.
>> Adding a feature bit means going back and updating RTL. The software
>> side of things is easy, re-validating things after a hardware/FPGA
>> change not so much.
>>
>> If this is a hard requirement I may just drop the virtio patch, push
>> what I have, and leave it to Mark/Dan to deal with the necessary RTL
>> and code changes needed to support Virtio as I don't expect the
>> turnaround to be as easy as just a patch.
>>
>> Thanks.
>>
>> - Alex
>
> Let's focus on virtio in this thread.

That is kind of what I was thinking, and why I was thinking it might
make sense to make the virtio specific changes a separate patch set. I
could get the PCI bits taken care of in the meantime since they effect
genetic PCI, NVMe, and the Amazon ENA interfaces.

> Involving the virtio TC in host/guest interface changes is a
> hard requirement. It's just too easy to create conflicts otherwise.
>
> So you guys should have just sent the proposal to the TC when you
> were doing your RTL and you would have been in the clear.

Agreed. I believe I brought this up when I was originally asked to
look into the coding for this.

> Generally adding a feature bit with any extension is a good idea:
> this way you merely reserve a feature bit for your feature through
> the TC and are more or less sure of forward and backward compatibility.
> It's incredibly easy.

Agreed, though in this case I am not sure it makes sense since this
isn't necessarily something that is a Virtio feature itself. It is
just a side effect of the fact that they are adding SR-IOV support to
a device that happens to emulate Virtio NET and apparently their PF
has to be identical to the VF other than the PCIe extended config
space.

> But maybe it's not needed here.  I am not making the decisions myself.
> Not too late: post to the TC list and let's see what the response is.
> Without a feature bit you are making a change affecting all future
> implementations without exception so the bar is a bit higher: you need
> to actually post a spec text proposal not just a patch showing how to
> use the feature, and TC needs to vote on it. Voting takes a week,
> review a week or two depending on change complexity.
>
> Hope this helps,
>
> --
> MST

I think I will leave this for Dan and Mark to handle since I am still
not all that familiar with the hardware in use here. Once a decision
has been made him and Mark could look at pushing either the one line
patch or something more complex involving a feature flag.

Thanks.

Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20 16:08                       ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-20 16:08 UTC (permalink / raw)


On Fri, Apr 20, 2018@8:28 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Apr 20, 2018@07:56:14AM -0700, Alexander Duyck wrote:
>> > I think for virtio it should include the feature bit, yes.
>> > Adding feature bit is very easy - post a patch to the virtio TC mailing
>> > list, wait about a week to give people time to respond (two weeks if it
>> > is around holidays and such).
>>
>> The problem is we are talking about hardware/FPGA, not software.
>> Adding a feature bit means going back and updating RTL. The software
>> side of things is easy, re-validating things after a hardware/FPGA
>> change not so much.
>>
>> If this is a hard requirement I may just drop the virtio patch, push
>> what I have, and leave it to Mark/Dan to deal with the necessary RTL
>> and code changes needed to support Virtio as I don't expect the
>> turnaround to be as easy as just a patch.
>>
>> Thanks.
>>
>> - Alex
>
> Let's focus on virtio in this thread.

That is kind of what I was thinking, and why I was thinking it might
make sense to make the virtio specific changes a separate patch set. I
could get the PCI bits taken care of in the meantime since they effect
genetic PCI, NVMe, and the Amazon ENA interfaces.

> Involving the virtio TC in host/guest interface changes is a
> hard requirement. It's just too easy to create conflicts otherwise.
>
> So you guys should have just sent the proposal to the TC when you
> were doing your RTL and you would have been in the clear.

Agreed. I believe I brought this up when I was originally asked to
look into the coding for this.

> Generally adding a feature bit with any extension is a good idea:
> this way you merely reserve a feature bit for your feature through
> the TC and are more or less sure of forward and backward compatibility.
> It's incredibly easy.

Agreed, though in this case I am not sure it makes sense since this
isn't necessarily something that is a Virtio feature itself. It is
just a side effect of the fact that they are adding SR-IOV support to
a device that happens to emulate Virtio NET and apparently their PF
has to be identical to the VF other than the PCIe extended config
space.

> But maybe it's not needed here.  I am not making the decisions myself.
> Not too late: post to the TC list and let's see what the response is.
> Without a feature bit you are making a change affecting all future
> implementations without exception so the bar is a bit higher: you need
> to actually post a spec text proposal not just a patch showing how to
> use the feature, and TC needs to vote on it. Voting takes a week,
> review a week or two depending on change complexity.
>
> Hope this helps,
>
> --
> MST

I think I will leave this for Dan and Mark to handle since I am still
not all that familiar with the hardware in use here. Once a decision
has been made him and Mark could look at pushing either the one line
patch or something more complex involving a feature flag.

Thanks.

Alex

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20 16:08                       ` Alexander Duyck
  0 siblings, 0 replies; 78+ messages in thread
From: Alexander Duyck @ 2018-04-20 16:08 UTC (permalink / raw)
  To: Michael S. Tsirkin, Daly, Dan, Rustad, Mark D
  Cc: Bjorn Helgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm,
	Netdev, LKML, linux-nvme, Keith Busch, netanel, Don Dutile,
	Maximilian Heyne, Wang, Liang-min, David Woodhouse,
	Christoph Hellwig, dwmw

On Fri, Apr 20, 2018 at 8:28 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Apr 20, 2018 at 07:56:14AM -0700, Alexander Duyck wrote:
>> > I think for virtio it should include the feature bit, yes.
>> > Adding feature bit is very easy - post a patch to the virtio TC mailing
>> > list, wait about a week to give people time to respond (two weeks if it
>> > is around holidays and such).
>>
>> The problem is we are talking about hardware/FPGA, not software.
>> Adding a feature bit means going back and updating RTL. The software
>> side of things is easy, re-validating things after a hardware/FPGA
>> change not so much.
>>
>> If this is a hard requirement I may just drop the virtio patch, push
>> what I have, and leave it to Mark/Dan to deal with the necessary RTL
>> and code changes needed to support Virtio as I don't expect the
>> turnaround to be as easy as just a patch.
>>
>> Thanks.
>>
>> - Alex
>
> Let's focus on virtio in this thread.

That is kind of what I was thinking, and why I was thinking it might
make sense to make the virtio specific changes a separate patch set. I
could get the PCI bits taken care of in the meantime since they effect
genetic PCI, NVMe, and the Amazon ENA interfaces.

> Involving the virtio TC in host/guest interface changes is a
> hard requirement. It's just too easy to create conflicts otherwise.
>
> So you guys should have just sent the proposal to the TC when you
> were doing your RTL and you would have been in the clear.

Agreed. I believe I brought this up when I was originally asked to
look into the coding for this.

> Generally adding a feature bit with any extension is a good idea:
> this way you merely reserve a feature bit for your feature through
> the TC and are more or less sure of forward and backward compatibility.
> It's incredibly easy.

Agreed, though in this case I am not sure it makes sense since this
isn't necessarily something that is a Virtio feature itself. It is
just a side effect of the fact that they are adding SR-IOV support to
a device that happens to emulate Virtio NET and apparently their PF
has to be identical to the VF other than the PCIe extended config
space.

> But maybe it's not needed here.  I am not making the decisions myself.
> Not too late: post to the TC list and let's see what the response is.
> Without a feature bit you are making a change affecting all future
> implementations without exception so the bar is a bit higher: you need
> to actually post a spec text proposal not just a patch showing how to
> use the feature, and TC needs to vote on it. Voting takes a week,
> review a week or two depending on change complexity.
>
> Hope this helps,
>
> --
> MST

I think I will leave this for Dan and Mark to handle since I am still
not all that familiar with the hardware in use here. Once a decision
has been made him and Mark could look at pushing either the one line
patch or something more complex involving a feature flag.

Thanks.

Alex

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-04-20 16:08                       ` Alexander Duyck
  (?)
@ 2018-04-20 16:14                         ` Michael S. Tsirkin
  -1 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20 16:14 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Rustad, Mark D, Bjorn Helgaas, Duyck, Alexander H,
	linux-pci, virtio-dev, kvm, Netdev, LKML, linux-nvme,
	Keith Busch, netanel, Don Dutile, Maximilian Heyne, Wang,
	Liang-min, David Woodhouse, Christoph Hellwig, dwmw

On Fri, Apr 20, 2018 at 09:08:51AM -0700, Alexander Duyck wrote:
> On Fri, Apr 20, 2018 at 8:28 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Apr 20, 2018 at 07:56:14AM -0700, Alexander Duyck wrote:
> >> > I think for virtio it should include the feature bit, yes.
> >> > Adding feature bit is very easy - post a patch to the virtio TC mailing
> >> > list, wait about a week to give people time to respond (two weeks if it
> >> > is around holidays and such).
> >>
> >> The problem is we are talking about hardware/FPGA, not software.
> >> Adding a feature bit means going back and updating RTL. The software
> >> side of things is easy, re-validating things after a hardware/FPGA
> >> change not so much.
> >>
> >> If this is a hard requirement I may just drop the virtio patch, push
> >> what I have, and leave it to Mark/Dan to deal with the necessary RTL
> >> and code changes needed to support Virtio as I don't expect the
> >> turnaround to be as easy as just a patch.
> >>
> >> Thanks.
> >>
> >> - Alex
> >
> > Let's focus on virtio in this thread.
> 
> That is kind of what I was thinking, and why I was thinking it might
> make sense to make the virtio specific changes a separate patch set. I
> could get the PCI bits taken care of in the meantime since they effect
> genetic PCI, NVMe, and the Amazon ENA interfaces.
> 
> > Involving the virtio TC in host/guest interface changes is a
> > hard requirement. It's just too easy to create conflicts otherwise.
> >
> > So you guys should have just sent the proposal to the TC when you
> > were doing your RTL and you would have been in the clear.
> 
> Agreed. I believe I brought this up when I was originally asked to
> look into the coding for this.
> 
> > Generally adding a feature bit with any extension is a good idea:
> > this way you merely reserve a feature bit for your feature through
> > the TC and are more or less sure of forward and backward compatibility.
> > It's incredibly easy.
> 
> Agreed, though in this case I am not sure it makes sense since this
> isn't necessarily something that is a Virtio feature itself. It is
> just a side effect of the fact that they are adding SR-IOV support to
> a device that happens to emulate Virtio NET and apparently their PF
> has to be identical to the VF other than the PCIe extended config
> space.

I got that. My point is not everyone implementing SR-IOV will
want to do it like this. Others might want to have VFs
be different from PFs somehow. Feature bits ensure forward
not just backward compatibility.


> > But maybe it's not needed here.  I am not making the decisions myself.
> > Not too late: post to the TC list and let's see what the response is.
> > Without a feature bit you are making a change affecting all future
> > implementations without exception so the bar is a bit higher: you need
> > to actually post a spec text proposal not just a patch showing how to
> > use the feature, and TC needs to vote on it. Voting takes a week,
> > review a week or two depending on change complexity.
> >
> > Hope this helps,
> >
> > --
> > MST
> 
> I think I will leave this for Dan and Mark to handle since I am still
> not all that familiar with the hardware in use here. Once a decision
> has been made him and Mark could look at pushing either the one line
> patch or something more complex involving a feature flag.
> 
> Thanks.
> 
> Alex

As long as the TC is involved.

I know it's a bit of a strange thing to block it at the driver level,
the issue is with the device, but it's literally the only handle I have
to prevent people from doing out of spec hacks then pushing it all on us
to maintain.

-- 
MST

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20 16:14                         ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20 16:14 UTC (permalink / raw)


On Fri, Apr 20, 2018@09:08:51AM -0700, Alexander Duyck wrote:
> On Fri, Apr 20, 2018@8:28 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Apr 20, 2018@07:56:14AM -0700, Alexander Duyck wrote:
> >> > I think for virtio it should include the feature bit, yes.
> >> > Adding feature bit is very easy - post a patch to the virtio TC mailing
> >> > list, wait about a week to give people time to respond (two weeks if it
> >> > is around holidays and such).
> >>
> >> The problem is we are talking about hardware/FPGA, not software.
> >> Adding a feature bit means going back and updating RTL. The software
> >> side of things is easy, re-validating things after a hardware/FPGA
> >> change not so much.
> >>
> >> If this is a hard requirement I may just drop the virtio patch, push
> >> what I have, and leave it to Mark/Dan to deal with the necessary RTL
> >> and code changes needed to support Virtio as I don't expect the
> >> turnaround to be as easy as just a patch.
> >>
> >> Thanks.
> >>
> >> - Alex
> >
> > Let's focus on virtio in this thread.
> 
> That is kind of what I was thinking, and why I was thinking it might
> make sense to make the virtio specific changes a separate patch set. I
> could get the PCI bits taken care of in the meantime since they effect
> genetic PCI, NVMe, and the Amazon ENA interfaces.
> 
> > Involving the virtio TC in host/guest interface changes is a
> > hard requirement. It's just too easy to create conflicts otherwise.
> >
> > So you guys should have just sent the proposal to the TC when you
> > were doing your RTL and you would have been in the clear.
> 
> Agreed. I believe I brought this up when I was originally asked to
> look into the coding for this.
> 
> > Generally adding a feature bit with any extension is a good idea:
> > this way you merely reserve a feature bit for your feature through
> > the TC and are more or less sure of forward and backward compatibility.
> > It's incredibly easy.
> 
> Agreed, though in this case I am not sure it makes sense since this
> isn't necessarily something that is a Virtio feature itself. It is
> just a side effect of the fact that they are adding SR-IOV support to
> a device that happens to emulate Virtio NET and apparently their PF
> has to be identical to the VF other than the PCIe extended config
> space.

I got that. My point is not everyone implementing SR-IOV will
want to do it like this. Others might want to have VFs
be different from PFs somehow. Feature bits ensure forward
not just backward compatibility.


> > But maybe it's not needed here.  I am not making the decisions myself.
> > Not too late: post to the TC list and let's see what the response is.
> > Without a feature bit you are making a change affecting all future
> > implementations without exception so the bar is a bit higher: you need
> > to actually post a spec text proposal not just a patch showing how to
> > use the feature, and TC needs to vote on it. Voting takes a week,
> > review a week or two depending on change complexity.
> >
> > Hope this helps,
> >
> > --
> > MST
> 
> I think I will leave this for Dan and Mark to handle since I am still
> not all that familiar with the hardware in use here. Once a decision
> has been made him and Mark could look at pushing either the one line
> patch or something more complex involving a feature flag.
> 
> Thanks.
> 
> Alex

As long as the TC is involved.

I know it's a bit of a strange thing to block it at the driver level,
the issue is with the device, but it's literally the only handle I have
to prevent people from doing out of spec hacks then pushing it all on us
to maintain.

-- 
MST

^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-20 16:14                         ` Michael S. Tsirkin
  0 siblings, 0 replies; 78+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20 16:14 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daly, Dan, Rustad, Mark D, Bjorn Helgaas, Duyck, Alexander H,
	linux-pci, virtio-dev, kvm, Netdev, LKML, linux-nvme,
	Keith Busch, netanel, Don Dutile, Maximilian Heyne, Wang,
	Liang-min, David Woodhouse, Christoph Hellwig, dwmw

On Fri, Apr 20, 2018 at 09:08:51AM -0700, Alexander Duyck wrote:
> On Fri, Apr 20, 2018 at 8:28 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Apr 20, 2018 at 07:56:14AM -0700, Alexander Duyck wrote:
> >> > I think for virtio it should include the feature bit, yes.
> >> > Adding feature bit is very easy - post a patch to the virtio TC mailing
> >> > list, wait about a week to give people time to respond (two weeks if it
> >> > is around holidays and such).
> >>
> >> The problem is we are talking about hardware/FPGA, not software.
> >> Adding a feature bit means going back and updating RTL. The software
> >> side of things is easy, re-validating things after a hardware/FPGA
> >> change not so much.
> >>
> >> If this is a hard requirement I may just drop the virtio patch, push
> >> what I have, and leave it to Mark/Dan to deal with the necessary RTL
> >> and code changes needed to support Virtio as I don't expect the
> >> turnaround to be as easy as just a patch.
> >>
> >> Thanks.
> >>
> >> - Alex
> >
> > Let's focus on virtio in this thread.
> 
> That is kind of what I was thinking, and why I was thinking it might
> make sense to make the virtio specific changes a separate patch set. I
> could get the PCI bits taken care of in the meantime since they effect
> genetic PCI, NVMe, and the Amazon ENA interfaces.
> 
> > Involving the virtio TC in host/guest interface changes is a
> > hard requirement. It's just too easy to create conflicts otherwise.
> >
> > So you guys should have just sent the proposal to the TC when you
> > were doing your RTL and you would have been in the clear.
> 
> Agreed. I believe I brought this up when I was originally asked to
> look into the coding for this.
> 
> > Generally adding a feature bit with any extension is a good idea:
> > this way you merely reserve a feature bit for your feature through
> > the TC and are more or less sure of forward and backward compatibility.
> > It's incredibly easy.
> 
> Agreed, though in this case I am not sure it makes sense since this
> isn't necessarily something that is a Virtio feature itself. It is
> just a side effect of the fact that they are adding SR-IOV support to
> a device that happens to emulate Virtio NET and apparently their PF
> has to be identical to the VF other than the PCIe extended config
> space.

I got that. My point is not everyone implementing SR-IOV will
want to do it like this. Others might want to have VFs
be different from PFs somehow. Feature bits ensure forward
not just backward compatibility.


> > But maybe it's not needed here.  I am not making the decisions myself.
> > Not too late: post to the TC list and let's see what the response is.
> > Without a feature bit you are making a change affecting all future
> > implementations without exception so the bar is a bit higher: you need
> > to actually post a spec text proposal not just a patch showing how to
> > use the feature, and TC needs to vote on it. Voting takes a week,
> > review a week or two depending on change complexity.
> >
> > Hope this helps,
> >
> > --
> > MST
> 
> I think I will leave this for Dan and Mark to handle since I am still
> not all that familiar with the hardware in use here. Once a decision
> has been made him and Mark could look at pushing either the one line
> patch or something more complex involving a feature flag.
> 
> Thanks.
> 
> Alex

As long as the TC is involved.

I know it's a bit of a strange thing to block it at the driver level,
the issue is with the device, but it's literally the only handle I have
to prevent people from doing out of spec hacks then pushing it all on us
to maintain.

-- 
MST

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 78+ messages in thread

* Re: [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-04-20 15:28                     ` Michael S. Tsirkin
@ 2018-04-21  7:05                       ` Christoph Hellwig
  -1 siblings, 0 replies; 78+ messages in thread
From: Christoph Hellwig @ 2018-04-21  7:05 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Alexander Duyck, Daly, Dan, Bjorn Helgaas, Duyck, Alexander H,
	linux-pci, virtio-dev, kvm, Netdev, LKML, linux-nvme,
	Keith Busch, netanel, Don Dutile, Maximilian Heyne, Wang,
	Liang-min, Rustad, Mark D, David Woodhouse, Christoph Hellwig,
	dwmw

On Fri, Apr 20, 2018 at 06:28:50PM +0300, Michael S. Tsirkin wrote:
> But maybe it's not needed here.  I am not making the decisions myself.
> Not too late: post to the TC list and let's see what the response is.
> Without a feature bit you are making a change affecting all future
> implementations without exception so the bar is a bit higher: you need
> to actually post a spec text proposal not just a patch showing how to
> use the feature, and TC needs to vote on it. Voting takes a week,
> review a week or two depending on change complexity.

Also IFF the hardware already is out we can quirk it in the PCI ID
table to manually set the feature in the driver as a workaround.

^ permalink raw reply	[flat|nested] 78+ messages in thread

* [virtio-dev] [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
@ 2018-04-21  7:05                       ` Christoph Hellwig
  0 siblings, 0 replies; 78+ messages in thread
From: Christoph Hellwig @ 2018-04-21  7:05 UTC (permalink / raw)


On Fri, Apr 20, 2018@06:28:50PM +0300, Michael S. Tsirkin wrote:
> But maybe it's not needed here.  I am not making the decisions myself.
> Not too late: post to the TC list and let's see what the response is.
> Without a feature bit you are making a change affecting all future
> implementations without exception so the bar is a bit higher: you need
> to actually post a spec text proposal not just a patch showing how to
> use the feature, and TC needs to vote on it. Voting takes a week,
> review a week or two depending on change complexity.

Also IFF the hardware already is out we can quirk it in the PCI ID
table to manually set the feature in the driver as a workaround.

^ permalink raw reply	[flat|nested] 78+ messages in thread

end of thread, other threads:[~2018-04-21  7:05 UTC | newest]

Thread overview: 78+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-15 18:40 [pci PATCH v7 0/5] Add support for unmanaged SR-IOV Alexander Duyck
2018-03-15 18:40 ` [virtio-dev] " Alexander Duyck
2018-03-15 18:40 ` Alexander Duyck
2018-03-15 18:41 ` [pci PATCH v7 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources Alexander Duyck
2018-03-15 18:41   ` [virtio-dev] " Alexander Duyck
2018-03-15 18:41   ` Alexander Duyck
2018-03-28 21:30   ` [virtio-dev] " Rustad, Mark D
2018-03-28 21:30     ` Rustad, Mark D
2018-03-28 21:30     ` Rustad, Mark D
2018-03-15 18:42 ` [pci PATCH v7 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices Alexander Duyck
2018-03-15 18:42   ` [virtio-dev] " Alexander Duyck
2018-03-15 18:42   ` Alexander Duyck
2018-03-16 16:34   ` [virtio-dev] " Michael S. Tsirkin
2018-03-16 16:34     ` Michael S. Tsirkin
2018-03-16 16:34     ` Michael S. Tsirkin
2018-03-16 16:40     ` Alexander Duyck
2018-03-16 16:40       ` Alexander Duyck
2018-03-16 16:40       ` Alexander Duyck
2018-04-03 13:12       ` Michael S. Tsirkin
2018-04-03 13:12         ` Michael S. Tsirkin
2018-04-03 13:12         ` Michael S. Tsirkin
2018-04-03 17:32         ` Alexander Duyck
2018-04-03 17:32           ` Alexander Duyck
2018-04-03 17:32           ` Alexander Duyck
2018-04-03 17:32           ` Alexander Duyck
2018-04-03 18:27           ` [virtio-dev] " Michael S. Tsirkin
2018-04-03 18:27             ` Michael S. Tsirkin
2018-04-03 18:27             ` Michael S. Tsirkin
2018-04-03 19:06             ` Alexander Duyck
2018-04-03 19:06               ` Alexander Duyck
2018-04-03 19:06               ` Alexander Duyck
2018-04-20  0:40               ` Michael S. Tsirkin
2018-04-20  0:40                 ` Michael S. Tsirkin
2018-04-20  0:40                 ` Michael S. Tsirkin
2018-04-20  0:40                 ` Michael S. Tsirkin
2018-04-20 14:56                 ` [virtio-dev] " Alexander Duyck
2018-04-20 14:56                   ` Alexander Duyck
2018-04-20 14:56                   ` Alexander Duyck
2018-04-20 15:28                   ` Michael S. Tsirkin
2018-04-20 15:28                     ` Michael S. Tsirkin
2018-04-20 15:28                     ` Michael S. Tsirkin
2018-04-20 15:28                     ` Michael S. Tsirkin
2018-04-20 16:08                     ` [virtio-dev] " Alexander Duyck
2018-04-20 16:08                       ` Alexander Duyck
2018-04-20 16:08                       ` Alexander Duyck
2018-04-20 16:14                       ` Michael S. Tsirkin
2018-04-20 16:14                         ` Michael S. Tsirkin
2018-04-20 16:14                         ` Michael S. Tsirkin
2018-04-21  7:05                     ` Christoph Hellwig
2018-04-21  7:05                       ` Christoph Hellwig
2018-04-03 19:18             ` Rustad, Mark D
2018-04-03 19:18               ` Rustad, Mark D
2018-04-03 19:18               ` Rustad, Mark D
2018-03-28 21:31   ` Rustad, Mark D
2018-03-28 21:31     ` [virtio-dev] " Rustad, Mark D
2018-03-28 21:31     ` Rustad, Mark D
2018-03-28 21:31     ` Rustad, Mark D
2018-04-03 13:11   ` [virtio-dev] " Michael S. Tsirkin
2018-04-03 13:11     ` Michael S. Tsirkin
2018-04-03 13:11     ` Michael S. Tsirkin
2018-04-03 13:11     ` Michael S. Tsirkin
2018-03-15 18:43 ` [pci PATCH v7 3/5] ena: Migrate over to unmanaged SR-IOV support Alexander Duyck
2018-03-15 18:43   ` [virtio-dev] " Alexander Duyck
2018-03-15 18:43   ` Alexander Duyck
2018-03-15 18:43 ` [pci PATCH v7 4/5] nvme: " Alexander Duyck
2018-03-15 18:43   ` [virtio-dev] " Alexander Duyck
2018-03-15 18:43   ` Alexander Duyck
2018-03-15 18:44 ` [pci PATCH v7 5/5] pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs Alexander Duyck
2018-03-15 18:44   ` [virtio-dev] " Alexander Duyck
2018-03-15 18:44   ` Alexander Duyck
2018-03-16 21:42 ` [pci PATCH v7 0/5] Add support for unmanaged SR-IOV Don Dutile
2018-03-16 21:42   ` Don Dutile
2018-04-19 22:54 ` Alexander Duyck
2018-04-19 22:54   ` [virtio-dev] " Alexander Duyck
2018-04-19 22:54   ` Alexander Duyck
2018-04-20  0:46   ` Michael S. Tsirkin
2018-04-20  0:46     ` [virtio-dev] " Michael S. Tsirkin
2018-04-20  0:46     ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.