* [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 8:55 ` Tiwei Bie
0 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-05-30 8:55 UTC (permalink / raw)
To: mst, bhelgaas, virtualization, linux-kernel, virtio-dev, linux-pci
Cc: dan.daly, mark.d.rustad, alexander.h.duyck, cunming.liang, zhihong.wang
There is a new feature bit allocated in virtio spec to
support SR-IOV (Single Root I/O Virtualization):
https://github.com/oasis-tcs/virtio-spec/issues/11
This patch enables the support for this feature bit in
virtio driver.
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
This patch depends on below proposal:
https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
This patch depends on below patch:
https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
include/uapi/linux/virtio_config.h | 7 ++++++-
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 48d4d1cf1cb6..023da80a7a3e 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
put_device(dev);
}
+static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
+{
+ struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
+ struct virtio_device *vdev = &vp_dev->vdev;
+ int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
+
+ if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
+ return -EBUSY;
+
+ if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
+ return -EINVAL;
+
+ sriov_configure = pci_sriov_configure_simple;
+ if (sriov_configure == NULL)
+ return -ENOENT;
+
+ return sriov_configure(pci_dev, num_vfs);
+}
+
static struct pci_driver virtio_pci_driver = {
.name = "virtio-pci",
.id_table = virtio_pci_id_table,
@@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
#ifdef CONFIG_PM_SLEEP
.driver.pm = &virtio_pci_pm_ops,
#endif
+ .sriov_configure = virtio_pci_sriov_configure,
};
module_pci_driver(virtio_pci_driver);
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 2555d80f6eec..07571daccfec 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
return features;
}
+static void vp_transport_features(struct virtio_device *vdev, u64 features)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct pci_dev *pci_dev = vp_dev->pci_dev;
+
+ if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
+ pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
+ __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
+}
+
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ u64 features = vdev->features;
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
+ /* Give virtio_pci a chance to accept features. */
+ vp_transport_features(vdev, features);
+
if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
dev_err(&vdev->dev, "virtio: device uses modern interface "
"but does not have VIRTIO_F_VERSION_1\n");
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 308e2096291f..b7c1f4e7d59e 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -49,7 +49,7 @@
* transport being used (eg. virtio_ring), the rest are per-device feature
* bits. */
#define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END 34
+#define VIRTIO_TRANSPORT_F_END 38
#ifndef VIRTIO_CONFIG_NO_LEGACY
/* Do we get callbacks when the ring is completely used, even if we've
@@ -71,4 +71,9 @@
* this is for compatibility with legacy systems.
*/
#define VIRTIO_F_IOMMU_PLATFORM 33
+
+/*
+ * Does the device support Single Root I/O Virtualization?
+ */
+#define VIRTIO_F_SR_IOV 37
#endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
--
2.17.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [virtio-dev] [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 8:55 ` Tiwei Bie
0 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-05-30 8:55 UTC (permalink / raw)
To: mst, bhelgaas, virtualization, linux-kernel, virtio-dev, linux-pci
Cc: dan.daly, mark.d.rustad, alexander.h.duyck, cunming.liang, zhihong.wang
There is a new feature bit allocated in virtio spec to
support SR-IOV (Single Root I/O Virtualization):
https://github.com/oasis-tcs/virtio-spec/issues/11
This patch enables the support for this feature bit in
virtio driver.
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
This patch depends on below proposal:
https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
This patch depends on below patch:
https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
include/uapi/linux/virtio_config.h | 7 ++++++-
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 48d4d1cf1cb6..023da80a7a3e 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
put_device(dev);
}
+static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
+{
+ struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
+ struct virtio_device *vdev = &vp_dev->vdev;
+ int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
+
+ if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
+ return -EBUSY;
+
+ if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
+ return -EINVAL;
+
+ sriov_configure = pci_sriov_configure_simple;
+ if (sriov_configure == NULL)
+ return -ENOENT;
+
+ return sriov_configure(pci_dev, num_vfs);
+}
+
static struct pci_driver virtio_pci_driver = {
.name = "virtio-pci",
.id_table = virtio_pci_id_table,
@@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
#ifdef CONFIG_PM_SLEEP
.driver.pm = &virtio_pci_pm_ops,
#endif
+ .sriov_configure = virtio_pci_sriov_configure,
};
module_pci_driver(virtio_pci_driver);
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 2555d80f6eec..07571daccfec 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
return features;
}
+static void vp_transport_features(struct virtio_device *vdev, u64 features)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct pci_dev *pci_dev = vp_dev->pci_dev;
+
+ if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
+ pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
+ __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
+}
+
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ u64 features = vdev->features;
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
+ /* Give virtio_pci a chance to accept features. */
+ vp_transport_features(vdev, features);
+
if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
dev_err(&vdev->dev, "virtio: device uses modern interface "
"but does not have VIRTIO_F_VERSION_1\n");
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 308e2096291f..b7c1f4e7d59e 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -49,7 +49,7 @@
* transport being used (eg. virtio_ring), the rest are per-device feature
* bits. */
#define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END 34
+#define VIRTIO_TRANSPORT_F_END 38
#ifndef VIRTIO_CONFIG_NO_LEGACY
/* Do we get callbacks when the ring is completely used, even if we've
@@ -71,4 +71,9 @@
* this is for compatibility with legacy systems.
*/
#define VIRTIO_F_IOMMU_PLATFORM 33
+
+/*
+ * Does the device support Single Root I/O Virtualization?
+ */
+#define VIRTIO_F_SR_IOV 37
#endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
--
2.17.0
---------------------------------------------------------------------
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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 8:55 ` [virtio-dev] " Tiwei Bie
(?)
@ 2018-05-30 12:29 ` Stefan Hajnoczi
-1 siblings, 0 replies; 55+ messages in thread
From: Stefan Hajnoczi @ 2018-05-30 12:29 UTC (permalink / raw)
To: Tiwei Bie
Cc: mst, bhelgaas, virtualization, linux-kernel, virtio-dev,
linux-pci, dan.daly, mark.d.rustad, alexander.h.duyck,
cunming.liang, zhihong.wang
[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]
On Wed, May 30, 2018 at 04:55:21PM +0800, Tiwei Bie wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
Looks good and matches the virtio spec change, but I'm not familiar with
PCI SR-IOV details, so not a full Reviewed-by.
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 8:55 ` [virtio-dev] " Tiwei Bie
(?)
(?)
@ 2018-05-30 12:29 ` Stefan Hajnoczi
-1 siblings, 0 replies; 55+ messages in thread
From: Stefan Hajnoczi @ 2018-05-30 12:29 UTC (permalink / raw)
To: Tiwei Bie
Cc: alexander.h.duyck, virtio-dev, mst, linux-pci, linux-kernel,
virtualization, zhihong.wang, bhelgaas, mark.d.rustad
[-- Attachment #1.1: Type: text/plain, Size: 1073 bytes --]
On Wed, May 30, 2018 at 04:55:21PM +0800, Tiwei Bie wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
Looks good and matches the virtio spec change, but I'm not familiar with
PCI SR-IOV details, so not a full Reviewed-by.
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-30 8:55 ` [virtio-dev] " Tiwei Bie
@ 2018-05-30 12:41 ` Michael S. Tsirkin
-1 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 12:41 UTC (permalink / raw)
To: Tiwei Bie
Cc: bhelgaas, virtualization, linux-kernel, virtio-dev, linux-pci,
dan.daly, mark.d.rustad, alexander.h.duyck, cunming.liang,
zhihong.wang
On Wed, May 30, 2018 at 04:55:21PM +0800, Tiwei Bie wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-30 8:55 ` [virtio-dev] " Tiwei Bie
` (3 preceding siblings ...)
(?)
@ 2018-05-30 12:41 ` Michael S. Tsirkin
-1 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 12:41 UTC (permalink / raw)
To: Tiwei Bie
Cc: alexander.h.duyck, virtio-dev, linux-pci, linux-kernel,
virtualization, zhihong.wang, bhelgaas, mark.d.rustad
On Wed, May 30, 2018 at 04:55:21PM +0800, Tiwei Bie wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
^ permalink raw reply [flat|nested] 55+ messages in thread
* [virtio-dev] Re: [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 12:41 ` Michael S. Tsirkin
0 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 12:41 UTC (permalink / raw)
To: Tiwei Bie
Cc: bhelgaas, virtualization, linux-kernel, virtio-dev, linux-pci,
dan.daly, mark.d.rustad, alexander.h.duyck, cunming.liang,
zhihong.wang
On Wed, May 30, 2018 at 04:55:21PM +0800, Tiwei Bie wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
---------------------------------------------------------------------
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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 8:55 ` [virtio-dev] " Tiwei Bie
` (4 preceding siblings ...)
(?)
@ 2018-05-30 16:03 ` Rustad, Mark D
2018-05-30 16:09 ` Michael S. Tsirkin
2018-05-30 16:09 ` Michael S. Tsirkin
-1 siblings, 2 replies; 55+ messages in thread
From: Rustad, Mark D @ 2018-05-30 16:03 UTC (permalink / raw)
To: Bie, Tiwei
Cc: Michael S. Tsirkin, Bjorn Helgaas, virtualization, LKML,
virtio-dev, linux-pci, Daly, Dan, Duyck, Alexander H, Liang,
Cunming, Wang, Zhihong
[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]
On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
<snip>
> diff --git a/include/uapi/linux/virtio_config.h
> b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
There is a value in the comment directly before this that should
be updated as well to be consistent with the new value for
VIRTIO_TRANSPORT_F_END below.
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
<snip>
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 8:55 ` [virtio-dev] " Tiwei Bie
` (5 preceding siblings ...)
(?)
@ 2018-05-30 16:03 ` Rustad, Mark D
-1 siblings, 0 replies; 55+ messages in thread
From: Rustad, Mark D @ 2018-05-30 16:03 UTC (permalink / raw)
To: Bie, Tiwei
Cc: Duyck, Alexander H, virtio-dev, Michael S. Tsirkin, linux-pci,
LKML, virtualization, Wang, Zhihong, Bjorn Helgaas
[-- Attachment #1.1: Type: text/plain, Size: 1671 bytes --]
On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
<snip>
> diff --git a/include/uapi/linux/virtio_config.h
> b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
There is a value in the comment directly before this that should
be updated as well to be consistent with the new value for
VIRTIO_TRANSPORT_F_END below.
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
<snip>
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #1.2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:03 ` [virtio-dev] " Rustad, Mark D
@ 2018-05-30 16:09 ` Michael S. Tsirkin
2018-05-30 16:09 ` Michael S. Tsirkin
1 sibling, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 16:09 UTC (permalink / raw)
To: Rustad, Mark D
Cc: Bie, Tiwei, Bjorn Helgaas, virtualization, LKML, virtio-dev,
linux-pci, Daly, Dan, Duyck, Alexander H, Liang, Cunming, Wang,
Zhihong
On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
>
> > There is a new feature bit allocated in virtio spec to
> > support SR-IOV (Single Root I/O Virtualization):
> >
> > https://github.com/oasis-tcs/virtio-spec/issues/11
> >
> > This patch enables the support for this feature bit in
> > virtio driver.
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > ---
> > This patch depends on below proposal:
> > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> >
> > This patch depends on below patch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> >
> > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > include/uapi/linux/virtio_config.h | 7 ++++++-
> > 3 files changed, 40 insertions(+), 1 deletion(-)
> >
>
> <snip>
>
> > diff --git a/include/uapi/linux/virtio_config.h
> > b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..b7c1f4e7d59e 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -49,7 +49,7 @@
>
> There is a value in the comment directly before this that should
> be updated as well to be consistent with the new value for
> VIRTIO_TRANSPORT_F_END below.
It hasn't been updated to 34 yet.
I suggest a separate patch to replace the numbers with
VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
in the comment.
Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
virtio_pci etc." as well.
> > * transport being used (eg. virtio_ring), the rest are per-device feature
> > * bits. */
> > #define VIRTIO_TRANSPORT_F_START 28
> > -#define VIRTIO_TRANSPORT_F_END 34
> > +#define VIRTIO_TRANSPORT_F_END 38
> >
> > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > /* Do we get callbacks when the ring is completely used, even if we've
>
> <snip>
>
> --
> Mark Rustad, Networking Division, Intel Corporation
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:03 ` [virtio-dev] " Rustad, Mark D
2018-05-30 16:09 ` Michael S. Tsirkin
@ 2018-05-30 16:09 ` Michael S. Tsirkin
1 sibling, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 16:09 UTC (permalink / raw)
To: Rustad, Mark D
Cc: Duyck, Alexander H, virtio-dev, linux-pci, LKML, virtualization,
Wang, Zhihong, Bjorn Helgaas
On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
>
> > There is a new feature bit allocated in virtio spec to
> > support SR-IOV (Single Root I/O Virtualization):
> >
> > https://github.com/oasis-tcs/virtio-spec/issues/11
> >
> > This patch enables the support for this feature bit in
> > virtio driver.
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > ---
> > This patch depends on below proposal:
> > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> >
> > This patch depends on below patch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> >
> > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > include/uapi/linux/virtio_config.h | 7 ++++++-
> > 3 files changed, 40 insertions(+), 1 deletion(-)
> >
>
> <snip>
>
> > diff --git a/include/uapi/linux/virtio_config.h
> > b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..b7c1f4e7d59e 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -49,7 +49,7 @@
>
> There is a value in the comment directly before this that should
> be updated as well to be consistent with the new value for
> VIRTIO_TRANSPORT_F_END below.
It hasn't been updated to 34 yet.
I suggest a separate patch to replace the numbers with
VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
in the comment.
Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
virtio_pci etc." as well.
> > * transport being used (eg. virtio_ring), the rest are per-device feature
> > * bits. */
> > #define VIRTIO_TRANSPORT_F_START 28
> > -#define VIRTIO_TRANSPORT_F_END 34
> > +#define VIRTIO_TRANSPORT_F_END 38
> >
> > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > /* Do we get callbacks when the ring is completely used, even if we've
>
> <snip>
>
> --
> Mark Rustad, Networking Division, Intel Corporation
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 16:09 ` Michael S. Tsirkin
0 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 16:09 UTC (permalink / raw)
To: Rustad, Mark D
Cc: Bie, Tiwei, Bjorn Helgaas, virtualization, LKML, virtio-dev,
linux-pci, Daly, Dan, Duyck, Alexander H, Liang, Cunming, Wang,
Zhihong
On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
>
> > There is a new feature bit allocated in virtio spec to
> > support SR-IOV (Single Root I/O Virtualization):
> >
> > https://github.com/oasis-tcs/virtio-spec/issues/11
> >
> > This patch enables the support for this feature bit in
> > virtio driver.
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > ---
> > This patch depends on below proposal:
> > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> >
> > This patch depends on below patch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> >
> > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > include/uapi/linux/virtio_config.h | 7 ++++++-
> > 3 files changed, 40 insertions(+), 1 deletion(-)
> >
>
> <snip>
>
> > diff --git a/include/uapi/linux/virtio_config.h
> > b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..b7c1f4e7d59e 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -49,7 +49,7 @@
>
> There is a value in the comment directly before this that should
> be updated as well to be consistent with the new value for
> VIRTIO_TRANSPORT_F_END below.
It hasn't been updated to 34 yet.
I suggest a separate patch to replace the numbers with
VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
in the comment.
Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
virtio_pci etc." as well.
> > * transport being used (eg. virtio_ring), the rest are per-device feature
> > * bits. */
> > #define VIRTIO_TRANSPORT_F_START 28
> > -#define VIRTIO_TRANSPORT_F_END 34
> > +#define VIRTIO_TRANSPORT_F_END 38
> >
> > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > /* Do we get callbacks when the ring is completely used, even if we've
>
> <snip>
>
> --
> Mark Rustad, Networking Division, Intel Corporation
---------------------------------------------------------------------
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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 8:55 ` [virtio-dev] " Tiwei Bie
@ 2018-05-30 16:10 ` Alexander Duyck
-1 siblings, 0 replies; 55+ messages in thread
From: Alexander Duyck @ 2018-05-30 16:10 UTC (permalink / raw)
To: Tiwei Bie
Cc: Michael S. Tsirkin, Bjorn Helgaas, virtualization, LKML,
virtio-dev, linux-pci, Daly, Dan, Rustad, Mark D, Duyck,
Alexander H, cunming.liang, zhihong.wang
On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
So from a quick glance it looks like we are leaving SR-IOV enabled if
the driver is removed. Do we want to have that behavior or should we
be adding the code to disable SR-IOV and free the VFs on driver
removal?
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
>
>
> ---------------------------------------------------------------------
> 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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 8:55 ` [virtio-dev] " Tiwei Bie
` (7 preceding siblings ...)
(?)
@ 2018-05-30 16:10 ` Alexander Duyck
-1 siblings, 0 replies; 55+ messages in thread
From: Alexander Duyck @ 2018-05-30 16:10 UTC (permalink / raw)
To: Tiwei Bie
Cc: Duyck, Alexander H, virtio-dev, Michael S. Tsirkin, linux-pci,
LKML, virtualization, zhihong.wang, Bjorn Helgaas, Rustad,
Mark D
On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
So from a quick glance it looks like we are leaving SR-IOV enabled if
the driver is removed. Do we want to have that behavior or should we
be adding the code to disable SR-IOV and free the VFs on driver
removal?
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
>
>
> ---------------------------------------------------------------------
> 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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 16:10 ` Alexander Duyck
0 siblings, 0 replies; 55+ messages in thread
From: Alexander Duyck @ 2018-05-30 16:10 UTC (permalink / raw)
To: Tiwei Bie
Cc: Michael S. Tsirkin, Bjorn Helgaas, virtualization, LKML,
virtio-dev, linux-pci, Daly, Dan, Rustad, Mark D, Duyck,
Alexander H, cunming.liang, zhihong.wang
On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
So from a quick glance it looks like we are leaving SR-IOV enabled if
the driver is removed. Do we want to have that behavior or should we
be adding the code to disable SR-IOV and free the VFs on driver
removal?
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
>
>
> ---------------------------------------------------------------------
> 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] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-30 8:55 ` [virtio-dev] " Tiwei Bie
@ 2018-05-30 16:22 ` Michael S. Tsirkin
-1 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 16:22 UTC (permalink / raw)
To: Tiwei Bie
Cc: bhelgaas, virtualization, linux-kernel, virtio-dev, linux-pci,
dan.daly, mark.d.rustad, alexander.h.duyck, cunming.liang,
zhihong.wang
On Wed, May 30, 2018 at 04:55:21PM +0800, Tiwei Bie wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
BTW what is all this trickery in aid of?
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-30 8:55 ` [virtio-dev] " Tiwei Bie
` (9 preceding siblings ...)
(?)
@ 2018-05-30 16:22 ` Michael S. Tsirkin
-1 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 16:22 UTC (permalink / raw)
To: Tiwei Bie
Cc: alexander.h.duyck, virtio-dev, linux-pci, linux-kernel,
virtualization, zhihong.wang, bhelgaas, mark.d.rustad
On Wed, May 30, 2018 at 04:55:21PM +0800, Tiwei Bie wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
BTW what is all this trickery in aid of?
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
^ permalink raw reply [flat|nested] 55+ messages in thread
* [virtio-dev] Re: [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 16:22 ` Michael S. Tsirkin
0 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 16:22 UTC (permalink / raw)
To: Tiwei Bie
Cc: bhelgaas, virtualization, linux-kernel, virtio-dev, linux-pci,
dan.daly, mark.d.rustad, alexander.h.duyck, cunming.liang,
zhihong.wang
On Wed, May 30, 2018 at 04:55:21PM +0800, Tiwei Bie wrote:
> There is a new feature bit allocated in virtio spec to
> support SR-IOV (Single Root I/O Virtualization):
>
> https://github.com/oasis-tcs/virtio-spec/issues/11
>
> This patch enables the support for this feature bit in
> virtio driver.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
> This patch depends on below proposal:
> https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
>
> This patch depends on below patch:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
>
> drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> include/uapi/linux/virtio_config.h | 7 ++++++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 48d4d1cf1cb6..023da80a7a3e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> put_device(dev);
> }
>
> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> +{
> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> + struct virtio_device *vdev = &vp_dev->vdev;
> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> +
> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> + return -EBUSY;
> +
> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> + return -EINVAL;
> +
> + sriov_configure = pci_sriov_configure_simple;
> + if (sriov_configure == NULL)
> + return -ENOENT;
BTW what is all this trickery in aid of?
> +
> + return sriov_configure(pci_dev, num_vfs);
> +}
> +
> static struct pci_driver virtio_pci_driver = {
> .name = "virtio-pci",
> .id_table = virtio_pci_id_table,
> @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> #ifdef CONFIG_PM_SLEEP
> .driver.pm = &virtio_pci_pm_ops,
> #endif
> + .sriov_configure = virtio_pci_sriov_configure,
> };
>
> module_pci_driver(virtio_pci_driver);
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 2555d80f6eec..07571daccfec 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> return features;
> }
>
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
> /* virtio config->finalize_features() implementation */
> static int vp_finalize_features(struct virtio_device *vdev)
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + u64 features = vdev->features;
>
> /* Give virtio_ring a chance to accept features. */
> vring_transport_features(vdev);
>
> + /* Give virtio_pci a chance to accept features. */
> + vp_transport_features(vdev, features);
> +
> if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> dev_err(&vdev->dev, "virtio: device uses modern interface "
> "but does not have VIRTIO_F_VERSION_1\n");
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..b7c1f4e7d59e 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
> * transport being used (eg. virtio_ring), the rest are per-device feature
> * bits. */
> #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END 34
> +#define VIRTIO_TRANSPORT_F_END 38
>
> #ifndef VIRTIO_CONFIG_NO_LEGACY
> /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,9 @@
> * this is for compatibility with legacy systems.
> */
> #define VIRTIO_F_IOMMU_PLATFORM 33
> +
> +/*
> + * Does the device support Single Root I/O Virtualization?
> + */
> +#define VIRTIO_F_SR_IOV 37
> #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> --
> 2.17.0
---------------------------------------------------------------------
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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:10 ` Alexander Duyck
@ 2018-05-30 16:22 ` Michael S. Tsirkin
-1 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 16:22 UTC (permalink / raw)
To: Alexander Duyck
Cc: Tiwei Bie, Bjorn Helgaas, virtualization, LKML, virtio-dev,
linux-pci, Daly, Dan, Rustad, Mark D, Duyck, Alexander H,
cunming.liang, zhihong.wang
On Wed, May 30, 2018 at 09:10:57AM -0700, Alexander Duyck wrote:
> On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > There is a new feature bit allocated in virtio spec to
> > support SR-IOV (Single Root I/O Virtualization):
> >
> > https://github.com/oasis-tcs/virtio-spec/issues/11
> >
> > This patch enables the support for this feature bit in
> > virtio driver.
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>
> So from a quick glance it looks like we are leaving SR-IOV enabled if
> the driver is removed. Do we want to have that behavior or should we
> be adding the code to disable SR-IOV and free the VFs on driver
> removal?
Could pci core handle it for us somehow?
> > ---
> > This patch depends on below proposal:
> > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> >
> > This patch depends on below patch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> >
> > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > include/uapi/linux/virtio_config.h | 7 ++++++-
> > 3 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > index 48d4d1cf1cb6..023da80a7a3e 100644
> > --- a/drivers/virtio/virtio_pci_common.c
> > +++ b/drivers/virtio/virtio_pci_common.c
> > @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> > put_device(dev);
> > }
> >
> > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> > +{
> > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> > + struct virtio_device *vdev = &vp_dev->vdev;
> > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> > +
> > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> > + return -EBUSY;
> > +
> > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> > + return -EINVAL;
> > +
> > + sriov_configure = pci_sriov_configure_simple;
> > + if (sriov_configure == NULL)
> > + return -ENOENT;
> > +
> > + return sriov_configure(pci_dev, num_vfs);
> > +}
> > +
> > static struct pci_driver virtio_pci_driver = {
> > .name = "virtio-pci",
> > .id_table = virtio_pci_id_table,
> > @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> > #ifdef CONFIG_PM_SLEEP
> > .driver.pm = &virtio_pci_pm_ops,
> > #endif
> > + .sriov_configure = virtio_pci_sriov_configure,
> > };
> >
> > module_pci_driver(virtio_pci_driver);
> > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> > index 2555d80f6eec..07571daccfec 100644
> > --- a/drivers/virtio/virtio_pci_modern.c
> > +++ b/drivers/virtio/virtio_pci_modern.c
> > @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> > return features;
> > }
> >
> > +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> > +{
> > + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + struct pci_dev *pci_dev = vp_dev->pci_dev;
> > +
> > + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> > + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> > + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> > +}
> > +
> > /* virtio config->finalize_features() implementation */
> > static int vp_finalize_features(struct virtio_device *vdev)
> > {
> > struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + u64 features = vdev->features;
> >
> > /* Give virtio_ring a chance to accept features. */
> > vring_transport_features(vdev);
> >
> > + /* Give virtio_pci a chance to accept features. */
> > + vp_transport_features(vdev, features);
> > +
> > if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> > dev_err(&vdev->dev, "virtio: device uses modern interface "
> > "but does not have VIRTIO_F_VERSION_1\n");
> > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..b7c1f4e7d59e 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -49,7 +49,7 @@
> > * transport being used (eg. virtio_ring), the rest are per-device feature
> > * bits. */
> > #define VIRTIO_TRANSPORT_F_START 28
> > -#define VIRTIO_TRANSPORT_F_END 34
> > +#define VIRTIO_TRANSPORT_F_END 38
> >
> > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > /* Do we get callbacks when the ring is completely used, even if we've
> > @@ -71,4 +71,9 @@
> > * this is for compatibility with legacy systems.
> > */
> > #define VIRTIO_F_IOMMU_PLATFORM 33
> > +
> > +/*
> > + * Does the device support Single Root I/O Virtualization?
> > + */
> > +#define VIRTIO_F_SR_IOV 37
> > #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> > --
> > 2.17.0
> >
> >
> > ---------------------------------------------------------------------
> > 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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:10 ` Alexander Duyck
(?)
@ 2018-05-30 16:22 ` Michael S. Tsirkin
-1 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 16:22 UTC (permalink / raw)
To: Alexander Duyck
Cc: Duyck, Alexander H, virtio-dev, linux-pci, LKML, virtualization,
zhihong.wang, Bjorn Helgaas, Rustad, Mark D
On Wed, May 30, 2018 at 09:10:57AM -0700, Alexander Duyck wrote:
> On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > There is a new feature bit allocated in virtio spec to
> > support SR-IOV (Single Root I/O Virtualization):
> >
> > https://github.com/oasis-tcs/virtio-spec/issues/11
> >
> > This patch enables the support for this feature bit in
> > virtio driver.
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>
> So from a quick glance it looks like we are leaving SR-IOV enabled if
> the driver is removed. Do we want to have that behavior or should we
> be adding the code to disable SR-IOV and free the VFs on driver
> removal?
Could pci core handle it for us somehow?
> > ---
> > This patch depends on below proposal:
> > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> >
> > This patch depends on below patch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> >
> > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > include/uapi/linux/virtio_config.h | 7 ++++++-
> > 3 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > index 48d4d1cf1cb6..023da80a7a3e 100644
> > --- a/drivers/virtio/virtio_pci_common.c
> > +++ b/drivers/virtio/virtio_pci_common.c
> > @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> > put_device(dev);
> > }
> >
> > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> > +{
> > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> > + struct virtio_device *vdev = &vp_dev->vdev;
> > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> > +
> > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> > + return -EBUSY;
> > +
> > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> > + return -EINVAL;
> > +
> > + sriov_configure = pci_sriov_configure_simple;
> > + if (sriov_configure == NULL)
> > + return -ENOENT;
> > +
> > + return sriov_configure(pci_dev, num_vfs);
> > +}
> > +
> > static struct pci_driver virtio_pci_driver = {
> > .name = "virtio-pci",
> > .id_table = virtio_pci_id_table,
> > @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> > #ifdef CONFIG_PM_SLEEP
> > .driver.pm = &virtio_pci_pm_ops,
> > #endif
> > + .sriov_configure = virtio_pci_sriov_configure,
> > };
> >
> > module_pci_driver(virtio_pci_driver);
> > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> > index 2555d80f6eec..07571daccfec 100644
> > --- a/drivers/virtio/virtio_pci_modern.c
> > +++ b/drivers/virtio/virtio_pci_modern.c
> > @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> > return features;
> > }
> >
> > +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> > +{
> > + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + struct pci_dev *pci_dev = vp_dev->pci_dev;
> > +
> > + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> > + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> > + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> > +}
> > +
> > /* virtio config->finalize_features() implementation */
> > static int vp_finalize_features(struct virtio_device *vdev)
> > {
> > struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + u64 features = vdev->features;
> >
> > /* Give virtio_ring a chance to accept features. */
> > vring_transport_features(vdev);
> >
> > + /* Give virtio_pci a chance to accept features. */
> > + vp_transport_features(vdev, features);
> > +
> > if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> > dev_err(&vdev->dev, "virtio: device uses modern interface "
> > "but does not have VIRTIO_F_VERSION_1\n");
> > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..b7c1f4e7d59e 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -49,7 +49,7 @@
> > * transport being used (eg. virtio_ring), the rest are per-device feature
> > * bits. */
> > #define VIRTIO_TRANSPORT_F_START 28
> > -#define VIRTIO_TRANSPORT_F_END 34
> > +#define VIRTIO_TRANSPORT_F_END 38
> >
> > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > /* Do we get callbacks when the ring is completely used, even if we've
> > @@ -71,4 +71,9 @@
> > * this is for compatibility with legacy systems.
> > */
> > #define VIRTIO_F_IOMMU_PLATFORM 33
> > +
> > +/*
> > + * Does the device support Single Root I/O Virtualization?
> > + */
> > +#define VIRTIO_F_SR_IOV 37
> > #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> > --
> > 2.17.0
> >
> >
> > ---------------------------------------------------------------------
> > 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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 16:22 ` Michael S. Tsirkin
0 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 16:22 UTC (permalink / raw)
To: Alexander Duyck
Cc: Tiwei Bie, Bjorn Helgaas, virtualization, LKML, virtio-dev,
linux-pci, Daly, Dan, Rustad, Mark D, Duyck, Alexander H,
cunming.liang, zhihong.wang
On Wed, May 30, 2018 at 09:10:57AM -0700, Alexander Duyck wrote:
> On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > There is a new feature bit allocated in virtio spec to
> > support SR-IOV (Single Root I/O Virtualization):
> >
> > https://github.com/oasis-tcs/virtio-spec/issues/11
> >
> > This patch enables the support for this feature bit in
> > virtio driver.
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>
> So from a quick glance it looks like we are leaving SR-IOV enabled if
> the driver is removed. Do we want to have that behavior or should we
> be adding the code to disable SR-IOV and free the VFs on driver
> removal?
Could pci core handle it for us somehow?
> > ---
> > This patch depends on below proposal:
> > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> >
> > This patch depends on below patch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> >
> > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > include/uapi/linux/virtio_config.h | 7 ++++++-
> > 3 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> > index 48d4d1cf1cb6..023da80a7a3e 100644
> > --- a/drivers/virtio/virtio_pci_common.c
> > +++ b/drivers/virtio/virtio_pci_common.c
> > @@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> > put_device(dev);
> > }
> >
> > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
> > +{
> > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> > + struct virtio_device *vdev = &vp_dev->vdev;
> > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> > +
> > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> > + return -EBUSY;
> > +
> > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> > + return -EINVAL;
> > +
> > + sriov_configure = pci_sriov_configure_simple;
> > + if (sriov_configure == NULL)
> > + return -ENOENT;
> > +
> > + return sriov_configure(pci_dev, num_vfs);
> > +}
> > +
> > static struct pci_driver virtio_pci_driver = {
> > .name = "virtio-pci",
> > .id_table = virtio_pci_id_table,
> > @@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
> > #ifdef CONFIG_PM_SLEEP
> > .driver.pm = &virtio_pci_pm_ops,
> > #endif
> > + .sriov_configure = virtio_pci_sriov_configure,
> > };
> >
> > module_pci_driver(virtio_pci_driver);
> > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> > index 2555d80f6eec..07571daccfec 100644
> > --- a/drivers/virtio/virtio_pci_modern.c
> > +++ b/drivers/virtio/virtio_pci_modern.c
> > @@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
> > return features;
> > }
> >
> > +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> > +{
> > + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + struct pci_dev *pci_dev = vp_dev->pci_dev;
> > +
> > + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> > + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> > + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> > +}
> > +
> > /* virtio config->finalize_features() implementation */
> > static int vp_finalize_features(struct virtio_device *vdev)
> > {
> > struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> > + u64 features = vdev->features;
> >
> > /* Give virtio_ring a chance to accept features. */
> > vring_transport_features(vdev);
> >
> > + /* Give virtio_pci a chance to accept features. */
> > + vp_transport_features(vdev, features);
> > +
> > if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
> > dev_err(&vdev->dev, "virtio: device uses modern interface "
> > "but does not have VIRTIO_F_VERSION_1\n");
> > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..b7c1f4e7d59e 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -49,7 +49,7 @@
> > * transport being used (eg. virtio_ring), the rest are per-device feature
> > * bits. */
> > #define VIRTIO_TRANSPORT_F_START 28
> > -#define VIRTIO_TRANSPORT_F_END 34
> > +#define VIRTIO_TRANSPORT_F_END 38
> >
> > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > /* Do we get callbacks when the ring is completely used, even if we've
> > @@ -71,4 +71,9 @@
> > * this is for compatibility with legacy systems.
> > */
> > #define VIRTIO_F_IOMMU_PLATFORM 33
> > +
> > +/*
> > + * Does the device support Single Root I/O Virtualization?
> > + */
> > +#define VIRTIO_F_SR_IOV 37
> > #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> > --
> > 2.17.0
> >
> >
> > ---------------------------------------------------------------------
> > 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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:22 ` Michael S. Tsirkin
@ 2018-05-30 16:26 ` Duyck, Alexander H
-1 siblings, 0 replies; 55+ messages in thread
From: Duyck, Alexander H @ 2018-05-30 16:26 UTC (permalink / raw)
To: mst, alexander.duyck
Cc: linux-kernel, virtualization, virtio-dev, Rustad, Mark D, Daly,
Dan, Bie, Tiwei, linux-pci, bhelgaas, Liang, Cunming, Wang,
Zhihong
On Wed, 2018-05-30 at 19:22 +0300, Michael S. Tsirkin wrote:
> On Wed, May 30, 2018 at 09:10:57AM -0700, Alexander Duyck wrote:
> > On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > > There is a new feature bit allocated in virtio spec to
> > > support SR-IOV (Single Root I/O Virtualization):
> > >
> > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > >
> > > This patch enables the support for this feature bit in
> > > virtio driver.
> > >
> > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> >
> > So from a quick glance it looks like we are leaving SR-IOV enabled if
> > the driver is removed. Do we want to have that behavior or should we
> > be adding the code to disable SR-IOV and free the VFs on driver
> > removal?
>
> Could pci core handle it for us somehow?
Maybe, but it would require changes to the pci core to do it.
The problem is some drivers want to leave the VFs there since the PF
doesn't really do anything, or they have the option of essentially
putting the VFs into a standby state when the PF is gone.
My main concern is do we care if VFs are allocated and then somebody
removes the driver and binds a different driver to the interface? If
not then this code and be left as is, but I just wanted to be certain
since I know this isn't just enabling SR-IOV we are having to do a
number of other checks against the virtio device.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 16:26 ` Duyck, Alexander H
0 siblings, 0 replies; 55+ messages in thread
From: Duyck, Alexander H @ 2018-05-30 16:26 UTC (permalink / raw)
To: mst, alexander.duyck
Cc: linux-kernel, virtualization, virtio-dev, Rustad, Mark D, Daly,
Dan, Bie, Tiwei, linux-pci, bhelgaas, Liang, Cunming, Wang,
Zhihong
T24gV2VkLCAyMDE4LTA1LTMwIGF0IDE5OjIyICswMzAwLCBNaWNoYWVsIFMuIFRzaXJraW4gd3Jv
dGU6DQo+IE9uIFdlZCwgTWF5IDMwLCAyMDE4IGF0IDA5OjEwOjU3QU0gLTA3MDAsIEFsZXhhbmRl
ciBEdXljayB3cm90ZToNCj4gPiBPbiBXZWQsIE1heSAzMCwgMjAxOCBhdCAxOjU1IEFNLCBUaXdl
aSBCaWUgPHRpd2VpLmJpZUBpbnRlbC5jb20+IHdyb3RlOg0KPiA+ID4gVGhlcmUgaXMgYSBuZXcg
ZmVhdHVyZSBiaXQgYWxsb2NhdGVkIGluIHZpcnRpbyBzcGVjIHRvDQo+ID4gPiBzdXBwb3J0IFNS
LUlPViAoU2luZ2xlIFJvb3QgSS9PIFZpcnR1YWxpemF0aW9uKToNCj4gPiA+IA0KPiA+ID4gaHR0
cHM6Ly9naXRodWIuY29tL29hc2lzLXRjcy92aXJ0aW8tc3BlYy9pc3N1ZXMvMTENCj4gPiA+IA0K
PiA+ID4gVGhpcyBwYXRjaCBlbmFibGVzIHRoZSBzdXBwb3J0IGZvciB0aGlzIGZlYXR1cmUgYml0
IGluDQo+ID4gPiB2aXJ0aW8gZHJpdmVyLg0KPiA+ID4gDQo+ID4gPiBTaWduZWQtb2ZmLWJ5OiBU
aXdlaSBCaWUgPHRpd2VpLmJpZUBpbnRlbC5jb20+DQo+ID4gDQo+ID4gU28gZnJvbSBhIHF1aWNr
IGdsYW5jZSBpdCBsb29rcyBsaWtlIHdlIGFyZSBsZWF2aW5nIFNSLUlPViBlbmFibGVkIGlmDQo+
ID4gdGhlIGRyaXZlciBpcyByZW1vdmVkLiBEbyB3ZSB3YW50IHRvIGhhdmUgdGhhdCBiZWhhdmlv
ciBvciBzaG91bGQgd2UNCj4gPiBiZSBhZGRpbmcgdGhlIGNvZGUgdG8gZGlzYWJsZSBTUi1JT1Yg
YW5kIGZyZWUgdGhlIFZGcyBvbiBkcml2ZXINCj4gPiByZW1vdmFsPw0KPiANCj4gQ291bGQgcGNp
IGNvcmUgaGFuZGxlIGl0IGZvciB1cyBzb21laG93Pw0KDQpNYXliZSwgYnV0IGl0IHdvdWxkIHJl
cXVpcmUgY2hhbmdlcyB0byB0aGUgcGNpIGNvcmUgdG8gZG8gaXQuDQoNClRoZSBwcm9ibGVtIGlz
IHNvbWUgZHJpdmVycyB3YW50IHRvIGxlYXZlIHRoZSBWRnMgdGhlcmUgc2luY2UgdGhlIFBGDQpk
b2Vzbid0IHJlYWxseSBkbyBhbnl0aGluZywgb3IgdGhleSBoYXZlIHRoZSBvcHRpb24gb2YgZXNz
ZW50aWFsbHkNCnB1dHRpbmcgdGhlIFZGcyBpbnRvIGEgc3RhbmRieSBzdGF0ZSB3aGVuIHRoZSBQ
RiBpcyBnb25lLg0KDQpNeSBtYWluIGNvbmNlcm4gaXMgZG8gd2UgY2FyZSBpZiBWRnMgYXJlIGFs
bG9jYXRlZCBhbmQgdGhlbiBzb21lYm9keQ0KcmVtb3ZlcyB0aGUgZHJpdmVyIGFuZCBiaW5kcyBh
IGRpZmZlcmVudCBkcml2ZXIgdG8gdGhlIGludGVyZmFjZT8gSWYNCm5vdCB0aGVuIHRoaXMgY29k
ZSBhbmQgYmUgbGVmdCBhcyBpcywgYnV0IEkganVzdCB3YW50ZWQgdG8gYmUgY2VydGFpbg0Kc2lu
Y2UgSSBrbm93IHRoaXMgaXNuJ3QganVzdCBlbmFibGluZyBTUi1JT1Ygd2UgYXJlIGhhdmluZyB0
byBkbyBhDQpudW1iZXIgb2Ygb3RoZXIgY2hlY2tzIGFnYWluc3QgdGhlIHZpcnRpbyBkZXZpY2Uu
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:22 ` Michael S. Tsirkin
(?)
@ 2018-05-30 16:26 ` Duyck, Alexander H
-1 siblings, 0 replies; 55+ messages in thread
From: Duyck, Alexander H @ 2018-05-30 16:26 UTC (permalink / raw)
To: mst, alexander.duyck
Cc: virtio-dev, linux-pci, linux-kernel, virtualization, Wang,
Zhihong, bhelgaas, Rustad, Mark D
On Wed, 2018-05-30 at 19:22 +0300, Michael S. Tsirkin wrote:
> On Wed, May 30, 2018 at 09:10:57AM -0700, Alexander Duyck wrote:
> > On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > > There is a new feature bit allocated in virtio spec to
> > > support SR-IOV (Single Root I/O Virtualization):
> > >
> > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > >
> > > This patch enables the support for this feature bit in
> > > virtio driver.
> > >
> > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> >
> > So from a quick glance it looks like we are leaving SR-IOV enabled if
> > the driver is removed. Do we want to have that behavior or should we
> > be adding the code to disable SR-IOV and free the VFs on driver
> > removal?
>
> Could pci core handle it for us somehow?
Maybe, but it would require changes to the pci core to do it.
The problem is some drivers want to leave the VFs there since the PF
doesn't really do anything, or they have the option of essentially
putting the VFs into a standby state when the PF is gone.
My main concern is do we care if VFs are allocated and then somebody
removes the driver and binds a different driver to the interface? If
not then this code and be left as is, but I just wanted to be certain
since I know this isn't just enabling SR-IOV we are having to do a
number of other checks against the virtio device.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:22 ` [virtio-dev] " Michael S. Tsirkin
(?)
@ 2018-05-30 16:44 ` Rustad, Mark D
-1 siblings, 0 replies; 55+ messages in thread
From: Rustad, Mark D @ 2018-05-30 16:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Bie, Tiwei, Bjorn Helgaas, virtualization, linux-kernel,
virtio-dev, linux-pci, Daly, Dan, Duyck, Alexander H, Liang,
Cunming, Wang, Zhihong
[-- Attachment #1: Type: text/plain, Size: 996 bytes --]
On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
>> num_vfs)
>> +{
>> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>> + struct virtio_device *vdev = &vp_dev->vdev;
>> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
>> +
>> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
>> + return -EBUSY;
>> +
>> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
>> + return -EINVAL;
>> +
>> + sriov_configure = pci_sriov_configure_simple;
>> + if (sriov_configure == NULL)
>> + return -ENOENT;
>
> BTW what is all this trickery in aid of?
When SR-IOV support is not compiled into the kernel,
pci_sriov_configure_simple is #defined as NULL. This allows it to compile
in that case, even though there is utterly no way for it to be called in
that case. It is an alternative to #ifs in the code.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 16:44 ` Rustad, Mark D
0 siblings, 0 replies; 55+ messages in thread
From: Rustad, Mark D @ 2018-05-30 16:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Duyck, Alexander H, virtio-dev, linux-pci, linux-kernel,
virtualization, Wang, Zhihong, Bjorn Helgaas
[-- Attachment #1.1: Type: text/plain, Size: 996 bytes --]
On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
>> num_vfs)
>> +{
>> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>> + struct virtio_device *vdev = &vp_dev->vdev;
>> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
>> +
>> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
>> + return -EBUSY;
>> +
>> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
>> + return -EINVAL;
>> +
>> + sriov_configure = pci_sriov_configure_simple;
>> + if (sriov_configure == NULL)
>> + return -ENOENT;
>
> BTW what is all this trickery in aid of?
When SR-IOV support is not compiled into the kernel,
pci_sriov_configure_simple is #defined as NULL. This allows it to compile
in that case, even though there is utterly no way for it to be called in
that case. It is an alternative to #ifs in the code.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #1.2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 55+ messages in thread
* [virtio-dev] Re: [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 16:44 ` Rustad, Mark D
0 siblings, 0 replies; 55+ messages in thread
From: Rustad, Mark D @ 2018-05-30 16:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Bie, Tiwei, Bjorn Helgaas, virtualization, linux-kernel,
virtio-dev, linux-pci, Daly, Dan, Duyck, Alexander H, Liang,
Cunming, Wang, Zhihong
[-- Attachment #1: Type: text/plain, Size: 996 bytes --]
On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
>> num_vfs)
>> +{
>> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>> + struct virtio_device *vdev = &vp_dev->vdev;
>> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
>> +
>> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
>> + return -EBUSY;
>> +
>> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
>> + return -EINVAL;
>> +
>> + sriov_configure = pci_sriov_configure_simple;
>> + if (sriov_configure == NULL)
>> + return -ENOENT;
>
> BTW what is all this trickery in aid of?
When SR-IOV support is not compiled into the kernel,
pci_sriov_configure_simple is #defined as NULL. This allows it to compile
in that case, even though there is utterly no way for it to be called in
that case. It is an alternative to #ifs in the code.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:44 ` Rustad, Mark D
@ 2018-05-30 16:54 ` Duyck, Alexander H
-1 siblings, 0 replies; 55+ messages in thread
From: Duyck, Alexander H @ 2018-05-30 16:54 UTC (permalink / raw)
To: Rustad, Mark D, mst
Cc: linux-kernel, virtualization, virtio-dev, Daly, Dan, Bie, Tiwei,
linux-pci, bhelgaas, Liang, Cunming, Wang, Zhihong
On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
> On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
> > > num_vfs)
> > > +{
> > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> > > + struct virtio_device *vdev = &vp_dev->vdev;
> > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> > > +
> > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> > > + return -EBUSY;
> > > +
> > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> > > + return -EINVAL;
> > > +
> > > + sriov_configure = pci_sriov_configure_simple;
> > > + if (sriov_configure == NULL)
> > > + return -ENOENT;
> >
> > BTW what is all this trickery in aid of?
>
> When SR-IOV support is not compiled into the kernel,
> pci_sriov_configure_simple is #defined as NULL. This allows it to compile
> in that case, even though there is utterly no way for it to be called in
> that case. It is an alternative to #ifs in the code.
Why even have the call though? I would wrap all of this in an #ifdef
and strip it out since you couldn't support SR-IOV if it isn't present
in the kernel anyway.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 16:54 ` Duyck, Alexander H
0 siblings, 0 replies; 55+ messages in thread
From: Duyck, Alexander H @ 2018-05-30 16:54 UTC (permalink / raw)
To: Rustad, Mark D, mst
Cc: linux-kernel, virtualization, virtio-dev, Daly, Dan, Bie, Tiwei,
linux-pci, bhelgaas, Liang, Cunming, Wang, Zhihong
T24gV2VkLCAyMDE4LTA1LTMwIGF0IDA5OjQ0IC0wNzAwLCBSdXN0YWQsIE1hcmsgRCB3cm90ZToN
Cj4gT24gTWF5IDMwLCAyMDE4LCBhdCA5OjIyIEFNLCBNaWNoYWVsIFMuIFRzaXJraW4gPG1zdEBy
ZWRoYXQuY29tPiB3cm90ZToNCj4gDQo+ID4gPiArc3RhdGljIGludCB2aXJ0aW9fcGNpX3NyaW92
X2NvbmZpZ3VyZShzdHJ1Y3QgcGNpX2RldiAqcGNpX2RldiwgaW50ICANCj4gPiA+IG51bV92ZnMp
DQo+ID4gPiArew0KPiA+ID4gKwlzdHJ1Y3QgdmlydGlvX3BjaV9kZXZpY2UgKnZwX2RldiA9IHBj
aV9nZXRfZHJ2ZGF0YShwY2lfZGV2KTsNCj4gPiA+ICsJc3RydWN0IHZpcnRpb19kZXZpY2UgKnZk
ZXYgPSAmdnBfZGV2LT52ZGV2Ow0KPiA+ID4gKwlpbnQgKCpzcmlvdl9jb25maWd1cmUpKHN0cnVj
dCBwY2lfZGV2ICpwY2lfZGV2LCBpbnQgbnVtX3Zmcyk7DQo+ID4gPiArDQo+ID4gPiArCWlmICgh
KHZkZXYtPmNvbmZpZy0+Z2V0X3N0YXR1cyh2ZGV2KSAmIFZJUlRJT19DT05GSUdfU19EUklWRVJf
T0spKQ0KPiA+ID4gKwkJcmV0dXJuIC1FQlVTWTsNCj4gPiA+ICsNCj4gPiA+ICsJaWYgKCFfX3Zp
cnRpb190ZXN0X2JpdCh2ZGV2LCBWSVJUSU9fRl9TUl9JT1YpKQ0KPiA+ID4gKwkJcmV0dXJuIC1F
SU5WQUw7DQo+ID4gPiArDQo+ID4gPiArCXNyaW92X2NvbmZpZ3VyZSA9IHBjaV9zcmlvdl9jb25m
aWd1cmVfc2ltcGxlOw0KPiA+ID4gKwlpZiAoc3Jpb3ZfY29uZmlndXJlID09IE5VTEwpDQo+ID4g
PiArCQlyZXR1cm4gLUVOT0VOVDsNCj4gPiANCj4gPiBCVFcgd2hhdCBpcyBhbGwgdGhpcyB0cmlj
a2VyeSBpbiBhaWQgb2Y/DQo+IA0KPiBXaGVuIFNSLUlPViBzdXBwb3J0IGlzIG5vdCBjb21waWxl
ZCBpbnRvIHRoZSBrZXJuZWwsICANCj4gcGNpX3NyaW92X2NvbmZpZ3VyZV9zaW1wbGUgaXMgI2Rl
ZmluZWQgYXMgTlVMTC4gVGhpcyBhbGxvd3MgaXQgdG8gY29tcGlsZSAgDQo+IGluIHRoYXQgY2Fz
ZSwgZXZlbiB0aG91Z2ggdGhlcmUgaXMgdXR0ZXJseSBubyB3YXkgZm9yIGl0IHRvIGJlIGNhbGxl
ZCBpbiAgDQo+IHRoYXQgY2FzZS4gSXQgaXMgYW4gYWx0ZXJuYXRpdmUgdG8gI2lmcyBpbiB0aGUg
Y29kZS4NCg0KV2h5IGV2ZW4gaGF2ZSB0aGUgY2FsbCB0aG91Z2g/IEkgd291bGQgd3JhcCBhbGwg
b2YgdGhpcyBpbiBhbiAjaWZkZWYNCmFuZCBzdHJpcCBpdCBvdXQgc2luY2UgeW91IGNvdWxkbid0
IHN1cHBvcnQgU1ItSU9WIGlmIGl0IGlzbid0IHByZXNlbnQNCmluIHRoZSBrZXJuZWwgYW55d2F5
Lg==
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:44 ` Rustad, Mark D
` (2 preceding siblings ...)
(?)
@ 2018-05-30 16:54 ` Duyck, Alexander H
-1 siblings, 0 replies; 55+ messages in thread
From: Duyck, Alexander H @ 2018-05-30 16:54 UTC (permalink / raw)
To: Rustad, Mark D, mst
Cc: virtio-dev, linux-pci, linux-kernel, virtualization, Wang,
Zhihong, bhelgaas
On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
> On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
> > > num_vfs)
> > > +{
> > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> > > + struct virtio_device *vdev = &vp_dev->vdev;
> > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> > > +
> > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> > > + return -EBUSY;
> > > +
> > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> > > + return -EINVAL;
> > > +
> > > + sriov_configure = pci_sriov_configure_simple;
> > > + if (sriov_configure == NULL)
> > > + return -ENOENT;
> >
> > BTW what is all this trickery in aid of?
>
> When SR-IOV support is not compiled into the kernel,
> pci_sriov_configure_simple is #defined as NULL. This allows it to compile
> in that case, even though there is utterly no way for it to be called in
> that case. It is an alternative to #ifs in the code.
Why even have the call though? I would wrap all of this in an #ifdef
and strip it out since you couldn't support SR-IOV if it isn't present
in the kernel anyway.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:54 ` Duyck, Alexander H
(?)
@ 2018-05-30 17:11 ` Rustad, Mark D
-1 siblings, 0 replies; 55+ messages in thread
From: Rustad, Mark D @ 2018-05-30 17:11 UTC (permalink / raw)
To: Duyck, Alexander H
Cc: mst, linux-kernel, virtualization, virtio-dev, Daly, Dan, Bie,
Tiwei, linux-pci, bhelgaas, Liang, Cunming, Wang, Zhihong
[-- Attachment #1: Type: text/plain, Size: 1571 bytes --]
On May 30, 2018, at 9:54 AM, Duyck, Alexander H
<alexander.h.duyck@intel.com> wrote:
> On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
>> On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>>>> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
>>>> num_vfs)
>>>> +{
>>>> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>>>> + struct virtio_device *vdev = &vp_dev->vdev;
>>>> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
>>>> +
>>>> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
>>>> + return -EBUSY;
>>>> +
>>>> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
>>>> + return -EINVAL;
>>>> +
>>>> + sriov_configure = pci_sriov_configure_simple;
>>>> + if (sriov_configure == NULL)
>>>> + return -ENOENT;
>>>
>>> BTW what is all this trickery in aid of?
>>
>> When SR-IOV support is not compiled into the kernel,
>> pci_sriov_configure_simple is #defined as NULL. This allows it to compile
>> in that case, even though there is utterly no way for it to be called in
>> that case. It is an alternative to #ifs in the code.
>
> Why even have the call though? I would wrap all of this in an #ifdef
> and strip it out since you couldn't support SR-IOV if it isn't present
> in the kernel anyway.
I am inclined to agree. In this case, the presence of #ifdefs I think would
be clearer. As written, someone will want to get rid of the pointer only to
create a build problem when SR-IOV is not configured.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 17:11 ` Rustad, Mark D
0 siblings, 0 replies; 55+ messages in thread
From: Rustad, Mark D @ 2018-05-30 17:11 UTC (permalink / raw)
To: Duyck, Alexander H
Cc: virtio-dev, mst, linux-pci, linux-kernel, virtualization, Wang,
Zhihong, bhelgaas
[-- Attachment #1.1: Type: text/plain, Size: 1571 bytes --]
On May 30, 2018, at 9:54 AM, Duyck, Alexander H
<alexander.h.duyck@intel.com> wrote:
> On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
>> On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>>>> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
>>>> num_vfs)
>>>> +{
>>>> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>>>> + struct virtio_device *vdev = &vp_dev->vdev;
>>>> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
>>>> +
>>>> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
>>>> + return -EBUSY;
>>>> +
>>>> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
>>>> + return -EINVAL;
>>>> +
>>>> + sriov_configure = pci_sriov_configure_simple;
>>>> + if (sriov_configure == NULL)
>>>> + return -ENOENT;
>>>
>>> BTW what is all this trickery in aid of?
>>
>> When SR-IOV support is not compiled into the kernel,
>> pci_sriov_configure_simple is #defined as NULL. This allows it to compile
>> in that case, even though there is utterly no way for it to be called in
>> that case. It is an alternative to #ifs in the code.
>
> Why even have the call though? I would wrap all of this in an #ifdef
> and strip it out since you couldn't support SR-IOV if it isn't present
> in the kernel anyway.
I am inclined to agree. In this case, the presence of #ifdefs I think would
be clearer. As written, someone will want to get rid of the pointer only to
create a build problem when SR-IOV is not configured.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #1.2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 55+ messages in thread
* [virtio-dev] Re: [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 17:11 ` Rustad, Mark D
0 siblings, 0 replies; 55+ messages in thread
From: Rustad, Mark D @ 2018-05-30 17:11 UTC (permalink / raw)
To: Duyck, Alexander H
Cc: mst, linux-kernel, virtualization, virtio-dev, Daly, Dan, Bie,
Tiwei, linux-pci, bhelgaas, Liang, Cunming, Wang, Zhihong
[-- Attachment #1: Type: text/plain, Size: 1571 bytes --]
On May 30, 2018, at 9:54 AM, Duyck, Alexander H
<alexander.h.duyck@intel.com> wrote:
> On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
>> On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>>>> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
>>>> num_vfs)
>>>> +{
>>>> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>>>> + struct virtio_device *vdev = &vp_dev->vdev;
>>>> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
>>>> +
>>>> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
>>>> + return -EBUSY;
>>>> +
>>>> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
>>>> + return -EINVAL;
>>>> +
>>>> + sriov_configure = pci_sriov_configure_simple;
>>>> + if (sriov_configure == NULL)
>>>> + return -ENOENT;
>>>
>>> BTW what is all this trickery in aid of?
>>
>> When SR-IOV support is not compiled into the kernel,
>> pci_sriov_configure_simple is #defined as NULL. This allows it to compile
>> in that case, even though there is utterly no way for it to be called in
>> that case. It is an alternative to #ifs in the code.
>
> Why even have the call though? I would wrap all of this in an #ifdef
> and strip it out since you couldn't support SR-IOV if it isn't present
> in the kernel anyway.
I am inclined to agree. In this case, the presence of #ifdefs I think would
be clearer. As written, someone will want to get rid of the pointer only to
create a build problem when SR-IOV is not configured.
--
Mark Rustad, Networking Division, Intel Corporation
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:26 ` Duyck, Alexander H
@ 2018-05-30 17:15 ` Michael S. Tsirkin
-1 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 17:15 UTC (permalink / raw)
To: Duyck, Alexander H
Cc: alexander.duyck, linux-kernel, virtualization, virtio-dev,
Rustad, Mark D, Daly, Dan, Bie, Tiwei, linux-pci, bhelgaas,
Liang, Cunming, Wang, Zhihong
On Wed, May 30, 2018 at 04:26:30PM +0000, Duyck, Alexander H wrote:
> On Wed, 2018-05-30 at 19:22 +0300, Michael S. Tsirkin wrote:
> > On Wed, May 30, 2018 at 09:10:57AM -0700, Alexander Duyck wrote:
> > > On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > > > There is a new feature bit allocated in virtio spec to
> > > > support SR-IOV (Single Root I/O Virtualization):
> > > >
> > > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > > >
> > > > This patch enables the support for this feature bit in
> > > > virtio driver.
> > > >
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > >
> > > So from a quick glance it looks like we are leaving SR-IOV enabled if
> > > the driver is removed. Do we want to have that behavior or should we
> > > be adding the code to disable SR-IOV and free the VFs on driver
> > > removal?
> >
> > Could pci core handle it for us somehow?
>
> Maybe, but it would require changes to the pci core to do it.
>
> The problem is some drivers want to leave the VFs there since the PF
> doesn't really do anything, or they have the option of essentially
> putting the VFs into a standby state when the PF is gone.
>
> My main concern is do we care if VFs are allocated and then somebody
> removes the driver and binds a different driver to the interface? If
> not then this code and be left as is, but I just wanted to be certain
> since I know this isn't just enabling SR-IOV we are having to do a
> number of other checks against the virtio device.
Well the spec says features have to be negotiated, and since we reset
the device when we unbind from it I think it's a given we should keep a
driver bound to the PF.
IOW until we are sure we need the capability to keep it enabled, let's
disable it to be safe.
--
MST
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:26 ` Duyck, Alexander H
(?)
@ 2018-05-30 17:15 ` Michael S. Tsirkin
-1 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 17:15 UTC (permalink / raw)
To: Duyck, Alexander H
Cc: virtio-dev, linux-pci, linux-kernel, alexander.duyck,
virtualization, Wang, Zhihong, bhelgaas, Rustad, Mark D
On Wed, May 30, 2018 at 04:26:30PM +0000, Duyck, Alexander H wrote:
> On Wed, 2018-05-30 at 19:22 +0300, Michael S. Tsirkin wrote:
> > On Wed, May 30, 2018 at 09:10:57AM -0700, Alexander Duyck wrote:
> > > On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > > > There is a new feature bit allocated in virtio spec to
> > > > support SR-IOV (Single Root I/O Virtualization):
> > > >
> > > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > > >
> > > > This patch enables the support for this feature bit in
> > > > virtio driver.
> > > >
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > >
> > > So from a quick glance it looks like we are leaving SR-IOV enabled if
> > > the driver is removed. Do we want to have that behavior or should we
> > > be adding the code to disable SR-IOV and free the VFs on driver
> > > removal?
> >
> > Could pci core handle it for us somehow?
>
> Maybe, but it would require changes to the pci core to do it.
>
> The problem is some drivers want to leave the VFs there since the PF
> doesn't really do anything, or they have the option of essentially
> putting the VFs into a standby state when the PF is gone.
>
> My main concern is do we care if VFs are allocated and then somebody
> removes the driver and binds a different driver to the interface? If
> not then this code and be left as is, but I just wanted to be certain
> since I know this isn't just enabling SR-IOV we are having to do a
> number of other checks against the virtio device.
Well the spec says features have to be negotiated, and since we reset
the device when we unbind from it I think it's a given we should keep a
driver bound to the PF.
IOW until we are sure we need the capability to keep it enabled, let's
disable it to be safe.
--
MST
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 17:15 ` Michael S. Tsirkin
0 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-05-30 17:15 UTC (permalink / raw)
To: Duyck, Alexander H
Cc: alexander.duyck, linux-kernel, virtualization, virtio-dev,
Rustad, Mark D, Daly, Dan, Bie, Tiwei, linux-pci, bhelgaas,
Liang, Cunming, Wang, Zhihong
On Wed, May 30, 2018 at 04:26:30PM +0000, Duyck, Alexander H wrote:
> On Wed, 2018-05-30 at 19:22 +0300, Michael S. Tsirkin wrote:
> > On Wed, May 30, 2018 at 09:10:57AM -0700, Alexander Duyck wrote:
> > > On Wed, May 30, 2018 at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > > > There is a new feature bit allocated in virtio spec to
> > > > support SR-IOV (Single Root I/O Virtualization):
> > > >
> > > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > > >
> > > > This patch enables the support for this feature bit in
> > > > virtio driver.
> > > >
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > >
> > > So from a quick glance it looks like we are leaving SR-IOV enabled if
> > > the driver is removed. Do we want to have that behavior or should we
> > > be adding the code to disable SR-IOV and free the VFs on driver
> > > removal?
> >
> > Could pci core handle it for us somehow?
>
> Maybe, but it would require changes to the pci core to do it.
>
> The problem is some drivers want to leave the VFs there since the PF
> doesn't really do anything, or they have the option of essentially
> putting the VFs into a standby state when the PF is gone.
>
> My main concern is do we care if VFs are allocated and then somebody
> removes the driver and binds a different driver to the interface? If
> not then this code and be left as is, but I just wanted to be certain
> since I know this isn't just enabling SR-IOV we are having to do a
> number of other checks against the virtio device.
Well the spec says features have to be negotiated, and since we reset
the device when we unbind from it I think it's a given we should keep a
driver bound to the PF.
IOW until we are sure we need the capability to keep it enabled, let's
disable it to be safe.
--
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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:09 ` Michael S. Tsirkin
@ 2018-05-31 2:55 ` Tiwei Bie
-1 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-05-31 2:55 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Rustad, Mark D, Bjorn Helgaas, virtualization, LKML, virtio-dev,
linux-pci, Daly, Dan, Duyck, Alexander H, Liang, Cunming, Wang,
Zhihong
On Wed, May 30, 2018 at 07:09:37PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> > On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> >
> > > There is a new feature bit allocated in virtio spec to
> > > support SR-IOV (Single Root I/O Virtualization):
> > >
> > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > >
> > > This patch enables the support for this feature bit in
> > > virtio driver.
> > >
> > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > ---
> > > This patch depends on below proposal:
> > > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> > >
> > > This patch depends on below patch:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> > >
> > > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > > include/uapi/linux/virtio_config.h | 7 ++++++-
> > > 3 files changed, 40 insertions(+), 1 deletion(-)
> > >
> >
> > <snip>
> >
> > > diff --git a/include/uapi/linux/virtio_config.h
> > > b/include/uapi/linux/virtio_config.h
> > > index 308e2096291f..b7c1f4e7d59e 100644
> > > --- a/include/uapi/linux/virtio_config.h
> > > +++ b/include/uapi/linux/virtio_config.h
> > > @@ -49,7 +49,7 @@
> >
> > There is a value in the comment directly before this that should
> > be updated as well to be consistent with the new value for
> > VIRTIO_TRANSPORT_F_END below.
>
> It hasn't been updated to 34 yet.
> I suggest a separate patch to replace the numbers with
> VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
> in the comment.
> Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
> virtio_pci etc." as well.
Good idea! Thanks for the suggestion! I'll do it.
Best regards,
Tiwei Bie
>
> > > * transport being used (eg. virtio_ring), the rest are per-device feature
> > > * bits. */
> > > #define VIRTIO_TRANSPORT_F_START 28
> > > -#define VIRTIO_TRANSPORT_F_END 34
> > > +#define VIRTIO_TRANSPORT_F_END 38
> > >
> > > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > /* Do we get callbacks when the ring is completely used, even if we've
> >
> > <snip>
> >
> > --
> > Mark Rustad, Networking Division, Intel Corporation
>
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-30 16:09 ` Michael S. Tsirkin
(?)
(?)
@ 2018-05-31 2:55 ` Tiwei Bie
-1 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-05-31 2:55 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Duyck, Alexander H, virtio-dev, linux-pci, LKML, virtualization,
Wang, Zhihong, Bjorn Helgaas, Rustad, Mark D
On Wed, May 30, 2018 at 07:09:37PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> > On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> >
> > > There is a new feature bit allocated in virtio spec to
> > > support SR-IOV (Single Root I/O Virtualization):
> > >
> > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > >
> > > This patch enables the support for this feature bit in
> > > virtio driver.
> > >
> > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > ---
> > > This patch depends on below proposal:
> > > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> > >
> > > This patch depends on below patch:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> > >
> > > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > > include/uapi/linux/virtio_config.h | 7 ++++++-
> > > 3 files changed, 40 insertions(+), 1 deletion(-)
> > >
> >
> > <snip>
> >
> > > diff --git a/include/uapi/linux/virtio_config.h
> > > b/include/uapi/linux/virtio_config.h
> > > index 308e2096291f..b7c1f4e7d59e 100644
> > > --- a/include/uapi/linux/virtio_config.h
> > > +++ b/include/uapi/linux/virtio_config.h
> > > @@ -49,7 +49,7 @@
> >
> > There is a value in the comment directly before this that should
> > be updated as well to be consistent with the new value for
> > VIRTIO_TRANSPORT_F_END below.
>
> It hasn't been updated to 34 yet.
> I suggest a separate patch to replace the numbers with
> VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
> in the comment.
> Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
> virtio_pci etc." as well.
Good idea! Thanks for the suggestion! I'll do it.
Best regards,
Tiwei Bie
>
> > > * transport being used (eg. virtio_ring), the rest are per-device feature
> > > * bits. */
> > > #define VIRTIO_TRANSPORT_F_START 28
> > > -#define VIRTIO_TRANSPORT_F_END 34
> > > +#define VIRTIO_TRANSPORT_F_END 38
> > >
> > > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > /* Do we get callbacks when the ring is completely used, even if we've
> >
> > <snip>
> >
> > --
> > Mark Rustad, Networking Division, Intel Corporation
>
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
@ 2018-05-31 2:55 ` Tiwei Bie
0 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-05-31 2:55 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Rustad, Mark D, Bjorn Helgaas, virtualization, LKML, virtio-dev,
linux-pci, Daly, Dan, Duyck, Alexander H, Liang, Cunming, Wang,
Zhihong
On Wed, May 30, 2018 at 07:09:37PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> > On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> >
> > > There is a new feature bit allocated in virtio spec to
> > > support SR-IOV (Single Root I/O Virtualization):
> > >
> > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > >
> > > This patch enables the support for this feature bit in
> > > virtio driver.
> > >
> > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > ---
> > > This patch depends on below proposal:
> > > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> > >
> > > This patch depends on below patch:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> > >
> > > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > > include/uapi/linux/virtio_config.h | 7 ++++++-
> > > 3 files changed, 40 insertions(+), 1 deletion(-)
> > >
> >
> > <snip>
> >
> > > diff --git a/include/uapi/linux/virtio_config.h
> > > b/include/uapi/linux/virtio_config.h
> > > index 308e2096291f..b7c1f4e7d59e 100644
> > > --- a/include/uapi/linux/virtio_config.h
> > > +++ b/include/uapi/linux/virtio_config.h
> > > @@ -49,7 +49,7 @@
> >
> > There is a value in the comment directly before this that should
> > be updated as well to be consistent with the new value for
> > VIRTIO_TRANSPORT_F_END below.
>
> It hasn't been updated to 34 yet.
> I suggest a separate patch to replace the numbers with
> VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
> in the comment.
> Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
> virtio_pci etc." as well.
Good idea! Thanks for the suggestion! I'll do it.
Best regards,
Tiwei Bie
>
> > > * transport being used (eg. virtio_ring), the rest are per-device feature
> > > * bits. */
> > > #define VIRTIO_TRANSPORT_F_START 28
> > > -#define VIRTIO_TRANSPORT_F_END 34
> > > +#define VIRTIO_TRANSPORT_F_END 38
> > >
> > > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > /* Do we get callbacks when the ring is completely used, even if we've
> >
> > <snip>
> >
> > --
> > Mark Rustad, Networking Division, Intel Corporation
>
>
---------------------------------------------------------------------
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] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-30 17:11 ` Rustad, Mark D
@ 2018-05-31 3:20 ` Tiwei Bie
-1 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-05-31 3:20 UTC (permalink / raw)
To: Duyck, Alexander H, Rustad, Mark D
Cc: mst, linux-kernel, virtualization, virtio-dev, Daly, Dan,
linux-pci, bhelgaas, Liang, Cunming, Wang, Zhihong
On Thu, May 31, 2018 at 01:11:37AM +0800, Rustad, Mark D wrote:
> On May 30, 2018, at 9:54 AM, Duyck, Alexander H
> <alexander.h.duyck@intel.com> wrote:
>
> > On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
> > > On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
> > > > > num_vfs)
> > > > > +{
> > > > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> > > > > + struct virtio_device *vdev = &vp_dev->vdev;
> > > > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> > > > > +
> > > > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> > > > > + return -EBUSY;
> > > > > +
> > > > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> > > > > + return -EINVAL;
> > > > > +
> > > > > + sriov_configure = pci_sriov_configure_simple;
> > > > > + if (sriov_configure == NULL)
> > > > > + return -ENOENT;
> > > >
> > > > BTW what is all this trickery in aid of?
> > >
> > > When SR-IOV support is not compiled into the kernel,
> > > pci_sriov_configure_simple is #defined as NULL. This allows it to compile
> > > in that case, even though there is utterly no way for it to be called in
> > > that case. It is an alternative to #ifs in the code.
> >
> > Why even have the call though? I would wrap all of this in an #ifdef
> > and strip it out since you couldn't support SR-IOV if it isn't present
> > in the kernel anyway.
>
> I am inclined to agree. In this case, the presence of #ifdefs I think would
> be clearer. As written, someone will want to get rid of the pointer only to
> create a build problem when SR-IOV is not configured.
In my opinion, maybe it would be better to make
pci_sriov_configure_simple() always available
just like other sriov functions.
Based on the comments in the original patch:
https://patchwork.kernel.org/patch/10353197/
"""
+/* this is expected to be used as a function pointer, just define as NULL */
+#define pci_sriov_configure_simple NULL
"""
This function could be defined as NULL just because
it was expected to be used as a function pointer.
But actually it could be called directly as a
function, just like this case.
So I prefer to make this function always available
just like other sriov functions.
Best regards,
Tiwei Bie
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-30 17:11 ` Rustad, Mark D
` (2 preceding siblings ...)
(?)
@ 2018-05-31 3:20 ` Tiwei Bie
-1 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-05-31 3:20 UTC (permalink / raw)
To: Duyck, Alexander H, Rustad, Mark D
Cc: virtio-dev, mst, linux-pci, linux-kernel, virtualization, Wang,
Zhihong, bhelgaas
On Thu, May 31, 2018 at 01:11:37AM +0800, Rustad, Mark D wrote:
> On May 30, 2018, at 9:54 AM, Duyck, Alexander H
> <alexander.h.duyck@intel.com> wrote:
>
> > On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
> > > On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
> > > > > num_vfs)
> > > > > +{
> > > > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> > > > > + struct virtio_device *vdev = &vp_dev->vdev;
> > > > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> > > > > +
> > > > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> > > > > + return -EBUSY;
> > > > > +
> > > > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> > > > > + return -EINVAL;
> > > > > +
> > > > > + sriov_configure = pci_sriov_configure_simple;
> > > > > + if (sriov_configure == NULL)
> > > > > + return -ENOENT;
> > > >
> > > > BTW what is all this trickery in aid of?
> > >
> > > When SR-IOV support is not compiled into the kernel,
> > > pci_sriov_configure_simple is #defined as NULL. This allows it to compile
> > > in that case, even though there is utterly no way for it to be called in
> > > that case. It is an alternative to #ifs in the code.
> >
> > Why even have the call though? I would wrap all of this in an #ifdef
> > and strip it out since you couldn't support SR-IOV if it isn't present
> > in the kernel anyway.
>
> I am inclined to agree. In this case, the presence of #ifdefs I think would
> be clearer. As written, someone will want to get rid of the pointer only to
> create a build problem when SR-IOV is not configured.
In my opinion, maybe it would be better to make
pci_sriov_configure_simple() always available
just like other sriov functions.
Based on the comments in the original patch:
https://patchwork.kernel.org/patch/10353197/
"""
+/* this is expected to be used as a function pointer, just define as NULL */
+#define pci_sriov_configure_simple NULL
"""
This function could be defined as NULL just because
it was expected to be used as a function pointer.
But actually it could be called directly as a
function, just like this case.
So I prefer to make this function always available
just like other sriov functions.
Best regards,
Tiwei Bie
^ permalink raw reply [flat|nested] 55+ messages in thread
* [virtio-dev] Re: [PATCH] virtio_pci: support enabling VFs
@ 2018-05-31 3:20 ` Tiwei Bie
0 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-05-31 3:20 UTC (permalink / raw)
To: Duyck, Alexander H, Rustad, Mark D
Cc: mst, linux-kernel, virtualization, virtio-dev, Daly, Dan,
linux-pci, bhelgaas, Liang, Cunming, Wang, Zhihong
On Thu, May 31, 2018 at 01:11:37AM +0800, Rustad, Mark D wrote:
> On May 30, 2018, at 9:54 AM, Duyck, Alexander H
> <alexander.h.duyck@intel.com> wrote:
>
> > On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
> > > On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
> > > > > num_vfs)
> > > > > +{
> > > > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> > > > > + struct virtio_device *vdev = &vp_dev->vdev;
> > > > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> > > > > +
> > > > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> > > > > + return -EBUSY;
> > > > > +
> > > > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> > > > > + return -EINVAL;
> > > > > +
> > > > > + sriov_configure = pci_sriov_configure_simple;
> > > > > + if (sriov_configure == NULL)
> > > > > + return -ENOENT;
> > > >
> > > > BTW what is all this trickery in aid of?
> > >
> > > When SR-IOV support is not compiled into the kernel,
> > > pci_sriov_configure_simple is #defined as NULL. This allows it to compile
> > > in that case, even though there is utterly no way for it to be called in
> > > that case. It is an alternative to #ifs in the code.
> >
> > Why even have the call though? I would wrap all of this in an #ifdef
> > and strip it out since you couldn't support SR-IOV if it isn't present
> > in the kernel anyway.
>
> I am inclined to agree. In this case, the presence of #ifdefs I think would
> be clearer. As written, someone will want to get rid of the pointer only to
> create a build problem when SR-IOV is not configured.
In my opinion, maybe it would be better to make
pci_sriov_configure_simple() always available
just like other sriov functions.
Based on the comments in the original patch:
https://patchwork.kernel.org/patch/10353197/
"""
+/* this is expected to be used as a function pointer, just define as NULL */
+#define pci_sriov_configure_simple NULL
"""
This function could be defined as NULL just because
it was expected to be used as a function pointer.
But actually it could be called directly as a
function, just like this case.
So I prefer to make this function always available
just like other sriov functions.
Best regards,
Tiwei Bie
---------------------------------------------------------------------
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] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-31 3:20 ` [virtio-dev] " Tiwei Bie
@ 2018-05-31 14:27 ` Alexander Duyck
-1 siblings, 0 replies; 55+ messages in thread
From: Alexander Duyck @ 2018-05-31 14:27 UTC (permalink / raw)
To: Tiwei Bie
Cc: Duyck, Alexander H, Rustad, Mark D, mst, linux-kernel,
virtualization, virtio-dev, Daly, Dan, linux-pci, bhelgaas,
Liang, Cunming, Wang, Zhihong
On Wed, May 30, 2018 at 8:20 PM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> On Thu, May 31, 2018 at 01:11:37AM +0800, Rustad, Mark D wrote:
>> On May 30, 2018, at 9:54 AM, Duyck, Alexander H
>> <alexander.h.duyck@intel.com> wrote:
>>
>> > On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
>> > > On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > >
>> > > > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
>> > > > > num_vfs)
>> > > > > +{
>> > > > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>> > > > > + struct virtio_device *vdev = &vp_dev->vdev;
>> > > > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
>> > > > > +
>> > > > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
>> > > > > + return -EBUSY;
>> > > > > +
>> > > > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
>> > > > > + return -EINVAL;
>> > > > > +
>> > > > > + sriov_configure = pci_sriov_configure_simple;
>> > > > > + if (sriov_configure == NULL)
>> > > > > + return -ENOENT;
>> > > >
>> > > > BTW what is all this trickery in aid of?
>> > >
>> > > When SR-IOV support is not compiled into the kernel,
>> > > pci_sriov_configure_simple is #defined as NULL. This allows it to compile
>> > > in that case, even though there is utterly no way for it to be called in
>> > > that case. It is an alternative to #ifs in the code.
>> >
>> > Why even have the call though? I would wrap all of this in an #ifdef
>> > and strip it out since you couldn't support SR-IOV if it isn't present
>> > in the kernel anyway.
>>
>> I am inclined to agree. In this case, the presence of #ifdefs I think would
>> be clearer. As written, someone will want to get rid of the pointer only to
>> create a build problem when SR-IOV is not configured.
>
> In my opinion, maybe it would be better to make
> pci_sriov_configure_simple() always available
> just like other sriov functions.
>
> Based on the comments in the original patch:
>
> https://patchwork.kernel.org/patch/10353197/
> """
> +/* this is expected to be used as a function pointer, just define as NULL */
> +#define pci_sriov_configure_simple NULL
> """
>
> This function could be defined as NULL just because
> it was expected to be used as a function pointer.
> But actually it could be called directly as a
> function, just like this case.
>
> So I prefer to make this function always available
> just like other sriov functions.
>
> Best regards,
> Tiwei Bie
The fact that you are having to add additional code kind of implies
that maybe this doesn't fall into the pci_sriov_configure_simple case
anymore. The PF itself is defining what the VF can and can't do via
the feature flags you are testing for.
For example how is the bit of code below valid if the kernel itself
doesn't support SR-IOV:
+static void vp_transport_features(struct virtio_device *vdev, u64 features)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct pci_dev *pci_dev = vp_dev->pci_dev;
+
+ if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
+ pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
+ __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
+}
+
It really seems like we should be wrapping these functions at the very
minimum so that they don't imply you have SR-IOV support when it isn't
supported in the kernel.
Also it seems like we should be disabling the VFs if the driver is
unbound from this interface. We need to add logic to disable SR-IOV if
the driver is removed. What we don't want to do is leave VFs allocated
and then have the potential for us to unbind/rebind the driver as the
new driver may change the negotiated features.
- Alex
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-31 3:20 ` [virtio-dev] " Tiwei Bie
(?)
@ 2018-05-31 14:27 ` Alexander Duyck
-1 siblings, 0 replies; 55+ messages in thread
From: Alexander Duyck @ 2018-05-31 14:27 UTC (permalink / raw)
To: Tiwei Bie
Cc: Duyck, Alexander H, virtio-dev, mst, linux-pci, linux-kernel,
virtualization, Wang, Zhihong, bhelgaas, Rustad, Mark D
On Wed, May 30, 2018 at 8:20 PM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> On Thu, May 31, 2018 at 01:11:37AM +0800, Rustad, Mark D wrote:
>> On May 30, 2018, at 9:54 AM, Duyck, Alexander H
>> <alexander.h.duyck@intel.com> wrote:
>>
>> > On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
>> > > On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > >
>> > > > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
>> > > > > num_vfs)
>> > > > > +{
>> > > > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>> > > > > + struct virtio_device *vdev = &vp_dev->vdev;
>> > > > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
>> > > > > +
>> > > > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
>> > > > > + return -EBUSY;
>> > > > > +
>> > > > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
>> > > > > + return -EINVAL;
>> > > > > +
>> > > > > + sriov_configure = pci_sriov_configure_simple;
>> > > > > + if (sriov_configure == NULL)
>> > > > > + return -ENOENT;
>> > > >
>> > > > BTW what is all this trickery in aid of?
>> > >
>> > > When SR-IOV support is not compiled into the kernel,
>> > > pci_sriov_configure_simple is #defined as NULL. This allows it to compile
>> > > in that case, even though there is utterly no way for it to be called in
>> > > that case. It is an alternative to #ifs in the code.
>> >
>> > Why even have the call though? I would wrap all of this in an #ifdef
>> > and strip it out since you couldn't support SR-IOV if it isn't present
>> > in the kernel anyway.
>>
>> I am inclined to agree. In this case, the presence of #ifdefs I think would
>> be clearer. As written, someone will want to get rid of the pointer only to
>> create a build problem when SR-IOV is not configured.
>
> In my opinion, maybe it would be better to make
> pci_sriov_configure_simple() always available
> just like other sriov functions.
>
> Based on the comments in the original patch:
>
> https://patchwork.kernel.org/patch/10353197/
> """
> +/* this is expected to be used as a function pointer, just define as NULL */
> +#define pci_sriov_configure_simple NULL
> """
>
> This function could be defined as NULL just because
> it was expected to be used as a function pointer.
> But actually it could be called directly as a
> function, just like this case.
>
> So I prefer to make this function always available
> just like other sriov functions.
>
> Best regards,
> Tiwei Bie
The fact that you are having to add additional code kind of implies
that maybe this doesn't fall into the pci_sriov_configure_simple case
anymore. The PF itself is defining what the VF can and can't do via
the feature flags you are testing for.
For example how is the bit of code below valid if the kernel itself
doesn't support SR-IOV:
+static void vp_transport_features(struct virtio_device *vdev, u64 features)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct pci_dev *pci_dev = vp_dev->pci_dev;
+
+ if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
+ pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
+ __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
+}
+
It really seems like we should be wrapping these functions at the very
minimum so that they don't imply you have SR-IOV support when it isn't
supported in the kernel.
Also it seems like we should be disabling the VFs if the driver is
unbound from this interface. We need to add logic to disable SR-IOV if
the driver is removed. What we don't want to do is leave VFs allocated
and then have the potential for us to unbind/rebind the driver as the
new driver may change the negotiated features.
- Alex
^ permalink raw reply [flat|nested] 55+ messages in thread
* [virtio-dev] Re: [PATCH] virtio_pci: support enabling VFs
@ 2018-05-31 14:27 ` Alexander Duyck
0 siblings, 0 replies; 55+ messages in thread
From: Alexander Duyck @ 2018-05-31 14:27 UTC (permalink / raw)
To: Tiwei Bie
Cc: Duyck, Alexander H, Rustad, Mark D, mst, linux-kernel,
virtualization, virtio-dev, Daly, Dan, linux-pci, bhelgaas,
Liang, Cunming, Wang, Zhihong
On Wed, May 30, 2018 at 8:20 PM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> On Thu, May 31, 2018 at 01:11:37AM +0800, Rustad, Mark D wrote:
>> On May 30, 2018, at 9:54 AM, Duyck, Alexander H
>> <alexander.h.duyck@intel.com> wrote:
>>
>> > On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
>> > > On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > >
>> > > > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
>> > > > > num_vfs)
>> > > > > +{
>> > > > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>> > > > > + struct virtio_device *vdev = &vp_dev->vdev;
>> > > > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
>> > > > > +
>> > > > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
>> > > > > + return -EBUSY;
>> > > > > +
>> > > > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
>> > > > > + return -EINVAL;
>> > > > > +
>> > > > > + sriov_configure = pci_sriov_configure_simple;
>> > > > > + if (sriov_configure == NULL)
>> > > > > + return -ENOENT;
>> > > >
>> > > > BTW what is all this trickery in aid of?
>> > >
>> > > When SR-IOV support is not compiled into the kernel,
>> > > pci_sriov_configure_simple is #defined as NULL. This allows it to compile
>> > > in that case, even though there is utterly no way for it to be called in
>> > > that case. It is an alternative to #ifs in the code.
>> >
>> > Why even have the call though? I would wrap all of this in an #ifdef
>> > and strip it out since you couldn't support SR-IOV if it isn't present
>> > in the kernel anyway.
>>
>> I am inclined to agree. In this case, the presence of #ifdefs I think would
>> be clearer. As written, someone will want to get rid of the pointer only to
>> create a build problem when SR-IOV is not configured.
>
> In my opinion, maybe it would be better to make
> pci_sriov_configure_simple() always available
> just like other sriov functions.
>
> Based on the comments in the original patch:
>
> https://patchwork.kernel.org/patch/10353197/
> """
> +/* this is expected to be used as a function pointer, just define as NULL */
> +#define pci_sriov_configure_simple NULL
> """
>
> This function could be defined as NULL just because
> it was expected to be used as a function pointer.
> But actually it could be called directly as a
> function, just like this case.
>
> So I prefer to make this function always available
> just like other sriov functions.
>
> Best regards,
> Tiwei Bie
The fact that you are having to add additional code kind of implies
that maybe this doesn't fall into the pci_sriov_configure_simple case
anymore. The PF itself is defining what the VF can and can't do via
the feature flags you are testing for.
For example how is the bit of code below valid if the kernel itself
doesn't support SR-IOV:
+static void vp_transport_features(struct virtio_device *vdev, u64 features)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct pci_dev *pci_dev = vp_dev->pci_dev;
+
+ if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
+ pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
+ __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
+}
+
It really seems like we should be wrapping these functions at the very
minimum so that they don't imply you have SR-IOV support when it isn't
supported in the kernel.
Also it seems like we should be disabling the VFs if the driver is
unbound from this interface. We need to add logic to disable SR-IOV if
the driver is removed. What we don't want to do is leave VFs allocated
and then have the potential for us to unbind/rebind the driver as the
new driver may change the negotiated features.
- 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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-31 2:55 ` Tiwei Bie
@ 2018-06-01 1:21 ` Michael S. Tsirkin
-1 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-06-01 1:21 UTC (permalink / raw)
To: Tiwei Bie
Cc: Rustad, Mark D, Bjorn Helgaas, virtualization, LKML, virtio-dev,
linux-pci, Daly, Dan, Duyck, Alexander H, Liang, Cunming, Wang,
Zhihong
On Thu, May 31, 2018 at 10:55:02AM +0800, Tiwei Bie wrote:
> On Wed, May 30, 2018 at 07:09:37PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> > > On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > >
> > > > There is a new feature bit allocated in virtio spec to
> > > > support SR-IOV (Single Root I/O Virtualization):
> > > >
> > > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > > >
> > > > This patch enables the support for this feature bit in
> > > > virtio driver.
> > > >
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > ---
> > > > This patch depends on below proposal:
> > > > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> > > >
> > > > This patch depends on below patch:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> > > >
> > > > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > > > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > > > include/uapi/linux/virtio_config.h | 7 ++++++-
> > > > 3 files changed, 40 insertions(+), 1 deletion(-)
> > > >
> > >
> > > <snip>
> > >
> > > > diff --git a/include/uapi/linux/virtio_config.h
> > > > b/include/uapi/linux/virtio_config.h
> > > > index 308e2096291f..b7c1f4e7d59e 100644
> > > > --- a/include/uapi/linux/virtio_config.h
> > > > +++ b/include/uapi/linux/virtio_config.h
> > > > @@ -49,7 +49,7 @@
> > >
> > > There is a value in the comment directly before this that should
> > > be updated as well to be consistent with the new value for
> > > VIRTIO_TRANSPORT_F_END below.
> >
> > It hasn't been updated to 34 yet.
> > I suggest a separate patch to replace the numbers with
> > VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
> > in the comment.
> > Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
> > virtio_pci etc." as well.
>
> Good idea! Thanks for the suggestion! I'll do it.
>
> Best regards,
> Tiwei Bie
Or just focus on the new feature, tweak start/end
as a separate patch. Up to you, the important thing
is to have a ready to roll patch before the merge
window.
>
> >
> > > > * transport being used (eg. virtio_ring), the rest are per-device feature
> > > > * bits. */
> > > > #define VIRTIO_TRANSPORT_F_START 28
> > > > -#define VIRTIO_TRANSPORT_F_END 34
> > > > +#define VIRTIO_TRANSPORT_F_END 38
> > > >
> > > > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > > /* Do we get callbacks when the ring is completely used, even if we've
> > >
> > > <snip>
> > >
> > > --
> > > Mark Rustad, Networking Division, Intel Corporation
> >
> >
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-05-31 2:55 ` Tiwei Bie
(?)
@ 2018-06-01 1:21 ` Michael S. Tsirkin
-1 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-06-01 1:21 UTC (permalink / raw)
To: Tiwei Bie
Cc: Duyck, Alexander H, virtio-dev, linux-pci, LKML, virtualization,
Wang, Zhihong, Bjorn Helgaas, Rustad, Mark D
On Thu, May 31, 2018 at 10:55:02AM +0800, Tiwei Bie wrote:
> On Wed, May 30, 2018 at 07:09:37PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> > > On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > >
> > > > There is a new feature bit allocated in virtio spec to
> > > > support SR-IOV (Single Root I/O Virtualization):
> > > >
> > > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > > >
> > > > This patch enables the support for this feature bit in
> > > > virtio driver.
> > > >
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > ---
> > > > This patch depends on below proposal:
> > > > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> > > >
> > > > This patch depends on below patch:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> > > >
> > > > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > > > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > > > include/uapi/linux/virtio_config.h | 7 ++++++-
> > > > 3 files changed, 40 insertions(+), 1 deletion(-)
> > > >
> > >
> > > <snip>
> > >
> > > > diff --git a/include/uapi/linux/virtio_config.h
> > > > b/include/uapi/linux/virtio_config.h
> > > > index 308e2096291f..b7c1f4e7d59e 100644
> > > > --- a/include/uapi/linux/virtio_config.h
> > > > +++ b/include/uapi/linux/virtio_config.h
> > > > @@ -49,7 +49,7 @@
> > >
> > > There is a value in the comment directly before this that should
> > > be updated as well to be consistent with the new value for
> > > VIRTIO_TRANSPORT_F_END below.
> >
> > It hasn't been updated to 34 yet.
> > I suggest a separate patch to replace the numbers with
> > VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
> > in the comment.
> > Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
> > virtio_pci etc." as well.
>
> Good idea! Thanks for the suggestion! I'll do it.
>
> Best regards,
> Tiwei Bie
Or just focus on the new feature, tweak start/end
as a separate patch. Up to you, the important thing
is to have a ready to roll patch before the merge
window.
>
> >
> > > > * transport being used (eg. virtio_ring), the rest are per-device feature
> > > > * bits. */
> > > > #define VIRTIO_TRANSPORT_F_START 28
> > > > -#define VIRTIO_TRANSPORT_F_END 34
> > > > +#define VIRTIO_TRANSPORT_F_END 38
> > > >
> > > > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > > /* Do we get callbacks when the ring is completely used, even if we've
> > >
> > > <snip>
> > >
> > > --
> > > Mark Rustad, Networking Division, Intel Corporation
> >
> >
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
@ 2018-06-01 1:21 ` Michael S. Tsirkin
0 siblings, 0 replies; 55+ messages in thread
From: Michael S. Tsirkin @ 2018-06-01 1:21 UTC (permalink / raw)
To: Tiwei Bie
Cc: Rustad, Mark D, Bjorn Helgaas, virtualization, LKML, virtio-dev,
linux-pci, Daly, Dan, Duyck, Alexander H, Liang, Cunming, Wang,
Zhihong
On Thu, May 31, 2018 at 10:55:02AM +0800, Tiwei Bie wrote:
> On Wed, May 30, 2018 at 07:09:37PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> > > On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > >
> > > > There is a new feature bit allocated in virtio spec to
> > > > support SR-IOV (Single Root I/O Virtualization):
> > > >
> > > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > > >
> > > > This patch enables the support for this feature bit in
> > > > virtio driver.
> > > >
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > ---
> > > > This patch depends on below proposal:
> > > > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> > > >
> > > > This patch depends on below patch:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> > > >
> > > > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > > > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > > > include/uapi/linux/virtio_config.h | 7 ++++++-
> > > > 3 files changed, 40 insertions(+), 1 deletion(-)
> > > >
> > >
> > > <snip>
> > >
> > > > diff --git a/include/uapi/linux/virtio_config.h
> > > > b/include/uapi/linux/virtio_config.h
> > > > index 308e2096291f..b7c1f4e7d59e 100644
> > > > --- a/include/uapi/linux/virtio_config.h
> > > > +++ b/include/uapi/linux/virtio_config.h
> > > > @@ -49,7 +49,7 @@
> > >
> > > There is a value in the comment directly before this that should
> > > be updated as well to be consistent with the new value for
> > > VIRTIO_TRANSPORT_F_END below.
> >
> > It hasn't been updated to 34 yet.
> > I suggest a separate patch to replace the numbers with
> > VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
> > in the comment.
> > Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
> > virtio_pci etc." as well.
>
> Good idea! Thanks for the suggestion! I'll do it.
>
> Best regards,
> Tiwei Bie
Or just focus on the new feature, tweak start/end
as a separate patch. Up to you, the important thing
is to have a ready to roll patch before the merge
window.
>
> >
> > > > * transport being used (eg. virtio_ring), the rest are per-device feature
> > > > * bits. */
> > > > #define VIRTIO_TRANSPORT_F_START 28
> > > > -#define VIRTIO_TRANSPORT_F_END 34
> > > > +#define VIRTIO_TRANSPORT_F_END 38
> > > >
> > > > #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > > /* Do we get callbacks when the ring is completely used, even if we've
> > >
> > > <snip>
> > >
> > > --
> > > Mark Rustad, Networking Division, Intel Corporation
> >
> >
---------------------------------------------------------------------
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] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-06-01 1:21 ` Michael S. Tsirkin
@ 2018-06-01 1:36 ` Tiwei Bie
-1 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-06-01 1:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Rustad, Mark D, Bjorn Helgaas, virtualization, LKML, virtio-dev,
linux-pci, Daly, Dan, Duyck, Alexander H, Liang, Cunming, Wang,
Zhihong
On Fri, Jun 01, 2018 at 04:21:03AM +0300, Michael S. Tsirkin wrote:
> On Thu, May 31, 2018 at 10:55:02AM +0800, Tiwei Bie wrote:
> > On Wed, May 30, 2018 at 07:09:37PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> > > > On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > > >
> > > > > There is a new feature bit allocated in virtio spec to
> > > > > support SR-IOV (Single Root I/O Virtualization):
> > > > >
> > > > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > > > >
> > > > > This patch enables the support for this feature bit in
> > > > > virtio driver.
> > > > >
> > > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > > ---
> > > > > This patch depends on below proposal:
> > > > > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> > > > >
> > > > > This patch depends on below patch:
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> > > > >
> > > > > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > > > > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > > > > include/uapi/linux/virtio_config.h | 7 ++++++-
> > > > > 3 files changed, 40 insertions(+), 1 deletion(-)
> > > > >
> > > >
> > > > <snip>
> > > >
> > > > > diff --git a/include/uapi/linux/virtio_config.h
> > > > > b/include/uapi/linux/virtio_config.h
> > > > > index 308e2096291f..b7c1f4e7d59e 100644
> > > > > --- a/include/uapi/linux/virtio_config.h
> > > > > +++ b/include/uapi/linux/virtio_config.h
> > > > > @@ -49,7 +49,7 @@
> > > >
> > > > There is a value in the comment directly before this that should
> > > > be updated as well to be consistent with the new value for
> > > > VIRTIO_TRANSPORT_F_END below.
> > >
> > > It hasn't been updated to 34 yet.
> > > I suggest a separate patch to replace the numbers with
> > > VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
> > > in the comment.
> > > Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
> > > virtio_pci etc." as well.
> >
> > Good idea! Thanks for the suggestion! I'll do it.
> >
> > Best regards,
> > Tiwei Bie
>
> Or just focus on the new feature, tweak start/end
> as a separate patch. Up to you, the important thing
> is to have a ready to roll patch before the merge
> window.
Got it! Thank you very much!
Best regards,
Tiwei Bie
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
2018-06-01 1:21 ` Michael S. Tsirkin
(?)
(?)
@ 2018-06-01 1:36 ` Tiwei Bie
-1 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-06-01 1:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Duyck, Alexander H, virtio-dev, linux-pci, LKML, virtualization,
Wang, Zhihong, Bjorn Helgaas, Rustad, Mark D
On Fri, Jun 01, 2018 at 04:21:03AM +0300, Michael S. Tsirkin wrote:
> On Thu, May 31, 2018 at 10:55:02AM +0800, Tiwei Bie wrote:
> > On Wed, May 30, 2018 at 07:09:37PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> > > > On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > > >
> > > > > There is a new feature bit allocated in virtio spec to
> > > > > support SR-IOV (Single Root I/O Virtualization):
> > > > >
> > > > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > > > >
> > > > > This patch enables the support for this feature bit in
> > > > > virtio driver.
> > > > >
> > > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > > ---
> > > > > This patch depends on below proposal:
> > > > > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> > > > >
> > > > > This patch depends on below patch:
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> > > > >
> > > > > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > > > > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > > > > include/uapi/linux/virtio_config.h | 7 ++++++-
> > > > > 3 files changed, 40 insertions(+), 1 deletion(-)
> > > > >
> > > >
> > > > <snip>
> > > >
> > > > > diff --git a/include/uapi/linux/virtio_config.h
> > > > > b/include/uapi/linux/virtio_config.h
> > > > > index 308e2096291f..b7c1f4e7d59e 100644
> > > > > --- a/include/uapi/linux/virtio_config.h
> > > > > +++ b/include/uapi/linux/virtio_config.h
> > > > > @@ -49,7 +49,7 @@
> > > >
> > > > There is a value in the comment directly before this that should
> > > > be updated as well to be consistent with the new value for
> > > > VIRTIO_TRANSPORT_F_END below.
> > >
> > > It hasn't been updated to 34 yet.
> > > I suggest a separate patch to replace the numbers with
> > > VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
> > > in the comment.
> > > Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
> > > virtio_pci etc." as well.
> >
> > Good idea! Thanks for the suggestion! I'll do it.
> >
> > Best regards,
> > Tiwei Bie
>
> Or just focus on the new feature, tweak start/end
> as a separate patch. Up to you, the important thing
> is to have a ready to roll patch before the merge
> window.
Got it! Thank you very much!
Best regards,
Tiwei Bie
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [virtio-dev] [PATCH] virtio_pci: support enabling VFs
@ 2018-06-01 1:36 ` Tiwei Bie
0 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-06-01 1:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Rustad, Mark D, Bjorn Helgaas, virtualization, LKML, virtio-dev,
linux-pci, Daly, Dan, Duyck, Alexander H, Liang, Cunming, Wang,
Zhihong
On Fri, Jun 01, 2018 at 04:21:03AM +0300, Michael S. Tsirkin wrote:
> On Thu, May 31, 2018 at 10:55:02AM +0800, Tiwei Bie wrote:
> > On Wed, May 30, 2018 at 07:09:37PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, May 30, 2018 at 04:03:37PM +0000, Rustad, Mark D wrote:
> > > > On May 30, 2018, at 1:55 AM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > > >
> > > > > There is a new feature bit allocated in virtio spec to
> > > > > support SR-IOV (Single Root I/O Virtualization):
> > > > >
> > > > > https://github.com/oasis-tcs/virtio-spec/issues/11
> > > > >
> > > > > This patch enables the support for this feature bit in
> > > > > virtio driver.
> > > > >
> > > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > > ---
> > > > > This patch depends on below proposal:
> > > > > https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
> > > > >
> > > > > This patch depends on below patch:
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
> > > > >
> > > > > drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
> > > > > drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
> > > > > include/uapi/linux/virtio_config.h | 7 ++++++-
> > > > > 3 files changed, 40 insertions(+), 1 deletion(-)
> > > > >
> > > >
> > > > <snip>
> > > >
> > > > > diff --git a/include/uapi/linux/virtio_config.h
> > > > > b/include/uapi/linux/virtio_config.h
> > > > > index 308e2096291f..b7c1f4e7d59e 100644
> > > > > --- a/include/uapi/linux/virtio_config.h
> > > > > +++ b/include/uapi/linux/virtio_config.h
> > > > > @@ -49,7 +49,7 @@
> > > >
> > > > There is a value in the comment directly before this that should
> > > > be updated as well to be consistent with the new value for
> > > > VIRTIO_TRANSPORT_F_END below.
> > >
> > > It hasn't been updated to 34 yet.
> > > I suggest a separate patch to replace the numbers with
> > > VIRTIO_TRANSPORT_F_START and VIRTIO_TRANSPORT_F_END
> > > in the comment.
> > > Maybe replace "e.g. virtio_ring" with "e.g. virtio_ring,
> > > virtio_pci etc." as well.
> >
> > Good idea! Thanks for the suggestion! I'll do it.
> >
> > Best regards,
> > Tiwei Bie
>
> Or just focus on the new feature, tweak start/end
> as a separate patch. Up to you, the important thing
> is to have a ready to roll patch before the merge
> window.
Got it! Thank you very much!
Best regards,
Tiwei Bie
---------------------------------------------------------------------
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] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-31 14:27 ` [virtio-dev] " Alexander Duyck
@ 2018-06-01 2:30 ` Tiwei Bie
-1 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-06-01 2:30 UTC (permalink / raw)
To: Alexander Duyck
Cc: Duyck, Alexander H, Rustad, Mark D, mst, linux-kernel,
virtualization, virtio-dev, Daly, Dan, linux-pci, bhelgaas,
Liang, Cunming, Wang, Zhihong
On Thu, May 31, 2018 at 07:27:43AM -0700, Alexander Duyck wrote:
> On Wed, May 30, 2018 at 8:20 PM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > On Thu, May 31, 2018 at 01:11:37AM +0800, Rustad, Mark D wrote:
> >> On May 30, 2018, at 9:54 AM, Duyck, Alexander H
> >> <alexander.h.duyck@intel.com> wrote:
> >>
> >> > On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
> >> > > On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > >
> >> > > > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
> >> > > > > num_vfs)
> >> > > > > +{
> >> > > > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> >> > > > > + struct virtio_device *vdev = &vp_dev->vdev;
> >> > > > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> >> > > > > +
> >> > > > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> >> > > > > + return -EBUSY;
> >> > > > > +
> >> > > > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> >> > > > > + return -EINVAL;
> >> > > > > +
> >> > > > > + sriov_configure = pci_sriov_configure_simple;
> >> > > > > + if (sriov_configure == NULL)
> >> > > > > + return -ENOENT;
> >> > > >
> >> > > > BTW what is all this trickery in aid of?
> >> > >
> >> > > When SR-IOV support is not compiled into the kernel,
> >> > > pci_sriov_configure_simple is #defined as NULL. This allows it to compile
> >> > > in that case, even though there is utterly no way for it to be called in
> >> > > that case. It is an alternative to #ifs in the code.
> >> >
> >> > Why even have the call though? I would wrap all of this in an #ifdef
> >> > and strip it out since you couldn't support SR-IOV if it isn't present
> >> > in the kernel anyway.
> >>
> >> I am inclined to agree. In this case, the presence of #ifdefs I think would
> >> be clearer. As written, someone will want to get rid of the pointer only to
> >> create a build problem when SR-IOV is not configured.
> >
> > In my opinion, maybe it would be better to make
> > pci_sriov_configure_simple() always available
> > just like other sriov functions.
> >
> > Based on the comments in the original patch:
> >
> > https://patchwork.kernel.org/patch/10353197/
> > """
> > +/* this is expected to be used as a function pointer, just define as NULL */
> > +#define pci_sriov_configure_simple NULL
> > """
> >
> > This function could be defined as NULL just because
> > it was expected to be used as a function pointer.
> > But actually it could be called directly as a
> > function, just like this case.
> >
> > So I prefer to make this function always available
> > just like other sriov functions.
> >
> > Best regards,
> > Tiwei Bie
>
> The fact that you are having to add additional code kind of implies
> that maybe this doesn't fall into the pci_sriov_configure_simple case
> anymore. The PF itself is defining what the VF can and can't do via
> the feature flags you are testing for.
I think you're right about pci_sriov_configure_simple
isn't designed for this case. I dropped the use of
pci_sriov_configure_simple in v2. Thanks!
>
> For example how is the bit of code below valid if the kernel itself
> doesn't support SR-IOV:
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
>
> It really seems like we should be wrapping these functions at the very
> minimum so that they don't imply you have SR-IOV support when it isn't
> supported in the kernel.
I think it's OK to accept this feature bit in this case.
The IOV support not enabled in kernel just means there is
no way for users to use SR-IOV. But it doesn't mean that
the virtio driver doesn't understand *this feature bit*.
Even if the IOV support is enabled in kernel, there is
still no way for virtio driver to know whether users will
enable VFs or not. Accepting this feature is to tell the
device the virtio driver understands this feature (i.e.
this is not an incompatible virtio driver). And it's not
to tell the device whether users will enable VFs or not,
or how many VFs will be enabled.
>
> Also it seems like we should be disabling the VFs if the driver is
> unbound from this interface. We need to add logic to disable SR-IOV if
> the driver is removed. What we don't want to do is leave VFs allocated
> and then have the potential for us to unbind/rebind the driver as the
> new driver may change the negotiated features.
Right. Thanks!
FYI, here is v2:
https://lists.oasis-open.org/archives/virtio-dev/201805/msg00206.html
Best regards,
Tiwei Bie
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH] virtio_pci: support enabling VFs
2018-05-31 14:27 ` [virtio-dev] " Alexander Duyck
(?)
@ 2018-06-01 2:30 ` Tiwei Bie
-1 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-06-01 2:30 UTC (permalink / raw)
To: Alexander Duyck
Cc: Duyck, Alexander H, virtio-dev, mst, linux-pci, linux-kernel,
virtualization, Wang, Zhihong, bhelgaas, Rustad, Mark D
On Thu, May 31, 2018 at 07:27:43AM -0700, Alexander Duyck wrote:
> On Wed, May 30, 2018 at 8:20 PM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > On Thu, May 31, 2018 at 01:11:37AM +0800, Rustad, Mark D wrote:
> >> On May 30, 2018, at 9:54 AM, Duyck, Alexander H
> >> <alexander.h.duyck@intel.com> wrote:
> >>
> >> > On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
> >> > > On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > >
> >> > > > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
> >> > > > > num_vfs)
> >> > > > > +{
> >> > > > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> >> > > > > + struct virtio_device *vdev = &vp_dev->vdev;
> >> > > > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> >> > > > > +
> >> > > > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> >> > > > > + return -EBUSY;
> >> > > > > +
> >> > > > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> >> > > > > + return -EINVAL;
> >> > > > > +
> >> > > > > + sriov_configure = pci_sriov_configure_simple;
> >> > > > > + if (sriov_configure == NULL)
> >> > > > > + return -ENOENT;
> >> > > >
> >> > > > BTW what is all this trickery in aid of?
> >> > >
> >> > > When SR-IOV support is not compiled into the kernel,
> >> > > pci_sriov_configure_simple is #defined as NULL. This allows it to compile
> >> > > in that case, even though there is utterly no way for it to be called in
> >> > > that case. It is an alternative to #ifs in the code.
> >> >
> >> > Why even have the call though? I would wrap all of this in an #ifdef
> >> > and strip it out since you couldn't support SR-IOV if it isn't present
> >> > in the kernel anyway.
> >>
> >> I am inclined to agree. In this case, the presence of #ifdefs I think would
> >> be clearer. As written, someone will want to get rid of the pointer only to
> >> create a build problem when SR-IOV is not configured.
> >
> > In my opinion, maybe it would be better to make
> > pci_sriov_configure_simple() always available
> > just like other sriov functions.
> >
> > Based on the comments in the original patch:
> >
> > https://patchwork.kernel.org/patch/10353197/
> > """
> > +/* this is expected to be used as a function pointer, just define as NULL */
> > +#define pci_sriov_configure_simple NULL
> > """
> >
> > This function could be defined as NULL just because
> > it was expected to be used as a function pointer.
> > But actually it could be called directly as a
> > function, just like this case.
> >
> > So I prefer to make this function always available
> > just like other sriov functions.
> >
> > Best regards,
> > Tiwei Bie
>
> The fact that you are having to add additional code kind of implies
> that maybe this doesn't fall into the pci_sriov_configure_simple case
> anymore. The PF itself is defining what the VF can and can't do via
> the feature flags you are testing for.
I think you're right about pci_sriov_configure_simple
isn't designed for this case. I dropped the use of
pci_sriov_configure_simple in v2. Thanks!
>
> For example how is the bit of code below valid if the kernel itself
> doesn't support SR-IOV:
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
>
> It really seems like we should be wrapping these functions at the very
> minimum so that they don't imply you have SR-IOV support when it isn't
> supported in the kernel.
I think it's OK to accept this feature bit in this case.
The IOV support not enabled in kernel just means there is
no way for users to use SR-IOV. But it doesn't mean that
the virtio driver doesn't understand *this feature bit*.
Even if the IOV support is enabled in kernel, there is
still no way for virtio driver to know whether users will
enable VFs or not. Accepting this feature is to tell the
device the virtio driver understands this feature (i.e.
this is not an incompatible virtio driver). And it's not
to tell the device whether users will enable VFs or not,
or how many VFs will be enabled.
>
> Also it seems like we should be disabling the VFs if the driver is
> unbound from this interface. We need to add logic to disable SR-IOV if
> the driver is removed. What we don't want to do is leave VFs allocated
> and then have the potential for us to unbind/rebind the driver as the
> new driver may change the negotiated features.
Right. Thanks!
FYI, here is v2:
https://lists.oasis-open.org/archives/virtio-dev/201805/msg00206.html
Best regards,
Tiwei Bie
^ permalink raw reply [flat|nested] 55+ messages in thread
* [virtio-dev] Re: [PATCH] virtio_pci: support enabling VFs
@ 2018-06-01 2:30 ` Tiwei Bie
0 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-06-01 2:30 UTC (permalink / raw)
To: Alexander Duyck
Cc: Duyck, Alexander H, Rustad, Mark D, mst, linux-kernel,
virtualization, virtio-dev, Daly, Dan, linux-pci, bhelgaas,
Liang, Cunming, Wang, Zhihong
On Thu, May 31, 2018 at 07:27:43AM -0700, Alexander Duyck wrote:
> On Wed, May 30, 2018 at 8:20 PM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> > On Thu, May 31, 2018 at 01:11:37AM +0800, Rustad, Mark D wrote:
> >> On May 30, 2018, at 9:54 AM, Duyck, Alexander H
> >> <alexander.h.duyck@intel.com> wrote:
> >>
> >> > On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:
> >> > > On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > >
> >> > > > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int
> >> > > > > num_vfs)
> >> > > > > +{
> >> > > > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
> >> > > > > + struct virtio_device *vdev = &vp_dev->vdev;
> >> > > > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
> >> > > > > +
> >> > > > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
> >> > > > > + return -EBUSY;
> >> > > > > +
> >> > > > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
> >> > > > > + return -EINVAL;
> >> > > > > +
> >> > > > > + sriov_configure = pci_sriov_configure_simple;
> >> > > > > + if (sriov_configure == NULL)
> >> > > > > + return -ENOENT;
> >> > > >
> >> > > > BTW what is all this trickery in aid of?
> >> > >
> >> > > When SR-IOV support is not compiled into the kernel,
> >> > > pci_sriov_configure_simple is #defined as NULL. This allows it to compile
> >> > > in that case, even though there is utterly no way for it to be called in
> >> > > that case. It is an alternative to #ifs in the code.
> >> >
> >> > Why even have the call though? I would wrap all of this in an #ifdef
> >> > and strip it out since you couldn't support SR-IOV if it isn't present
> >> > in the kernel anyway.
> >>
> >> I am inclined to agree. In this case, the presence of #ifdefs I think would
> >> be clearer. As written, someone will want to get rid of the pointer only to
> >> create a build problem when SR-IOV is not configured.
> >
> > In my opinion, maybe it would be better to make
> > pci_sriov_configure_simple() always available
> > just like other sriov functions.
> >
> > Based on the comments in the original patch:
> >
> > https://patchwork.kernel.org/patch/10353197/
> > """
> > +/* this is expected to be used as a function pointer, just define as NULL */
> > +#define pci_sriov_configure_simple NULL
> > """
> >
> > This function could be defined as NULL just because
> > it was expected to be used as a function pointer.
> > But actually it could be called directly as a
> > function, just like this case.
> >
> > So I prefer to make this function always available
> > just like other sriov functions.
> >
> > Best regards,
> > Tiwei Bie
>
> The fact that you are having to add additional code kind of implies
> that maybe this doesn't fall into the pci_sriov_configure_simple case
> anymore. The PF itself is defining what the VF can and can't do via
> the feature flags you are testing for.
I think you're right about pci_sriov_configure_simple
isn't designed for this case. I dropped the use of
pci_sriov_configure_simple in v2. Thanks!
>
> For example how is the bit of code below valid if the kernel itself
> doesn't support SR-IOV:
> +static void vp_transport_features(struct virtio_device *vdev, u64 features)
> +{
> + struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> + struct pci_dev *pci_dev = vp_dev->pci_dev;
> +
> + if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
> + pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
> + __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
> +}
> +
>
> It really seems like we should be wrapping these functions at the very
> minimum so that they don't imply you have SR-IOV support when it isn't
> supported in the kernel.
I think it's OK to accept this feature bit in this case.
The IOV support not enabled in kernel just means there is
no way for users to use SR-IOV. But it doesn't mean that
the virtio driver doesn't understand *this feature bit*.
Even if the IOV support is enabled in kernel, there is
still no way for virtio driver to know whether users will
enable VFs or not. Accepting this feature is to tell the
device the virtio driver understands this feature (i.e.
this is not an incompatible virtio driver). And it's not
to tell the device whether users will enable VFs or not,
or how many VFs will be enabled.
>
> Also it seems like we should be disabling the VFs if the driver is
> unbound from this interface. We need to add logic to disable SR-IOV if
> the driver is removed. What we don't want to do is leave VFs allocated
> and then have the potential for us to unbind/rebind the driver as the
> new driver may change the negotiated features.
Right. Thanks!
FYI, here is v2:
https://lists.oasis-open.org/archives/virtio-dev/201805/msg00206.html
Best regards,
Tiwei Bie
---------------------------------------------------------------------
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] 55+ messages in thread
* [PATCH] virtio_pci: support enabling VFs
@ 2018-05-30 8:55 Tiwei Bie
0 siblings, 0 replies; 55+ messages in thread
From: Tiwei Bie @ 2018-05-30 8:55 UTC (permalink / raw)
To: mst, bhelgaas, virtualization, linux-kernel, virtio-dev, linux-pci
Cc: alexander.h.duyck, mark.d.rustad, zhihong.wang
There is a new feature bit allocated in virtio spec to
support SR-IOV (Single Root I/O Virtualization):
https://github.com/oasis-tcs/virtio-spec/issues/11
This patch enables the support for this feature bit in
virtio driver.
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
This patch depends on below proposal:
https://lists.oasis-open.org/archives/virtio-dev/201805/msg00154.html
This patch depends on below patch:
https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/virtualization&id=8effc395c2097e258fcedfc02ed4a66d45fb4238
drivers/virtio/virtio_pci_common.c | 20 ++++++++++++++++++++
drivers/virtio/virtio_pci_modern.c | 14 ++++++++++++++
include/uapi/linux/virtio_config.h | 7 ++++++-
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 48d4d1cf1cb6..023da80a7a3e 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -588,6 +588,25 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
put_device(dev);
}
+static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int num_vfs)
+{
+ struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
+ struct virtio_device *vdev = &vp_dev->vdev;
+ int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs);
+
+ if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK))
+ return -EBUSY;
+
+ if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV))
+ return -EINVAL;
+
+ sriov_configure = pci_sriov_configure_simple;
+ if (sriov_configure == NULL)
+ return -ENOENT;
+
+ return sriov_configure(pci_dev, num_vfs);
+}
+
static struct pci_driver virtio_pci_driver = {
.name = "virtio-pci",
.id_table = virtio_pci_id_table,
@@ -596,6 +615,7 @@ static struct pci_driver virtio_pci_driver = {
#ifdef CONFIG_PM_SLEEP
.driver.pm = &virtio_pci_pm_ops,
#endif
+ .sriov_configure = virtio_pci_sriov_configure,
};
module_pci_driver(virtio_pci_driver);
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 2555d80f6eec..07571daccfec 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -153,14 +153,28 @@ static u64 vp_get_features(struct virtio_device *vdev)
return features;
}
+static void vp_transport_features(struct virtio_device *vdev, u64 features)
+{
+ struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ struct pci_dev *pci_dev = vp_dev->pci_dev;
+
+ if ((features & BIT_ULL(VIRTIO_F_SR_IOV)) &&
+ pci_find_ext_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV))
+ __virtio_set_bit(vdev, VIRTIO_F_SR_IOV);
+}
+
/* virtio config->finalize_features() implementation */
static int vp_finalize_features(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ u64 features = vdev->features;
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
+ /* Give virtio_pci a chance to accept features. */
+ vp_transport_features(vdev, features);
+
if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
dev_err(&vdev->dev, "virtio: device uses modern interface "
"but does not have VIRTIO_F_VERSION_1\n");
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 308e2096291f..b7c1f4e7d59e 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -49,7 +49,7 @@
* transport being used (eg. virtio_ring), the rest are per-device feature
* bits. */
#define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END 34
+#define VIRTIO_TRANSPORT_F_END 38
#ifndef VIRTIO_CONFIG_NO_LEGACY
/* Do we get callbacks when the ring is completely used, even if we've
@@ -71,4 +71,9 @@
* this is for compatibility with legacy systems.
*/
#define VIRTIO_F_IOMMU_PLATFORM 33
+
+/*
+ * Does the device support Single Root I/O Virtualization?
+ */
+#define VIRTIO_F_SR_IOV 37
#endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
--
2.17.0
^ permalink raw reply related [flat|nested] 55+ messages in thread
end of thread, other threads:[~2018-06-01 2:30 UTC | newest]
Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-30 8:55 [PATCH] virtio_pci: support enabling VFs Tiwei Bie
2018-05-30 8:55 ` [virtio-dev] " Tiwei Bie
2018-05-30 12:29 ` Stefan Hajnoczi
2018-05-30 12:29 ` Stefan Hajnoczi
2018-05-30 12:41 ` Michael S. Tsirkin
2018-05-30 12:41 ` [virtio-dev] " Michael S. Tsirkin
2018-05-30 12:41 ` Michael S. Tsirkin
2018-05-30 16:03 ` [virtio-dev] " Rustad, Mark D
2018-05-30 16:09 ` Michael S. Tsirkin
2018-05-30 16:09 ` Michael S. Tsirkin
2018-05-31 2:55 ` Tiwei Bie
2018-05-31 2:55 ` Tiwei Bie
2018-06-01 1:21 ` Michael S. Tsirkin
2018-06-01 1:21 ` Michael S. Tsirkin
2018-06-01 1:21 ` Michael S. Tsirkin
2018-06-01 1:36 ` Tiwei Bie
2018-06-01 1:36 ` Tiwei Bie
2018-06-01 1:36 ` Tiwei Bie
2018-05-31 2:55 ` Tiwei Bie
2018-05-30 16:09 ` Michael S. Tsirkin
2018-05-30 16:03 ` Rustad, Mark D
2018-05-30 16:10 ` Alexander Duyck
2018-05-30 16:10 ` Alexander Duyck
2018-05-30 16:22 ` Michael S. Tsirkin
2018-05-30 16:22 ` Michael S. Tsirkin
2018-05-30 16:22 ` Michael S. Tsirkin
2018-05-30 16:26 ` Duyck, Alexander H
2018-05-30 16:26 ` Duyck, Alexander H
2018-05-30 16:26 ` Duyck, Alexander H
2018-05-30 17:15 ` Michael S. Tsirkin
2018-05-30 17:15 ` Michael S. Tsirkin
2018-05-30 17:15 ` Michael S. Tsirkin
2018-05-30 16:10 ` Alexander Duyck
2018-05-30 16:22 ` Michael S. Tsirkin
2018-05-30 16:22 ` [virtio-dev] " Michael S. Tsirkin
2018-05-30 16:44 ` Rustad, Mark D
2018-05-30 16:44 ` [virtio-dev] " Rustad, Mark D
2018-05-30 16:44 ` Rustad, Mark D
2018-05-30 16:54 ` Duyck, Alexander H
2018-05-30 16:54 ` Duyck, Alexander H
2018-05-30 17:11 ` Rustad, Mark D
2018-05-30 17:11 ` [virtio-dev] " Rustad, Mark D
2018-05-30 17:11 ` Rustad, Mark D
2018-05-31 3:20 ` Tiwei Bie
2018-05-31 3:20 ` [virtio-dev] " Tiwei Bie
2018-05-31 14:27 ` Alexander Duyck
2018-05-31 14:27 ` Alexander Duyck
2018-05-31 14:27 ` [virtio-dev] " Alexander Duyck
2018-06-01 2:30 ` Tiwei Bie
2018-06-01 2:30 ` Tiwei Bie
2018-06-01 2:30 ` [virtio-dev] " Tiwei Bie
2018-05-31 3:20 ` Tiwei Bie
2018-05-30 16:54 ` Duyck, Alexander H
2018-05-30 16:22 ` Michael S. Tsirkin
-- strict thread matches above, loose matches on Subject: below --
2018-05-30 8:55 Tiwei Bie
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.