Re: [PATCH 1/1] virtio: Add support for the virtio suspend feature
On 4/17/2024 4:54 PM, David Stevens wrote: Add support for the VIRTIO_F_SUSPEND feature. When this feature is negotiated, power management can use it to suspend virtio devices instead of resorting to resetting the devices entirely. Signed-off-by: David Stevens --- drivers/virtio/virtio.c| 32 ++ drivers/virtio/virtio_pci_common.c | 29 +++ drivers/virtio/virtio_pci_modern.c | 19 ++ include/linux/virtio.h | 2 ++ include/uapi/linux/virtio_config.h | 10 +- 5 files changed, 74 insertions(+), 18 deletions(-) diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index f4080692b351..cd11495a5098 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only #include +#include #include #include #include @@ -580,6 +581,37 @@ int virtio_device_restore(struct virtio_device *dev) return ret; } EXPORT_SYMBOL_GPL(virtio_device_restore); + +static int virtio_device_set_suspend_bit(struct virtio_device *dev, bool enabled) +{ + u8 status, target; + + status = dev->config->get_status(dev); + if (enabled) + target = status | VIRTIO_CONFIG_S_SUSPEND; + else + target = status & ~VIRTIO_CONFIG_S_SUSPEND; + dev->config->set_status(dev, target); I think it is better to verify whether the device SUSPEND bit is already set or clear, we can just return if status == target. Thanks Zhu Lingshan + + while ((status = dev->config->get_status(dev)) != target) { + if (status & VIRTIO_CONFIG_S_NEEDS_RESET) + return -EIO; + mdelay(10); + } + return 0; +} + +int virtio_device_suspend(struct virtio_device *dev) +{ + return virtio_device_set_suspend_bit(dev, true); +} +EXPORT_SYMBOL_GPL(virtio_device_suspend); + +int virtio_device_resume(struct virtio_device *dev) +{ + return virtio_device_set_suspend_bit(dev, false); +} +EXPORT_SYMBOL_GPL(virtio_device_resume); #endif static int virtio_init(void) diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index b655fccaf773..4d542de05970 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -495,31 +495,26 @@ static int virtio_pci_restore(struct device *dev) return virtio_device_restore(&vp_dev->vdev); } -static bool vp_supports_pm_no_reset(struct device *dev) +static int virtio_pci_suspend(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - u16 pmcsr; - - if (!pci_dev->pm_cap) - return false; - - pci_read_config_word(pci_dev, pci_dev->pm_cap + PCI_PM_CTRL, &pmcsr); - if (PCI_POSSIBLE_ERROR(pmcsr)) { - dev_err(dev, "Unable to query pmcsr"); - return false; - } + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); - return pmcsr & PCI_PM_CTRL_NO_SOFT_RESET; -} + if (virtio_has_feature(&vp_dev->vdev, VIRTIO_F_SUSPEND)) + return virtio_device_suspend(&vp_dev->vdev); -static int virtio_pci_suspend(struct device *dev) -{ - return vp_supports_pm_no_reset(dev) ? 0 : virtio_pci_freeze(dev); + return virtio_pci_freeze(dev); } static int virtio_pci_resume(struct device *dev) { - return vp_supports_pm_no_reset(dev) ? 0 : virtio_pci_restore(dev); + struct pci_dev *pci_dev = to_pci_dev(dev); + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); + + if (virtio_has_feature(&vp_dev->vdev, VIRTIO_F_SUSPEND)) + return virtio_device_resume(&vp_dev->vdev); + + return virtio_pci_restore(dev); } static const struct dev_pm_ops virtio_pci_pm_ops = { diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index f62b530aa3b5..ac8734526b8d 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -209,6 +209,22 @@ static void vp_modern_avq_deactivate(struct virtio_device *vdev) __virtqueue_break(admin_vq->info.vq); } +static bool vp_supports_pm_no_reset(struct pci_dev *pci_dev) +{ + u16 pmcsr; + + if (!pci_dev->pm_cap) + return false; + + pci_read_config_word(pci_dev, pci_dev->pm_cap + PCI_PM_CTRL, &pmcsr); + if (PCI_POSSIBLE_ERROR(pmcsr)) { + dev_err(&pci_dev->dev, "Unable to query pmcsr"); + return false; + } + + return pmcsr & PCI_PM_CTRL_NO_SOFT_RESET; +} + static void vp_transport_features(struct virtio_device *vdev, u64 features) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); @@ -223,6 +239,9 @@ static void vp_transport_features(struct virtio_device *vdev, u
Re: [PATCH] vhost-vdpa: change ioctl # for VDPA_GET_VRING_SIZE
On 4/3/2024 5:21 AM, Michael S. Tsirkin wrote: VDPA_GET_VRING_SIZE by mistake uses the already occupied ioctl # 0x80 and we never noticed - it happens to work because the direction and size are different, but confuses tools such as perf which like to look at just the number, and breaks the extra robustness of the ioctl numbering macros. To fix, sort the entries and renumber the ioctl - not too late since it wasn't in any released kernels yet. Cc: Arnaldo Carvalho de Melo Reported-by: Namhyung Kim Fixes: 1496c47065f9 ("vhost-vdpa: uapi to support reporting per vq size") Cc: "Zhu Lingshan" Signed-off-by: Michael S. Tsirkin Reviewed-by: Zhu Lingshan Thanks for the fix and sorry for the mess, I should read the whole header file to check whether the number is available. --- Build tested only - userspace patches using this will have to adjust. I will merge this in a week or so unless I hear otherwise, and afterwards perf can update there header. include/uapi/linux/vhost.h | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h index bea697390613..b95dd84eef2d 100644 --- a/include/uapi/linux/vhost.h +++ b/include/uapi/linux/vhost.h @@ -179,12 +179,6 @@ /* Get the config size */ #define VHOST_VDPA_GET_CONFIG_SIZE_IOR(VHOST_VIRTIO, 0x79, __u32) -/* Get the count of all virtqueues */ -#define VHOST_VDPA_GET_VQS_COUNT _IOR(VHOST_VIRTIO, 0x80, __u32) - -/* Get the number of virtqueue groups. */ -#define VHOST_VDPA_GET_GROUP_NUM _IOR(VHOST_VIRTIO, 0x81, __u32) - /* Get the number of address spaces. */ #define VHOST_VDPA_GET_AS_NUM _IOR(VHOST_VIRTIO, 0x7A, unsigned int) @@ -228,10 +222,17 @@ #define VHOST_VDPA_GET_VRING_DESC_GROUP _IOWR(VHOST_VIRTIO, 0x7F, \ struct vhost_vring_state) + +/* Get the count of all virtqueues */ +#define VHOST_VDPA_GET_VQS_COUNT _IOR(VHOST_VIRTIO, 0x80, __u32) + +/* Get the number of virtqueue groups. */ +#define VHOST_VDPA_GET_GROUP_NUM _IOR(VHOST_VIRTIO, 0x81, __u32) + /* Get the queue size of a specific virtqueue. * userspace set the vring index in vhost_vring_state.index * kernel set the queue size in vhost_vring_state.num */ -#define VHOST_VDPA_GET_VRING_SIZE _IOWR(VHOST_VIRTIO, 0x80, \ +#define VHOST_VDPA_GET_VRING_SIZE _IOWR(VHOST_VIRTIO, 0x82, \ struct vhost_vring_state) #endif
[PATCH V4 3/3] vDPA/ifcvf: get_config_size should return dev specific config size
get_config_size() should return the size based on the decected device type. Signed-off-by: Zhu Lingshan Reviewed-by: Stefano Garzarella Acked-by: Jason Wang --- drivers/vdpa/ifcvf/ifcvf_main.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 9a4a6df91f08..e48e6b74fe2e 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -356,7 +356,24 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) { - return sizeof(struct virtio_net_config); + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + size_t size; + + switch (vf->dev_type) { + case VIRTIO_ID_NET: + size = sizeof(struct virtio_net_config); + break; + case VIRTIO_ID_BLOCK: + size = sizeof(struct virtio_blk_config); + break; + default: + size = 0; + IFCVF_ERR(pdev, "VIRTIO ID %u not supported\n", vf->dev_type); + } + + return size; } static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev, -- 2.27.0
[PATCH V4 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA
This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block for vDPA. Signed-off-by: Zhu Lingshan Reviewed-by: Stefano Garzarella Acked-by: Jason Wang --- drivers/vdpa/ifcvf/ifcvf_base.h | 8 +++- drivers/vdpa/ifcvf/ifcvf_main.c | 19 ++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 1c04cd256fa7..0111bfdeb342 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -28,7 +29,12 @@ #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 -#define IFCVF_SUPPORTED_FEATURES \ +#define C5000X_PL_BLK_VENDOR_ID0x1AF4 +#define C5000X_PL_BLK_DEVICE_ID0x1001 +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 + +#define IFCVF_NET_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ (1ULL << VIRTIO_F_VERSION_1) | \ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 66927ec81fa5..9a4a6df91f08 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -168,10 +168,23 @@ static struct ifcvf_hw *vdpa_to_vf(struct vdpa_device *vdpa_dev) static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) { + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + u64 features; - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; + switch (vf->dev_type) { + case VIRTIO_ID_NET: + features = ifcvf_get_features(vf) & IFCVF_NET_SUPPORTED_FEATURES; + break; + case VIRTIO_ID_BLOCK: + features = ifcvf_get_features(vf); + break; + default: + features = 0; + IFCVF_ERR(pdev, "VIRTIO ID %u not supported\n", vf->dev_type); + } return features; } @@ -514,6 +527,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, C5000X_PL_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, +C5000X_PL_BLK_DEVICE_ID, +C5000X_PL_BLK_SUBSYS_VENDOR_ID, +C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, { 0 }, }; -- 2.27.0
[PATCH V4 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe
This commit deduces VIRTIO device ID as device type when probe, then ifcvf_vdpa_get_device_id() can simply return the ID. ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() can work properly based on the device ID. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 27 +++ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index b2eeb16b9c2c..1c04cd256fa7 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -84,6 +84,7 @@ struct ifcvf_hw { u32 notify_off_multiplier; u64 req_features; u64 hw_features; + u32 dev_type; struct virtio_pci_common_cfg __iomem *common_cfg; void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 44d7586019da..66927ec81fa5 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) { - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); - struct pci_dev *pdev = adapter->pdev; - u32 ret = -ENODEV; - - if (pdev->device < 0x1000 || pdev->device > 0x107f) - return ret; - - if (pdev->device < 0x1040) - ret = pdev->subsystem_device; - else - ret = pdev->device - 0x1040; + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); - return ret; + return vf->dev_type; } static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) @@ -466,6 +456,19 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) pci_set_drvdata(pdev, adapter); vf = &adapter->vf; + + /* This drirver drives both modern virtio devices and transitional +* devices in modern mode. +* vDPA requires feature bit VIRTIO_F_ACCESS_PLATFORM, +* so legacy devices and transitional devices in legacy +* mode will not work for vDPA, this driver will not +* drive devices with legacy interface. +*/ + if (pdev->device < 0x1040) + vf->dev_type = pdev->subsystem_device; + else + vf->dev_type = pdev->device - 0x1040; + vf->base = pcim_iomap_table(pdev); adapter->pdev = pdev; -- 2.27.0
[PATCH V4 0/3] vDPA/ifcvf: enables Intel C5000X-PL virtio-blk
This series enabled Intel FGPA SmartNIC C5000X-PL virtio-blk for vDPA. This series requires: Stefano's vdpa block patchset: https://lkml.org/lkml/2021/3/15/2113 my patchset to enable Intel FGPA SmartNIC C5000X-PL virtio-net for vDPA: https://lkml.org/lkml/2021/3/17/432 changes from V3: remove (pdev->device < 0x1000 || pdev->device > 0x107f) checks in probe(), because id_table already cut them off(Jason) changes from V2: both get_features() and get_config_size() use switch code block now(Stefano) changes from V1: (1)add comments to explain this driver drives virtio modern devices and transitional devices in modern mode.(Jason) (2)remove IFCVF_BLK_SUPPORTED_FEATURES, use hardware feature bits directly(Jason) (3)add error handling and message in get_config_size(Stefano) Thanks! Zhu Lingshan (3): vDPA/ifcvf: deduce VIRTIO device ID when probe vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA vDPA/ifcvf: get_config_size should return dev specific config size drivers/vdpa/ifcvf/ifcvf_base.h | 9 - drivers/vdpa/ifcvf/ifcvf_main.c | 65 ++--- 2 files changed, 59 insertions(+), 15 deletions(-) -- 2.27.0
[PATCH V3 3/3] vDPA/ifcvf: get_config_size should return dev specific config size
get_config_size() should return the size based on the decected device type. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 376b2014916a..3b6f7862dbb8 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -356,7 +356,24 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) { - return sizeof(struct virtio_net_config); + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + size_t size; + + switch (vf->dev_type) { + case VIRTIO_ID_NET: + size = sizeof(struct virtio_net_config); + break; + case VIRTIO_ID_BLOCK: + size = sizeof(struct virtio_blk_config); + break; + default: + size = 0; + IFCVF_ERR(pdev, "VIRTIO ID %u not supported\n", vf->dev_type); + } + + return size; } static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev, -- 2.27.0
[PATCH V3 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe
This commit deduces VIRTIO device ID as device type when probe, then ifcvf_vdpa_get_device_id() can simply return the ID. ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() can work properly based on the device ID. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 30 ++ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index b2eeb16b9c2c..1c04cd256fa7 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -84,6 +84,7 @@ struct ifcvf_hw { u32 notify_off_multiplier; u64 req_features; u64 hw_features; + u32 dev_type; struct virtio_pci_common_cfg __iomem *common_cfg; void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 44d7586019da..469a9b5737b7 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) { - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); - struct pci_dev *pdev = adapter->pdev; - u32 ret = -ENODEV; - - if (pdev->device < 0x1000 || pdev->device > 0x107f) - return ret; - - if (pdev->device < 0x1040) - ret = pdev->subsystem_device; - else - ret = pdev->device - 0x1040; + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); - return ret; + return vf->dev_type; } static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) @@ -466,6 +456,22 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) pci_set_drvdata(pdev, adapter); vf = &adapter->vf; + + /* This drirver drives both modern virtio devices and transitional +* devices in modern mode. +* vDPA requires feature bit VIRTIO_F_ACCESS_PLATFORM, +* so legacy devices and transitional devices in legacy +* mode will not work for vDPA, this driver will not +* drive devices with legacy interface. +*/ + if (pdev->device < 0x1000 || pdev->device > 0x107f) + return -EOPNOTSUPP; + + if (pdev->device < 0x1040) + vf->dev_type = pdev->subsystem_device; + else + vf->dev_type = pdev->device - 0x1040; + vf->base = pcim_iomap_table(pdev); adapter->pdev = pdev; -- 2.27.0
[PATCH V3 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA
This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block for vDPA. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 8 +++- drivers/vdpa/ifcvf/ifcvf_main.c | 19 ++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 1c04cd256fa7..0111bfdeb342 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -28,7 +29,12 @@ #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 -#define IFCVF_SUPPORTED_FEATURES \ +#define C5000X_PL_BLK_VENDOR_ID0x1AF4 +#define C5000X_PL_BLK_DEVICE_ID0x1001 +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 + +#define IFCVF_NET_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ (1ULL << VIRTIO_F_VERSION_1) | \ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 469a9b5737b7..376b2014916a 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -168,10 +168,23 @@ static struct ifcvf_hw *vdpa_to_vf(struct vdpa_device *vdpa_dev) static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) { + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + u64 features; - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; + switch (vf->dev_type) { + case VIRTIO_ID_NET: + features = ifcvf_get_features(vf) & IFCVF_NET_SUPPORTED_FEATURES; + break; + case VIRTIO_ID_BLOCK: + features = ifcvf_get_features(vf); + break; + default: + features = 0; + IFCVF_ERR(pdev, "VIRTIO ID %u not supported\n", vf->dev_type); + } return features; } @@ -517,6 +530,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, C5000X_PL_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, +C5000X_PL_BLK_DEVICE_ID, +C5000X_PL_BLK_SUBSYS_VENDOR_ID, +C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, { 0 }, }; -- 2.27.0
[PATCH V3 0/3] vDPA/ifcvf: enables Intel C5000X-PL virtio-blk
This series enabled Intel FGPA SmartNIC C5000X-PL virtio-blk for vDPA. This series requires: Stefano's vdpa block patchset: https://lkml.org/lkml/2021/3/15/2113 my patchset to enable Intel FGPA SmartNIC C5000X-PL virtio-net for vDPA: https://lkml.org/lkml/2021/3/17/432 changes from V2: both get_features() and get_config_size() use switch code block now(Stefano) changes from V1: (1)add comments to explain this driver drives virtio modern devices and transitional devices in modern mode.(Jason) (2)remove IFCVF_BLK_SUPPORTED_FEATURES, use hardware feature bits directly(Jason) (3)add error handling and message in get_config_size(Stefano) Thanks! Zhu Lingshan (3): vDPA/ifcvf: deduce VIRTIO device ID when probe vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA vDPA/ifcvf: get_config_size should return dev specific config size drivers/vdpa/ifcvf/ifcvf_base.h | 9 - drivers/vdpa/ifcvf/ifcvf_main.c | 68 ++--- 2 files changed, 62 insertions(+), 15 deletions(-) -- 2.27.0
Re: [PATCH V2 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA
On 4/15/2021 9:41 PM, Stefano Garzarella wrote: On Thu, Apr 15, 2021 at 05:53:35PM +0800, Zhu Lingshan wrote: This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block for vDPA. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 8 +++- drivers/vdpa/ifcvf/ifcvf_main.c | 10 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 1c04cd256fa7..0111bfdeb342 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -28,7 +29,12 @@ #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 -#define IFCVF_SUPPORTED_FEATURES \ +#define C5000X_PL_BLK_VENDOR_ID 0x1AF4 +#define C5000X_PL_BLK_DEVICE_ID 0x1001 +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 + +#define IFCVF_NET_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ (1ULL << VIRTIO_F_VERSION_1) | \ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 469a9b5737b7..cea1313b1a3f 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -171,7 +171,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); u64 features; - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; + if (vf->dev_type == VIRTIO_ID_NET) + features = ifcvf_get_features(vf) & IFCVF_NET_SUPPORTED_FEATURES; + + if (vf->dev_type == VIRTIO_ID_BLOCK) + features = ifcvf_get_features(vf); Should we put a warning here too otherwise feature could be seen unassigned? Thanks, it will be a switch code block too. Thanks, Stefano return features; } @@ -517,6 +521,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, C5000X_PL_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, + C5000X_PL_BLK_DEVICE_ID, + C5000X_PL_BLK_SUBSYS_VENDOR_ID, + C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, { 0 }, }; -- 2.27.0
Re: [PATCH V2 3/3] vDPA/ifcvf: get_config_size should return dev specific config size
On 4/15/2021 9:48 PM, Stefano Garzarella wrote: On Thu, Apr 15, 2021 at 05:53:36PM +0800, Zhu Lingshan wrote: get_config_size() should return the size based on the decected device type. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index cea1313b1a3f..6844c49fe1de 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -347,7 +347,23 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) { - return sizeof(struct virtio_net_config); + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + size_t size; + + if (vf->dev_type == VIRTIO_ID_NET) + size = sizeof(struct virtio_net_config); + + else if (vf->dev_type == VIRTIO_ID_BLOCK) + size = sizeof(struct virtio_blk_config); + + else { + size = 0; + IFCVF_ERR(pdev, "VIRTIO ID %u not supported\n", vf->dev_type); + } I slightly prefer the switch, but I don't have a strong opinion. However, if we want to use if/else, we should follow `Documentation/process/coding-style.rst` line 166: Note that the closing brace is empty on a line of its own, **except** in the cases where it is followed by a continuation of the same statement, ie a ``while`` in a do-statement or an ``else`` in an if-statement, like also `scripts/checkpatch.pl --strict` complains: CHECK: braces {} should be used on all arms of this statement #209: FILE: drivers/vdpa/ifcvf/ifcvf_main.c:355: + if (vf->dev_type == VIRTIO_ID_NET) [...] + else if (vf->dev_type == VIRTIO_ID_BLOCK) [...] + else { [...] CHECK: Unbalanced braces around else statement #215: FILE: drivers/vdpa/ifcvf/ifcvf_main.c:361: + else { Thanks Stefano, the reason is we only have one line code after if, so looks like {} is unnecessary, I agree switch can clear up code style confusions. I will add this in v3. Thanks! Thanks, Stefano ___ Virtualization mailing list virtualizat...@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH V2 3/3] vDPA/ifcvf: get_config_size should return dev specific config size
get_config_size() should return the size based on the decected device type. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index cea1313b1a3f..6844c49fe1de 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -347,7 +347,23 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) { - return sizeof(struct virtio_net_config); + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + size_t size; + + if (vf->dev_type == VIRTIO_ID_NET) + size = sizeof(struct virtio_net_config); + + else if (vf->dev_type == VIRTIO_ID_BLOCK) + size = sizeof(struct virtio_blk_config); + + else { + size = 0; + IFCVF_ERR(pdev, "VIRTIO ID %u not supported\n", vf->dev_type); + } + + return size; } static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev, -- 2.27.0
[PATCH V2 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe
This commit deduces VIRTIO device ID as device type when probe, then ifcvf_vdpa_get_device_id() can simply return the ID. ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() can work properly based on the device ID. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 30 ++ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index b2eeb16b9c2c..1c04cd256fa7 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -84,6 +84,7 @@ struct ifcvf_hw { u32 notify_off_multiplier; u64 req_features; u64 hw_features; + u32 dev_type; struct virtio_pci_common_cfg __iomem *common_cfg; void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 44d7586019da..469a9b5737b7 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) { - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); - struct pci_dev *pdev = adapter->pdev; - u32 ret = -ENODEV; - - if (pdev->device < 0x1000 || pdev->device > 0x107f) - return ret; - - if (pdev->device < 0x1040) - ret = pdev->subsystem_device; - else - ret = pdev->device - 0x1040; + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); - return ret; + return vf->dev_type; } static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) @@ -466,6 +456,22 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) pci_set_drvdata(pdev, adapter); vf = &adapter->vf; + + /* This drirver drives both modern virtio devices and transitional +* devices in modern mode. +* vDPA requires feature bit VIRTIO_F_ACCESS_PLATFORM, +* so legacy devices and transitional devices in legacy +* mode will not work for vDPA, this driver will not +* drive devices with legacy interface. +*/ + if (pdev->device < 0x1000 || pdev->device > 0x107f) + return -EOPNOTSUPP; + + if (pdev->device < 0x1040) + vf->dev_type = pdev->subsystem_device; + else + vf->dev_type = pdev->device - 0x1040; + vf->base = pcim_iomap_table(pdev); adapter->pdev = pdev; -- 2.27.0
[PATCH V2 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA
This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block for vDPA. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 8 +++- drivers/vdpa/ifcvf/ifcvf_main.c | 10 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 1c04cd256fa7..0111bfdeb342 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -28,7 +29,12 @@ #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 -#define IFCVF_SUPPORTED_FEATURES \ +#define C5000X_PL_BLK_VENDOR_ID0x1AF4 +#define C5000X_PL_BLK_DEVICE_ID0x1001 +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 + +#define IFCVF_NET_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ (1ULL << VIRTIO_F_VERSION_1) | \ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 469a9b5737b7..cea1313b1a3f 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -171,7 +171,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); u64 features; - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; + if (vf->dev_type == VIRTIO_ID_NET) + features = ifcvf_get_features(vf) & IFCVF_NET_SUPPORTED_FEATURES; + + if (vf->dev_type == VIRTIO_ID_BLOCK) + features = ifcvf_get_features(vf); return features; } @@ -517,6 +521,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, C5000X_PL_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, +C5000X_PL_BLK_DEVICE_ID, +C5000X_PL_BLK_SUBSYS_VENDOR_ID, +C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, { 0 }, }; -- 2.27.0
[PATCH V2 0/3] vDPA/ifcvf: enables Intel C5000X-PL virtio-blk
This series enabled Intel FGPA SmartNIC C5000X-PL virtio-blk for vDPA. This series requires: Stefano's vdpa block patchset: https://lkml.org/lkml/2021/3/15/2113 my patchset to enable Intel FGPA SmartNIC C5000X-PL virtio-net for vDPA: https://lkml.org/lkml/2021/3/17/432 changes from V1: (1)add comments to explain this driver drives virtio modern devices and transitional devices in modern mode.(Jason) (2)remove IFCVF_BLK_SUPPORTED_FEATURES, use hardware feature bits directly(Jason) (3)add error handling and message in get_config_size(Stefano) Thanks! Zhu Lingshan (3): vDPA/ifcvf: deduce VIRTIO device ID when probe vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA vDPA/ifcvf: get_config_size should return dev specific config size drivers/vdpa/ifcvf/ifcvf_base.h | 9 - drivers/vdpa/ifcvf/ifcvf_main.c | 58 + 2 files changed, 52 insertions(+), 15 deletions(-) -- 2.27.0
Re: [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA
On 4/15/2021 3:17 PM, Jason Wang wrote: 在 2021/4/15 下午2:41, Zhu Lingshan 写道: I think we've discussed this sometime in the past but what's the reason for such whitelist consider there's already a get_features() implemention? E.g Any reason to block VIRTIO_BLK_F_WRITE_ZEROS or VIRTIO_F_RING_PACKED? Thanks The reason is some feature bits are supported in the device but not supported by the driver, e.g, for virtio-net, mq & cq implementation is not ready in the driver. I understand the case of virtio-net but I wonder why we need this for block where we don't vq cvq. Thanks This is still a subset of the feature bits read from hardware, I leave it here to code consistently, and indicate what we support clearly. Are you suggesting remove this feature bits list and just use what we read from hardware? Thansk Yes, please do that. The whiltelist doesn't help in this case I think. OK, will remove this in V2 Thanks Thanks
Re: [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe
On 4/15/2021 3:16 PM, Jason Wang wrote: 在 2021/4/15 下午2:36, Zhu Lingshan 写道: On 4/15/2021 2:30 PM, Jason Wang wrote: 在 2021/4/15 下午1:52, Zhu Lingshan 写道: On 4/15/2021 11:30 AM, Jason Wang wrote: 在 2021/4/14 下午5:18, Zhu Lingshan 写道: This commit deduces VIRTIO device ID as device type when probe, then ifcvf_vdpa_get_device_id() can simply return the ID. ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() can work properly based on the device ID. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index b2eeb16b9c2c..1c04cd256fa7 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -84,6 +84,7 @@ struct ifcvf_hw { u32 notify_off_multiplier; u64 req_features; u64 hw_features; + u32 dev_type; struct virtio_pci_common_cfg __iomem *common_cfg; void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 44d7586019da..99b0a6b4c227 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) { - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); - struct pci_dev *pdev = adapter->pdev; - u32 ret = -ENODEV; - - if (pdev->device < 0x1000 || pdev->device > 0x107f) - return ret; - - if (pdev->device < 0x1040) - ret = pdev->subsystem_device; - else - ret = pdev->device-0x1040; + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); - return ret; + return vf->dev_type; } static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) pci_set_drvdata(pdev, adapter); vf = &adapter->vf; + if (pdev->device < 0x1000 || pdev->device > 0x107f) + return -EOPNOTSUPP; + + if (pdev->device < 0x1040) + vf->dev_type = pdev->subsystem_device; + else + vf->dev_type = pdev->device - 0x1040; So a question here, is the device a transtional device or modern one? If it's a transitonal one, can it swtich endianess automatically or not? Thanks Hi Jason, This driver should drive both modern and transitional devices as we discussed before. If it's a transitional one, it will act as a modern device by default, legacy mode is a fail-over path. Note that legacy driver use native endian, support legacy driver requires the device to know native endian which I'm not sure your device can do that. Thanks Yes, legacy requires guest native endianess, I think we don't need to worry about this because our transitional device should work in modern mode by default(legacy mode is the failover path we will never reach, get_features will fail if no ACCESS_PLATFORM), we don't support legacy device in vDPA. Thanks Ok, so I think it's better to add a comment here. sure, will add a comment in V2 Thanks Thanks For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it must in modern mode. I think we don't need to worry about endianess for legacy mode. Thanks Zhu Lingshan + vf->base = pcim_iomap_table(pdev); adapter->pdev = pdev;
Re: [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA
On 4/15/2021 2:31 PM, Jason Wang wrote: 在 2021/4/15 下午1:55, Zhu Lingshan 写道: On 4/15/2021 11:34 AM, Jason Wang wrote: 在 2021/4/14 下午5:18, Zhu Lingshan 写道: This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block for vDPA. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 17 - drivers/vdpa/ifcvf/ifcvf_main.c | 10 +- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 1c04cd256fa7..8b403522bf06 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -28,7 +29,12 @@ #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 -#define IFCVF_SUPPORTED_FEATURES \ +#define C5000X_PL_BLK_VENDOR_ID 0x1AF4 +#define C5000X_PL_BLK_DEVICE_ID 0x1001 +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 + +#define IFCVF_NET_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ (1ULL << VIRTIO_F_VERSION_1) | \ @@ -37,6 +43,15 @@ (1ULL << VIRTIO_F_ACCESS_PLATFORM) | \ (1ULL << VIRTIO_NET_F_MRG_RXBUF)) +#define IFCVF_BLK_SUPPORTED_FEATURES \ + ((1ULL << VIRTIO_BLK_F_SIZE_MAX) | \ + (1ULL << VIRTIO_BLK_F_SEG_MAX) | \ + (1ULL << VIRTIO_BLK_F_BLK_SIZE) | \ + (1ULL << VIRTIO_BLK_F_TOPOLOGY) | \ + (1ULL << VIRTIO_BLK_F_MQ) | \ + (1ULL << VIRTIO_F_VERSION_1) | \ + (1ULL << VIRTIO_F_ACCESS_PLATFORM)) I think we've discussed this sometime in the past but what's the reason for such whitelist consider there's already a get_features() implemention? E.g Any reason to block VIRTIO_BLK_F_WRITE_ZEROS or VIRTIO_F_RING_PACKED? Thanks The reason is some feature bits are supported in the device but not supported by the driver, e.g, for virtio-net, mq & cq implementation is not ready in the driver. I understand the case of virtio-net but I wonder why we need this for block where we don't vq cvq. Thanks This is still a subset of the feature bits read from hardware, I leave it here to code consistently, and indicate what we support clearly. Are you suggesting remove this feature bits list and just use what we read from hardware? Thansk Thanks! + /* Only one queue pair for now. */ #define IFCVF_MAX_QUEUE_PAIRS 1 diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 99b0a6b4c227..9b6a38b798fa 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -171,7 +171,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); u64 features; - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; + if (vf->dev_type == VIRTIO_ID_NET) + features = ifcvf_get_features(vf) & IFCVF_NET_SUPPORTED_FEATURES; + + if (vf->dev_type == VIRTIO_ID_BLOCK) + features = ifcvf_get_features(vf) & IFCVF_BLK_SUPPORTED_FEATURES; return features; } @@ -509,6 +513,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, C5000X_PL_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, + C5000X_PL_BLK_DEVICE_ID, + C5000X_PL_BLK_SUBSYS_VENDOR_ID, + C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, { 0 }, };
Re: [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe
On 4/15/2021 2:30 PM, Jason Wang wrote: 在 2021/4/15 下午1:52, Zhu Lingshan 写道: On 4/15/2021 11:30 AM, Jason Wang wrote: 在 2021/4/14 下午5:18, Zhu Lingshan 写道: This commit deduces VIRTIO device ID as device type when probe, then ifcvf_vdpa_get_device_id() can simply return the ID. ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() can work properly based on the device ID. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index b2eeb16b9c2c..1c04cd256fa7 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -84,6 +84,7 @@ struct ifcvf_hw { u32 notify_off_multiplier; u64 req_features; u64 hw_features; + u32 dev_type; struct virtio_pci_common_cfg __iomem *common_cfg; void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 44d7586019da..99b0a6b4c227 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) { - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); - struct pci_dev *pdev = adapter->pdev; - u32 ret = -ENODEV; - - if (pdev->device < 0x1000 || pdev->device > 0x107f) - return ret; - - if (pdev->device < 0x1040) - ret = pdev->subsystem_device; - else - ret = pdev->device -0x1040; + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); - return ret; + return vf->dev_type; } static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) pci_set_drvdata(pdev, adapter); vf = &adapter->vf; + if (pdev->device < 0x1000 || pdev->device > 0x107f) + return -EOPNOTSUPP; + + if (pdev->device < 0x1040) + vf->dev_type = pdev->subsystem_device; + else + vf->dev_type = pdev->device - 0x1040; So a question here, is the device a transtional device or modern one? If it's a transitonal one, can it swtich endianess automatically or not? Thanks Hi Jason, This driver should drive both modern and transitional devices as we discussed before. If it's a transitional one, it will act as a modern device by default, legacy mode is a fail-over path. Note that legacy driver use native endian, support legacy driver requires the device to know native endian which I'm not sure your device can do that. Thanks Yes, legacy requires guest native endianess, I think we don't need to worry about this because our transitional device should work in modern mode by default(legacy mode is the failover path we will never reach, get_features will fail if no ACCESS_PLATFORM), we don't support legacy device in vDPA. Thanks For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it must in modern mode. I think we don't need to worry about endianess for legacy mode. Thanks Zhu Lingshan + vf->base = pcim_iomap_table(pdev); adapter->pdev = pdev;
Re: [PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA
On 4/15/2021 11:34 AM, Jason Wang wrote: 在 2021/4/14 下午5:18, Zhu Lingshan 写道: This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block for vDPA. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 17 - drivers/vdpa/ifcvf/ifcvf_main.c | 10 +- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 1c04cd256fa7..8b403522bf06 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -28,7 +29,12 @@ #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 -#define IFCVF_SUPPORTED_FEATURES \ +#define C5000X_PL_BLK_VENDOR_ID 0x1AF4 +#define C5000X_PL_BLK_DEVICE_ID 0x1001 +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 + +#define IFCVF_NET_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ (1ULL << VIRTIO_F_VERSION_1) | \ @@ -37,6 +43,15 @@ (1ULL << VIRTIO_F_ACCESS_PLATFORM) | \ (1ULL << VIRTIO_NET_F_MRG_RXBUF)) +#define IFCVF_BLK_SUPPORTED_FEATURES \ + ((1ULL << VIRTIO_BLK_F_SIZE_MAX) | \ + (1ULL << VIRTIO_BLK_F_SEG_MAX) | \ + (1ULL << VIRTIO_BLK_F_BLK_SIZE) | \ + (1ULL << VIRTIO_BLK_F_TOPOLOGY) | \ + (1ULL << VIRTIO_BLK_F_MQ) | \ + (1ULL << VIRTIO_F_VERSION_1) | \ + (1ULL << VIRTIO_F_ACCESS_PLATFORM)) I think we've discussed this sometime in the past but what's the reason for such whitelist consider there's already a get_features() implemention? E.g Any reason to block VIRTIO_BLK_F_WRITE_ZEROS or VIRTIO_F_RING_PACKED? Thanks The reason is some feature bits are supported in the device but not supported by the driver, e.g, for virtio-net, mq & cq implementation is not ready in the driver. Thanks! + /* Only one queue pair for now. */ #define IFCVF_MAX_QUEUE_PAIRS 1 diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 99b0a6b4c227..9b6a38b798fa 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -171,7 +171,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); u64 features; - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; + if (vf->dev_type == VIRTIO_ID_NET) + features = ifcvf_get_features(vf) & IFCVF_NET_SUPPORTED_FEATURES; + + if (vf->dev_type == VIRTIO_ID_BLOCK) + features = ifcvf_get_features(vf) & IFCVF_BLK_SUPPORTED_FEATURES; return features; } @@ -509,6 +513,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, C5000X_PL_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, + C5000X_PL_BLK_DEVICE_ID, + C5000X_PL_BLK_SUBSYS_VENDOR_ID, + C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, { 0 }, };
Re: [PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe
On 4/15/2021 11:30 AM, Jason Wang wrote: 在 2021/4/14 下午5:18, Zhu Lingshan 写道: This commit deduces VIRTIO device ID as device type when probe, then ifcvf_vdpa_get_device_id() can simply return the ID. ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() can work properly based on the device ID. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index b2eeb16b9c2c..1c04cd256fa7 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -84,6 +84,7 @@ struct ifcvf_hw { u32 notify_off_multiplier; u64 req_features; u64 hw_features; + u32 dev_type; struct virtio_pci_common_cfg __iomem *common_cfg; void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 44d7586019da..99b0a6b4c227 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) { - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); - struct pci_dev *pdev = adapter->pdev; - u32 ret = -ENODEV; - - if (pdev->device < 0x1000 || pdev->device > 0x107f) - return ret; - - if (pdev->device < 0x1040) - ret = pdev->subsystem_device; - else - ret = pdev->device - 0x1040; + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); - return ret; + return vf->dev_type; } static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) pci_set_drvdata(pdev, adapter); vf = &adapter->vf; + if (pdev->device < 0x1000 || pdev->device > 0x107f) + return -EOPNOTSUPP; + + if (pdev->device < 0x1040) + vf->dev_type = pdev->subsystem_device; + else + vf->dev_type = pdev->device - 0x1040; So a question here, is the device a transtional device or modern one? If it's a transitonal one, can it swtich endianess automatically or not? Thanks Hi Jason, This driver should drive both modern and transitional devices as we discussed before. If it's a transitional one, it will act as a modern device by default, legacy mode is a fail-over path. For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it must in modern mode. I think we don't need to worry about endianess for legacy mode. Thanks Zhu Lingshan + vf->base = pcim_iomap_table(pdev); adapter->pdev = pdev;
[PATCH 3/3] vDPA/ifcvf: get_config_size should return dev specific config size
get_config_size() should return the size based on the decected device type. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 9b6a38b798fa..b48b9789b69e 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -347,7 +347,16 @@ static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) static size_t ifcvf_vdpa_get_config_size(struct vdpa_device *vdpa_dev) { - return sizeof(struct virtio_net_config); + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + size_t size; + + if (vf->dev_type == VIRTIO_ID_NET) + size = sizeof(struct virtio_net_config); + + if (vf->dev_type == VIRTIO_ID_BLOCK) + size = sizeof(struct virtio_blk_config); + + return size; } static void ifcvf_vdpa_get_config(struct vdpa_device *vdpa_dev, -- 2.27.0
[PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA
This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block for vDPA. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 17 - drivers/vdpa/ifcvf/ifcvf_main.c | 10 +- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 1c04cd256fa7..8b403522bf06 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -28,7 +29,12 @@ #define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 #define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 -#define IFCVF_SUPPORTED_FEATURES \ +#define C5000X_PL_BLK_VENDOR_ID0x1AF4 +#define C5000X_PL_BLK_DEVICE_ID0x1001 +#define C5000X_PL_BLK_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_BLK_SUBSYS_DEVICE_ID 0x0002 + +#define IFCVF_NET_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ (1ULL << VIRTIO_F_VERSION_1) | \ @@ -37,6 +43,15 @@ (1ULL << VIRTIO_F_ACCESS_PLATFORM) | \ (1ULL << VIRTIO_NET_F_MRG_RXBUF)) +#define IFCVF_BLK_SUPPORTED_FEATURES \ + ((1ULL << VIRTIO_BLK_F_SIZE_MAX)| \ +(1ULL << VIRTIO_BLK_F_SEG_MAX) | \ +(1ULL << VIRTIO_BLK_F_BLK_SIZE)| \ +(1ULL << VIRTIO_BLK_F_TOPOLOGY)| \ +(1ULL << VIRTIO_BLK_F_MQ) | \ +(1ULL << VIRTIO_F_VERSION_1) | \ +(1ULL << VIRTIO_F_ACCESS_PLATFORM)) + /* Only one queue pair for now. */ #define IFCVF_MAX_QUEUE_PAIRS 1 diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 99b0a6b4c227..9b6a38b798fa 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -171,7 +171,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); u64 features; - features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; + if (vf->dev_type == VIRTIO_ID_NET) + features = ifcvf_get_features(vf) & IFCVF_NET_SUPPORTED_FEATURES; + + if (vf->dev_type == VIRTIO_ID_BLOCK) + features = ifcvf_get_features(vf) & IFCVF_BLK_SUPPORTED_FEATURES; return features; } @@ -509,6 +513,10 @@ static struct pci_device_id ifcvf_pci_ids[] = { C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, C5000X_PL_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_BLK_VENDOR_ID, +C5000X_PL_BLK_DEVICE_ID, +C5000X_PL_BLK_SUBSYS_VENDOR_ID, +C5000X_PL_BLK_SUBSYS_DEVICE_ID) }, { 0 }, }; -- 2.27.0
[PATCH 0/3] vDPA/ifcvf: enables Intel C5000X-PL virtio-blk
This series enabled Intel FGPA SmartNIC C5000X-PL virtio-blk for vDPA. This series requires: Stefano's vdpa block patchset: https://lkml.org/lkml/2021/3/15/2113 my patchset to enable Intel FGPA SmartNIC C5000X-PL virtio-net for vDPA: https://lkml.org/lkml/2021/3/17/432 Thanks! Zhu Lingshan (3): vDPA/ifcvf: deduce VIRTIO device ID when probe vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA vDPA/ifcvf: get_config_size should return dev specific config size drivers/vdpa/ifcvf/ifcvf_base.h | 18 +- drivers/vdpa/ifcvf/ifcvf_main.c | 43 ++--- 2 files changed, 46 insertions(+), 15 deletions(-) -- 2.27.0
[PATCH 1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe
This commit deduces VIRTIO device ID as device type when probe, then ifcvf_vdpa_get_device_id() can simply return the ID. ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size() can work properly based on the device ID. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index b2eeb16b9c2c..1c04cd256fa7 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -84,6 +84,7 @@ struct ifcvf_hw { u32 notify_off_multiplier; u64 req_features; u64 hw_features; + u32 dev_type; struct virtio_pci_common_cfg __iomem *common_cfg; void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 44d7586019da..99b0a6b4c227 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) { - struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); - struct pci_dev *pdev = adapter->pdev; - u32 ret = -ENODEV; - - if (pdev->device < 0x1000 || pdev->device > 0x107f) - return ret; - - if (pdev->device < 0x1040) - ret = pdev->subsystem_device; - else - ret = pdev->device - 0x1040; + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); - return ret; + return vf->dev_type; } static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) pci_set_drvdata(pdev, adapter); vf = &adapter->vf; + if (pdev->device < 0x1000 || pdev->device > 0x107f) + return -EOPNOTSUPP; + + if (pdev->device < 0x1040) + vf->dev_type = pdev->subsystem_device; + else + vf->dev_type = pdev->device - 0x1040; + vf->base = pcim_iomap_table(pdev); adapter->pdev = pdev; -- 2.27.0
[PATCH V5 7/7] vDPA/ifcvf: deduce VIRTIO device ID from pdev ids
This commit deduces the VIRTIO device ID of a probed device from its pdev device ids. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 14 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index f77239fc1644..b2eeb16b9c2c 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -127,4 +127,5 @@ int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); +int ifcvf_probed_virtio_net(struct ifcvf_hw *hw); #endif /* _IFCVF_H_ */ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index ea93ea7fd5df..9fade400b5a4 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -323,7 +323,19 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) { - return VIRTIO_ID_NET; + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + u32 ret = -ENODEV; + + if (pdev->device < 0x1000 || pdev->device > 0x107f) + return ret; + + if (pdev->device < 0x1040) + ret = pdev->subsystem_device; + else + ret = pdev->device - 0x1040; + + return ret; } static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) -- 2.27.0
[PATCH V5 6/7] vDPA/ifcvf: verify mandatory feature bits for vDPA
vDPA requres VIRTIO_F_ACCESS_PLATFORM as a must, this commit examines this when set features. Signed-off-by: Zhu Lingshan Acked-by: Jason Wang --- drivers/vdpa/ifcvf/ifcvf_base.c | 12 drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 3 files changed, 18 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index ea6a78791c9b..1a661ab45af5 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -224,6 +224,18 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw) return hw->hw_features; } +int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features) +{ + struct ifcvf_adapter *ifcvf = vf_to_adapter(hw); + + if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)) && features) { + IFCVF_ERR(ifcvf->pdev, "VIRTIO_F_ACCESS_PLATFORM is not negotiated\n"); + return -EINVAL; + } + + return 0; +} + void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset, void *dst, int length) { diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index dbb8c10aa3b1..f77239fc1644 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -123,6 +123,7 @@ void io_write64_twopart(u64 val, u32 *lo, u32 *hi); void ifcvf_reset(struct ifcvf_hw *hw); u64 ifcvf_get_features(struct ifcvf_hw *hw); u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); +int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 25fb9dfe23f0..ea93ea7fd5df 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -179,6 +179,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) static int ifcvf_vdpa_set_features(struct vdpa_device *vdpa_dev, u64 features) { struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + int ret; + + ret = ifcvf_verify_min_features(vf, features); + if (ret) + return ret; vf->req_features = features; -- 2.27.0
[PATCH V5 5/7] vDPA/ifcvf: fetch device feature bits when probe
This commit would read and store device feature bits when probe. rename ifcvf_get_features() to ifcvf_get_hw_features(), it reads and stores features of the probed device. new ifcvf_get_features() simply returns stored feature bits. Signed-off-by: Zhu Lingshan Acked-by: Jason Wang --- drivers/vdpa/ifcvf/ifcvf_base.c | 12 ++-- drivers/vdpa/ifcvf/ifcvf_base.h | 2 ++ drivers/vdpa/ifcvf/ifcvf_main.c | 2 ++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index f2a128e56de5..ea6a78791c9b 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -202,10 +202,11 @@ static void ifcvf_add_status(struct ifcvf_hw *hw, u8 status) ifcvf_get_status(hw); } -u64 ifcvf_get_features(struct ifcvf_hw *hw) +u64 ifcvf_get_hw_features(struct ifcvf_hw *hw) { struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg; u32 features_lo, features_hi; + u64 features; ifc_iowrite32(0, &cfg->device_feature_select); features_lo = ifc_ioread32(&cfg->device_feature); @@ -213,7 +214,14 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw) ifc_iowrite32(1, &cfg->device_feature_select); features_hi = ifc_ioread32(&cfg->device_feature); - return ((u64)features_hi << 32) | features_lo; + features = ((u64)features_hi << 32) | features_lo; + + return features; +} + +u64 ifcvf_get_features(struct ifcvf_hw *hw) +{ + return hw->hw_features; } void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset, diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 794d1505d857..dbb8c10aa3b1 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -83,6 +83,7 @@ struct ifcvf_hw { void __iomem *notify_base; u32 notify_off_multiplier; u64 req_features; + u64 hw_features; struct virtio_pci_common_cfg __iomem *common_cfg; void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; @@ -121,6 +122,7 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status); void io_write64_twopart(u64 val, u32 *lo, u32 *hi); void ifcvf_reset(struct ifcvf_hw *hw); u64 ifcvf_get_features(struct ifcvf_hw *hw); +u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index c34e1eec6b6c..25fb9dfe23f0 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -458,6 +458,8 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) vf->vring[i].irq = -EINVAL; + vf->hw_features = ifcvf_get_hw_features(vf); + ret = vdpa_register_device(&adapter->vdpa); if (ret) { IFCVF_ERR(pdev, "Failed to register ifcvf to vdpa bus"); -- 2.27.0
[PATCH V5 4/7] vDPA/ifcvf: remove the version number string
This commit removes the version number string, using kernel version is enough. Signed-off-by: Zhu Lingshan Reviewed-by: Leon Romanovsky Acked-by: Jason Wang --- drivers/vdpa/ifcvf/ifcvf_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fd5befc5cbcc..c34e1eec6b6c 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -14,7 +14,6 @@ #include #include "ifcvf_base.h" -#define VERSION_STRING "0.1" #define DRIVER_AUTHOR "Intel Corporation" #define IFCVF_DRIVER_NAME "ifcvf" @@ -503,4 +502,3 @@ static struct pci_driver ifcvf_driver = { module_pci_driver(ifcvf_driver); MODULE_LICENSE("GPL v2"); -MODULE_VERSION(VERSION_STRING); -- 2.27.0
[PATCH V5 3/7] vDPA/ifcvf: rename original IFCVF dev ids to N3000 ids
IFCVF driver probes multiple types of devices now, to distinguish the original device driven by IFCVF from others, it is renamed as "N3000". Signed-off-by: Zhu Lingshan Acked-by: Jason Wang --- drivers/vdpa/ifcvf/ifcvf_base.h | 8 drivers/vdpa/ifcvf/ifcvf_main.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 75d9a8052039..794d1505d857 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -18,10 +18,10 @@ #include #include -#define IFCVF_VENDOR_ID0x1AF4 -#define IFCVF_DEVICE_ID0x1041 -#define IFCVF_SUBSYS_VENDOR_ID 0x8086 -#define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define N3000_VENDOR_ID0x1AF4 +#define N3000_DEVICE_ID0x1041 +#define N3000_SUBSYS_VENDOR_ID 0x8086 +#define N3000_SUBSYS_DEVICE_ID 0x001A #define C5000X_PL_VENDOR_ID0x1AF4 #define C5000X_PL_DEVICE_ID0x1000 diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 26a2dab7ca66..fd5befc5cbcc 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -480,10 +480,10 @@ static void ifcvf_remove(struct pci_dev *pdev) } static struct pci_device_id ifcvf_pci_ids[] = { - { PCI_DEVICE_SUB(IFCVF_VENDOR_ID, - IFCVF_DEVICE_ID, - IFCVF_SUBSYS_VENDOR_ID, - IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(N3000_VENDOR_ID, +N3000_DEVICE_ID, +N3000_SUBSYS_VENDOR_ID, +N3000_SUBSYS_DEVICE_ID) }, { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, -- 2.27.0
[PATCH V5 1/7] vDPA/ifcvf: get_vendor_id returns a device specific vendor id
In this commit, ifcvf_get_vendor_id() will return a device specific vendor id of the probed pci device than a hard code. Signed-off-by: Zhu Lingshan Acked-by: Jason Wang --- drivers/vdpa/ifcvf/ifcvf_main.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fa1af301cf55..e501ee07de17 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) { - return IFCVF_SUBSYS_VENDOR_ID; + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + + return pdev->subsystem_vendor; } static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) -- 2.27.0
[PATCH V5 2/7] vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA
This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-net for vDPA Signed-off-by: Zhu Lingshan Acked-by: Jason Wang --- drivers/vdpa/ifcvf/ifcvf_base.h | 5 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 2 files changed, 10 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 64696d63fe07..75d9a8052039 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -23,6 +23,11 @@ #define IFCVF_SUBSYS_VENDOR_ID 0x8086 #define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define C5000X_PL_VENDOR_ID0x1AF4 +#define C5000X_PL_DEVICE_ID0x1000 +#define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 + #define IFCVF_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index e501ee07de17..26a2dab7ca66 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -484,6 +484,11 @@ static struct pci_device_id ifcvf_pci_ids[] = { IFCVF_DEVICE_ID, IFCVF_SUBSYS_VENDOR_ID, IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, +C5000X_PL_DEVICE_ID, +C5000X_PL_SUBSYS_VENDOR_ID, +C5000X_PL_SUBSYS_DEVICE_ID) }, + { 0 }, }; MODULE_DEVICE_TABLE(pci, ifcvf_pci_ids); -- 2.27.0
[PATCH V5 0/7] vDPA/ifcvf: enables Intel C5000X-PL virtio-net
This series enabled Intel FGPA SmartNIC C5000X-PL virtio-net for vDPA. vDPA requires VIRTIO_F_ACCESS_PLATFORM as a must, this series verify this feature bit when set features. Changes from V4: deduce VIRTIO device ID from pdev device id or subsystem device id(Jason) Changes from V3: checks features to set in verify_min_features(Jason) deduce VIRTIO device ID from pdev ids in get_device_id(Jason) Changes from V2: verify VIRTIO_F_ACCESS_PLATFORM when set features(Jason) Changes from V1: remove version number string(Leon) add new device ids and remove original device ids in separate patches(Jason) Zhu Lingshan (7): vDPA/ifcvf: get_vendor_id returns a device specific vendor id vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA vDPA/ifcvf: rename original IFCVF dev ids to N3000 ids vDPA/ifcvf: remove the version number string vDPA/ifcvf: fetch device feature bits when probe vDPA/ifcvf: verify mandatory feature bits for vDPA vDPA/ifcvf: deduce VIRTIO device ID from pdev ids drivers/vdpa/ifcvf/ifcvf_base.c | 24 +-- drivers/vdpa/ifcvf/ifcvf_base.h | 17 ++ drivers/vdpa/ifcvf/ifcvf_main.c | 41 ++--- 3 files changed, 68 insertions(+), 14 deletions(-) -- 2.27.0
[PATCH V4 5/7] vDPA/ifcvf: fetch device feature bits when probe
This commit would read and store device feature bits when probe. rename ifcvf_get_features() to ifcvf_get_hw_features(), it reads and stores features of the probed device. new ifcvf_get_features() simply returns stored feature bits. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.c | 12 ++-- drivers/vdpa/ifcvf/ifcvf_base.h | 2 ++ drivers/vdpa/ifcvf/ifcvf_main.c | 2 ++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index f2a128e56de5..ea6a78791c9b 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -202,10 +202,11 @@ static void ifcvf_add_status(struct ifcvf_hw *hw, u8 status) ifcvf_get_status(hw); } -u64 ifcvf_get_features(struct ifcvf_hw *hw) +u64 ifcvf_get_hw_features(struct ifcvf_hw *hw) { struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg; u32 features_lo, features_hi; + u64 features; ifc_iowrite32(0, &cfg->device_feature_select); features_lo = ifc_ioread32(&cfg->device_feature); @@ -213,7 +214,14 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw) ifc_iowrite32(1, &cfg->device_feature_select); features_hi = ifc_ioread32(&cfg->device_feature); - return ((u64)features_hi << 32) | features_lo; + features = ((u64)features_hi << 32) | features_lo; + + return features; +} + +u64 ifcvf_get_features(struct ifcvf_hw *hw) +{ + return hw->hw_features; } void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset, diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 794d1505d857..dbb8c10aa3b1 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -83,6 +83,7 @@ struct ifcvf_hw { void __iomem *notify_base; u32 notify_off_multiplier; u64 req_features; + u64 hw_features; struct virtio_pci_common_cfg __iomem *common_cfg; void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; @@ -121,6 +122,7 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status); void io_write64_twopart(u64 val, u32 *lo, u32 *hi); void ifcvf_reset(struct ifcvf_hw *hw); u64 ifcvf_get_features(struct ifcvf_hw *hw); +u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index c34e1eec6b6c..25fb9dfe23f0 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -458,6 +458,8 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) vf->vring[i].irq = -EINVAL; + vf->hw_features = ifcvf_get_hw_features(vf); + ret = vdpa_register_device(&adapter->vdpa); if (ret) { IFCVF_ERR(pdev, "Failed to register ifcvf to vdpa bus"); -- 2.27.0
[PATCH V4 7/7] vDPA/ifcvf: deduce VIRTIO device ID from pdev ids
This commit checks the device ids from pdev, then deduce VIRTIO device ID from the probed device. Here we checks all four device ids than only subsystem_device_id, help detecting a certain device for furture enabling. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.c | 42 + drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 8 ++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index 4f257c4b2f76..1a6ad7a11f16 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -408,3 +408,45 @@ void ifcvf_notify_queue(struct ifcvf_hw *hw, u16 qid) { ifc_iowrite16(qid, hw->vring[qid].notify_addr); } + +static int ifcvf_probed_N3000(struct pci_dev *pdev) +{ + int ret = false; + + if (pdev->device == N3000_DEVICE_ID && + pdev->vendor == N3000_VENDOR_ID && + pdev->subsystem_device == N3000_SUBSYS_DEVICE_ID && + pdev->subsystem_vendor == N3000_SUBSYS_VENDOR_ID) + ret = true; + + return ret; +} + +static int ifcvf_probed_C5000X_PL(struct pci_dev *pdev) +{ + int ret = false; + + if (pdev->device == C5000X_PL_DEVICE_ID && + pdev->vendor == C5000X_PL_VENDOR_ID && + pdev->subsystem_device == C5000X_PL_SUBSYS_DEVICE_ID && + pdev->subsystem_vendor == C5000X_PL_SUBSYS_VENDOR_ID) + ret = true; + + return ret; +} + +int ifcvf_probed_virtio_net(struct ifcvf_hw *hw) +{ + struct ifcvf_adapter *adapter; + struct pci_dev *pdev; + int ret = false; + + adapter = vf_to_adapter(hw); + pdev = adapter->pdev; + + if (ifcvf_probed_N3000(pdev) || + ifcvf_probed_C5000X_PL(pdev)) + ret = true; + + return ret; +} diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index f77239fc1644..b2eeb16b9c2c 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -127,4 +127,5 @@ int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); +int ifcvf_probed_virtio_net(struct ifcvf_hw *hw); #endif /* _IFCVF_H_ */ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index ea93ea7fd5df..b0787f79dac3 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -323,7 +323,13 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) { - return VIRTIO_ID_NET; + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + u32 ret = -EOPNOTSUPP; + + if (ifcvf_probed_virtio_net(vf)) + ret = VIRTIO_ID_NET; + + return ret; } static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) -- 2.27.0
[PATCH V4 6/7] vDPA/ifcvf: verify mandatory feature bits for vDPA
vDPA requres VIRTIO_F_ACCESS_PLATFORM as a must, this commit examines this when set features. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.c | 12 drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 3 files changed, 18 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index ea6a78791c9b..4f257c4b2f76 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -224,6 +224,18 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw) return hw->hw_features; } +int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features) +{ + struct ifcvf_adapter *ifcvf = vf_to_adapter(hw); + + if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)) && features) { + IFCVF_ERR(ifcvf->pdev, "VIRTIO_F_ACCESS_PLATFORM not negotiated\n"); + return -EINVAL; + } + + return 0; +} + void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset, void *dst, int length) { diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index dbb8c10aa3b1..f77239fc1644 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -123,6 +123,7 @@ void io_write64_twopart(u64 val, u32 *lo, u32 *hi); void ifcvf_reset(struct ifcvf_hw *hw); u64 ifcvf_get_features(struct ifcvf_hw *hw); u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); +int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 25fb9dfe23f0..ea93ea7fd5df 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -179,6 +179,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) static int ifcvf_vdpa_set_features(struct vdpa_device *vdpa_dev, u64 features) { struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + int ret; + + ret = ifcvf_verify_min_features(vf, features); + if (ret) + return ret; vf->req_features = features; -- 2.27.0
[PATCH V4 4/7] vDPA/ifcvf: remove the version number string
This commit removes the version number string, using kernel version is enough. Signed-off-by: Zhu Lingshan Reviewed-by: Leon Romanovsky --- drivers/vdpa/ifcvf/ifcvf_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fd5befc5cbcc..c34e1eec6b6c 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -14,7 +14,6 @@ #include #include "ifcvf_base.h" -#define VERSION_STRING "0.1" #define DRIVER_AUTHOR "Intel Corporation" #define IFCVF_DRIVER_NAME "ifcvf" @@ -503,4 +502,3 @@ static struct pci_driver ifcvf_driver = { module_pci_driver(ifcvf_driver); MODULE_LICENSE("GPL v2"); -MODULE_VERSION(VERSION_STRING); -- 2.27.0
[PATCH V4 3/7] vDPA/ifcvf: rename original IFCVF dev ids to N3000 ids
IFCVF driver probes multiple types of devices now, to distinguish the original device driven by IFCVF from others, it is renamed as "N3000". Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 8 drivers/vdpa/ifcvf/ifcvf_main.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 75d9a8052039..794d1505d857 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -18,10 +18,10 @@ #include #include -#define IFCVF_VENDOR_ID0x1AF4 -#define IFCVF_DEVICE_ID0x1041 -#define IFCVF_SUBSYS_VENDOR_ID 0x8086 -#define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define N3000_VENDOR_ID0x1AF4 +#define N3000_DEVICE_ID0x1041 +#define N3000_SUBSYS_VENDOR_ID 0x8086 +#define N3000_SUBSYS_DEVICE_ID 0x001A #define C5000X_PL_VENDOR_ID0x1AF4 #define C5000X_PL_DEVICE_ID0x1000 diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 26a2dab7ca66..fd5befc5cbcc 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -480,10 +480,10 @@ static void ifcvf_remove(struct pci_dev *pdev) } static struct pci_device_id ifcvf_pci_ids[] = { - { PCI_DEVICE_SUB(IFCVF_VENDOR_ID, - IFCVF_DEVICE_ID, - IFCVF_SUBSYS_VENDOR_ID, - IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(N3000_VENDOR_ID, +N3000_DEVICE_ID, +N3000_SUBSYS_VENDOR_ID, +N3000_SUBSYS_DEVICE_ID) }, { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, -- 2.27.0
[PATCH V4 1/7] vDPA/ifcvf: get_vendor_id returns a device specific vendor id
In this commit, ifcvf_get_vendor_id() will return a device specific vendor id of the probed pci device than a hard code. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fa1af301cf55..e501ee07de17 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) { - return IFCVF_SUBSYS_VENDOR_ID; + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + + return pdev->subsystem_vendor; } static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) -- 2.27.0
[PATCH V4 2/7] vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA
This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-net for vDPA Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 5 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 2 files changed, 10 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 64696d63fe07..75d9a8052039 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -23,6 +23,11 @@ #define IFCVF_SUBSYS_VENDOR_ID 0x8086 #define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define C5000X_PL_VENDOR_ID0x1AF4 +#define C5000X_PL_DEVICE_ID0x1000 +#define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 + #define IFCVF_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index e501ee07de17..26a2dab7ca66 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -484,6 +484,11 @@ static struct pci_device_id ifcvf_pci_ids[] = { IFCVF_DEVICE_ID, IFCVF_SUBSYS_VENDOR_ID, IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, +C5000X_PL_DEVICE_ID, +C5000X_PL_SUBSYS_VENDOR_ID, +C5000X_PL_SUBSYS_DEVICE_ID) }, + { 0 }, }; MODULE_DEVICE_TABLE(pci, ifcvf_pci_ids); -- 2.27.0
[PATCH V4 0/7] vDPA/ifcvf: enables Intel C5000X-PL virtio-net
This series enabled Intel FGPA SmartNIC C5000X-PL virtio-net for vDPA. vDPA requires VIRTIO_F_ACCESS_PLATFORM as a must, this series verify this feature bit when set features. Changes from V3: checks features to set in verify_min_features(Jason) deduce VIRTIO device ID from pdev ids in get_device_id(Jason) Changes from V2: verify VIRTIO_F_ACCESS_PLATFORM when set features(Jason) Changes from V1: remove version number string(Leon) add new device ids and remove original device ids in separate patches(Jason) Zhu Lingshan (7): vDPA/ifcvf: get_vendor_id returns a device specific vendor id vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA vDPA/ifcvf: rename original IFCVF dev ids to N3000 ids vDPA/ifcvf: remove the version number string vDPA/ifcvf: fetch device feature bits when probe vDPA/ifcvf: verify mandatory feature bits for vDPA vDPA/ifcvf: deduce VIRTIO device ID from pdev ids drivers/vdpa/ifcvf/ifcvf_base.c | 66 - drivers/vdpa/ifcvf/ifcvf_base.h | 17 +++-- drivers/vdpa/ifcvf/ifcvf_main.c | 35 + 3 files changed, 104 insertions(+), 14 deletions(-) -- 2.27.0
Re: [PATCH V3 6/6] vDPA/ifcvf: verify mandatory feature bits for vDPA
On 3/12/2021 3:00 PM, Jason Wang wrote: On 2021/3/12 2:40 下午, Zhu, Lingshan wrote: On 3/12/2021 1:52 PM, Jason Wang wrote: On 2021/3/11 3:19 下午, Zhu, Lingshan wrote: On 3/11/2021 2:20 PM, Jason Wang wrote: On 2021/3/11 12:16 下午, Zhu Lingshan wrote: On 3/11/2021 11:20 AM, Jason Wang wrote: On 2021/3/10 5:00 下午, Zhu Lingshan wrote: vDPA requres VIRTIO_F_ACCESS_PLATFORM as a must, this commit examines this when set features. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.c | 8 drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 3 files changed, 14 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index ea6a78791c9b..58f47fdce385 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -224,6 +224,14 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw) return hw->hw_features; } +int ifcvf_verify_min_features(struct ifcvf_hw *hw) +{ + if (!(hw->hw_features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM))) + return -EINVAL; + + return 0; +} + void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset, void *dst, int length) { diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index dbb8c10aa3b1..91c5735d4dc9 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -123,6 +123,7 @@ void io_write64_twopart(u64 val, u32 *lo, u32 *hi); void ifcvf_reset(struct ifcvf_hw *hw); u64 ifcvf_get_features(struct ifcvf_hw *hw); u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); +int ifcvf_verify_min_features(struct ifcvf_hw *hw); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 25fb9dfe23f0..f624f202447d 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -179,6 +179,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) static int ifcvf_vdpa_set_features(struct vdpa_device *vdpa_dev, u64 features) { struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + int ret; + + ret = ifcvf_verify_min_features(vf); So this validate device features instead of driver which is the one we really want to check? Thanks Hi Jason, Here we check device feature bits to make sure the device support ACCESS_PLATFORM. If you want to check device features, you need to do that during probe() and fail the probing if without the feature. But I think you won't ship cards without ACCESS_PLATFORM. Yes, there are no reasons ship a card without ACCESS_PLATFORM In get_features(), it will return a intersection of device features bit and driver supported features bits(which includes ACCESS_PLATFORM). Other components like QEMU should not set features bits more than this intersection of bits. so we can make sure if this ifcvf_verify_min_features() passed, both device and driver support ACCESS_PLATFORM. Are you suggesting check driver feature bits in ifcvf_verify_min_features() in the meantime as well? So it really depends on your hardware. If you hardware can always offer ACCESS_PLATFORM, you just need to check driver features. This is how vdpa_sim and mlx5_vdpa work. Yes, we always support ACCESS_PLATFORM, so it is hard coded in the macro IFCVF_SUPPORTED_FEATURES. That's not what I read from the code: features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; ifcvf_get_features() reads device feature bits(which should always has ACCSSS_PLATFORM) and IFCVF_SUPPORTED_FEATURES is the driver supported feature bits For "driver" you probably mean IFCVF. So there's some misunderstanding before, what I meant for "driver" is virtio driver that do feature negotaitation with the device. I wonder what features are supported by the device but not the IFCVF driver? Thanks we did not use TSO hardware feature bits in IFCVF driver for now. Anyway, we will check the features bits to set in set_features than hw/ifcvf driver feature bits. THanks! which hard coded ACCESS_PLATFORM, so the intersection should include ACCESS_PLATFORM. the intersection "features" is returned in get_features(), qemu should set features according to it. Now we check whether device support this feature bit as a double conformation, are you suggesting we should check whether ACCESS_PLATFORM & IFCVF_SUPPORTED_FEATURES in set_features() as well? If we know device will always offer ACCESS_PLATFORM, there's no need to check it again. What we should check if whether driver set that, and if it doesn't we need to fail set_features(). I think there's little chance that IFCVF can work when IOMMU_PLATFORM is not negotiated. Agree, will check
Re: [PATCH V3 6/6] vDPA/ifcvf: verify mandatory feature bits for vDPA
On 3/12/2021 1:52 PM, Jason Wang wrote: On 2021/3/11 3:19 下午, Zhu, Lingshan wrote: On 3/11/2021 2:20 PM, Jason Wang wrote: On 2021/3/11 12:16 下午, Zhu Lingshan wrote: On 3/11/2021 11:20 AM, Jason Wang wrote: On 2021/3/10 5:00 下午, Zhu Lingshan wrote: vDPA requres VIRTIO_F_ACCESS_PLATFORM as a must, this commit examines this when set features. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.c | 8 drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 3 files changed, 14 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index ea6a78791c9b..58f47fdce385 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -224,6 +224,14 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw) return hw->hw_features; } +int ifcvf_verify_min_features(struct ifcvf_hw *hw) +{ + if (!(hw->hw_features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM))) + return -EINVAL; + + return 0; +} + void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset, void *dst, int length) { diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index dbb8c10aa3b1..91c5735d4dc9 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -123,6 +123,7 @@ void io_write64_twopart(u64 val, u32 *lo, u32 *hi); void ifcvf_reset(struct ifcvf_hw *hw); u64 ifcvf_get_features(struct ifcvf_hw *hw); u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); +int ifcvf_verify_min_features(struct ifcvf_hw *hw); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 25fb9dfe23f0..f624f202447d 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -179,6 +179,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) static int ifcvf_vdpa_set_features(struct vdpa_device *vdpa_dev, u64 features) { struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + int ret; + + ret = ifcvf_verify_min_features(vf); So this validate device features instead of driver which is the one we really want to check? Thanks Hi Jason, Here we check device feature bits to make sure the device support ACCESS_PLATFORM. If you want to check device features, you need to do that during probe() and fail the probing if without the feature. But I think you won't ship cards without ACCESS_PLATFORM. Yes, there are no reasons ship a card without ACCESS_PLATFORM In get_features(), it will return a intersection of device features bit and driver supported features bits(which includes ACCESS_PLATFORM). Other components like QEMU should not set features bits more than this intersection of bits. so we can make sure if this ifcvf_verify_min_features() passed, both device and driver support ACCESS_PLATFORM. Are you suggesting check driver feature bits in ifcvf_verify_min_features() in the meantime as well? So it really depends on your hardware. If you hardware can always offer ACCESS_PLATFORM, you just need to check driver features. This is how vdpa_sim and mlx5_vdpa work. Yes, we always support ACCESS_PLATFORM, so it is hard coded in the macro IFCVF_SUPPORTED_FEATURES. That's not what I read from the code: features = ifcvf_get_features(vf) & IFCVF_SUPPORTED_FEATURES; ifcvf_get_features() reads device feature bits(which should always has ACCSSS_PLATFORM) and IFCVF_SUPPORTED_FEATURES is the driver supported feature bits which hard coded ACCESS_PLATFORM, so the intersection should include ACCESS_PLATFORM. the intersection "features" is returned in get_features(), qemu should set features according to it. Now we check whether device support this feature bit as a double conformation, are you suggesting we should check whether ACCESS_PLATFORM & IFCVF_SUPPORTED_FEATURES in set_features() as well? If we know device will always offer ACCESS_PLATFORM, there's no need to check it again. What we should check if whether driver set that, and if it doesn't we need to fail set_features(). I think there's little chance that IFCVF can work when IOMMU_PLATFORM is not negotiated. Agree, will check the features bit to set instead of device feature bits. Thanks! I prefer check both device and IFCVF_SUPPORTED_FEATURES both, more reliable. So again, if you want to check device features, set_features() is not the proper place. We need to fail the probe in this case. Thanks Thanks! Thanks Thanks! + if (ret) + return ret; vf->req_features = features; ___ Virtualization mailing list virtualizat...@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/2] vhost-vdpa: fix use-after-free of v->config_ctx
On 3/11/2021 9:52 PM, Stefano Garzarella wrote: When the 'v->config_ctx' eventfd_ctx reference is released we didn't set it to NULL. So if the same character device (e.g. /dev/vhost-vdpa-0) is re-opened, the 'v->config_ctx' is invalid and calling again vhost_vdpa_config_put() causes use-after-free issues like the following refcount_t underflow: refcount_t: underflow; use-after-free. WARNING: CPU: 2 PID: 872 at lib/refcount.c:28 refcount_warn_saturate+0xae/0xf0 RIP: 0010:refcount_warn_saturate+0xae/0xf0 Call Trace: eventfd_ctx_put+0x5b/0x70 vhost_vdpa_release+0xcd/0x150 [vhost_vdpa] __fput+0x8e/0x240 fput+0xe/0x10 task_work_run+0x66/0xa0 exit_to_user_mode_prepare+0x118/0x120 syscall_exit_to_user_mode+0x21/0x50 ? __x64_sys_close+0x12/0x40 do_syscall_64+0x45/0x50 entry_SYSCALL_64_after_hwframe+0x44/0xae Fixes: 776f395004d8 ("vhost_vdpa: Support config interrupt in vdpa") Cc: lingshan@intel.com Cc: sta...@vger.kernel.org Signed-off-by: Stefano Garzarella --- drivers/vhost/vdpa.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index ef688c8c0e0e..00796e4ecfdf 100644 --- a/drivers/vhost/vdpa.c +++ b/drivers/vhost/vdpa.c @@ -308,8 +308,10 @@ static long vhost_vdpa_get_vring_num(struct vhost_vdpa *v, u16 __user *argp) static void vhost_vdpa_config_put(struct vhost_vdpa *v) { - if (v->config_ctx) + if (v->config_ctx) { eventfd_ctx_put(v->config_ctx); + v->config_ctx = NULL; + } } static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp) Thanks Stefano! Reviewed-by: Zhu Lingshan
Re: [PATCH V3 1/6] vDPA/ifcvf: get_vendor_id returns a device specific vendor id
On 3/11/2021 2:13 PM, Jason Wang wrote: On 2021/3/11 12:21 下午, Zhu Lingshan wrote: On 3/11/2021 11:23 AM, Jason Wang wrote: On 2021/3/10 5:00 下午, Zhu Lingshan wrote: In this commit, ifcvf_get_vendor_id() will return a device specific vendor id of the probed pci device than a hard code. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fa1af301cf55..e501ee07de17 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) { - return IFCVF_SUBSYS_VENDOR_ID; + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + + return pdev->subsystem_vendor; } While at this, I wonder if we can do something similar in get_device_id() if it could be simple deduced from some simple math from the pci device id? Thanks Hi Jason, IMHO, this implementation is just some memory read ops, I think other implementations may not save many cpu cycles, an if cost at least three cpu cycles. Thanks! Well, I meant whehter you can deduce virtio device id from pdev->subsystem_device. Thanks Oh, sure, I get you static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev)
Re: [PATCH V3 6/6] vDPA/ifcvf: verify mandatory feature bits for vDPA
On 3/11/2021 2:20 PM, Jason Wang wrote: On 2021/3/11 12:16 下午, Zhu Lingshan wrote: On 3/11/2021 11:20 AM, Jason Wang wrote: On 2021/3/10 5:00 下午, Zhu Lingshan wrote: vDPA requres VIRTIO_F_ACCESS_PLATFORM as a must, this commit examines this when set features. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.c | 8 drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 3 files changed, 14 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index ea6a78791c9b..58f47fdce385 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -224,6 +224,14 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw) return hw->hw_features; } +int ifcvf_verify_min_features(struct ifcvf_hw *hw) +{ + if (!(hw->hw_features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM))) + return -EINVAL; + + return 0; +} + void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset, void *dst, int length) { diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index dbb8c10aa3b1..91c5735d4dc9 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -123,6 +123,7 @@ void io_write64_twopart(u64 val, u32 *lo, u32 *hi); void ifcvf_reset(struct ifcvf_hw *hw); u64 ifcvf_get_features(struct ifcvf_hw *hw); u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); +int ifcvf_verify_min_features(struct ifcvf_hw *hw); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 25fb9dfe23f0..f624f202447d 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -179,6 +179,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) static int ifcvf_vdpa_set_features(struct vdpa_device *vdpa_dev, u64 features) { struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + int ret; + + ret = ifcvf_verify_min_features(vf); So this validate device features instead of driver which is the one we really want to check? Thanks Hi Jason, Here we check device feature bits to make sure the device support ACCESS_PLATFORM. If you want to check device features, you need to do that during probe() and fail the probing if without the feature. But I think you won't ship cards without ACCESS_PLATFORM. Yes, there are no reasons ship a card without ACCESS_PLATFORM In get_features(), it will return a intersection of device features bit and driver supported features bits(which includes ACCESS_PLATFORM). Other components like QEMU should not set features bits more than this intersection of bits. so we can make sure if this ifcvf_verify_min_features() passed, both device and driver support ACCESS_PLATFORM. Are you suggesting check driver feature bits in ifcvf_verify_min_features() in the meantime as well? So it really depends on your hardware. If you hardware can always offer ACCESS_PLATFORM, you just need to check driver features. This is how vdpa_sim and mlx5_vdpa work. Yes, we always support ACCESS_PLATFORM, so it is hard coded in the macro IFCVF_SUPPORTED_FEATURES. Now we check whether device support this feature bit as a double conformation, are you suggesting we should check whether ACCESS_PLATFORM & IFCVF_SUPPORTED_FEATURES in set_features() as well? I prefer check both device and IFCVF_SUPPORTED_FEATURES both, more reliable. Thanks! Thanks Thanks! + if (ret) + return ret; vf->req_features = features; ___ Virtualization mailing list virtualizat...@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH V3 3/6] vDPA/ifcvf: rename original IFCVF dev ids to N3000 ids
On 3/11/2021 11:25 AM, Jason Wang wrote: On 2021/3/10 5:00 下午, Zhu Lingshan wrote: IFCVF driver probes multiple types of devices now, to distinguish the original device driven by IFCVF from others, it is renamed as "N3000". Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 8 drivers/vdpa/ifcvf/ifcvf_main.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 75d9a8052039..794d1505d857 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -18,10 +18,10 @@ #include #include -#define IFCVF_VENDOR_ID 0x1AF4 -#define IFCVF_DEVICE_ID 0x1041 -#define IFCVF_SUBSYS_VENDOR_ID 0x8086 -#define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define N3000_VENDOR_ID 0x1AF4 +#define N3000_DEVICE_ID 0x1041 +#define N3000_SUBSYS_VENDOR_ID 0x8086 +#define N3000_SUBSYS_DEVICE_ID 0x001A #define C5000X_PL_VENDOR_ID 0x1AF4 #define C5000X_PL_DEVICE_ID 0x1000 diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 26a2dab7ca66..fd5befc5cbcc 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -480,10 +480,10 @@ static void ifcvf_remove(struct pci_dev *pdev) } static struct pci_device_id ifcvf_pci_ids[] = { - { PCI_DEVICE_SUB(IFCVF_VENDOR_ID, - IFCVF_DEVICE_ID, - IFCVF_SUBSYS_VENDOR_ID, - IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(N3000_VENDOR_ID, + N3000_DEVICE_ID, I am not sure the plan for Intel but I wonder if we can simply use PCI_ANY_ID for device id here. Otherewise you need to maintain a very long list of ids here. Thanks Hi Jason, Thanks! but maybe if we present a very simple and clear list like what e1000 does can help the users understand what we support easily. Thanks! + N3000_SUBSYS_VENDOR_ID, + N3000_SUBSYS_DEVICE_ID) }, { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID,
Re: [PATCH V3 1/6] vDPA/ifcvf: get_vendor_id returns a device specific vendor id
On 3/11/2021 11:23 AM, Jason Wang wrote: On 2021/3/10 5:00 下午, Zhu Lingshan wrote: In this commit, ifcvf_get_vendor_id() will return a device specific vendor id of the probed pci device than a hard code. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fa1af301cf55..e501ee07de17 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) { - return IFCVF_SUBSYS_VENDOR_ID; + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + + return pdev->subsystem_vendor; } While at this, I wonder if we can do something similar in get_device_id() if it could be simple deduced from some simple math from the pci device id? Thanks Hi Jason, IMHO, this implementation is just some memory read ops, I think other implementations may not save many cpu cycles, an if cost at least three cpu cycles. Thanks! static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev)
Re: [PATCH V3 6/6] vDPA/ifcvf: verify mandatory feature bits for vDPA
On 3/11/2021 11:20 AM, Jason Wang wrote: On 2021/3/10 5:00 下午, Zhu Lingshan wrote: vDPA requres VIRTIO_F_ACCESS_PLATFORM as a must, this commit examines this when set features. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.c | 8 drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 3 files changed, 14 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index ea6a78791c9b..58f47fdce385 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -224,6 +224,14 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw) return hw->hw_features; } +int ifcvf_verify_min_features(struct ifcvf_hw *hw) +{ + if (!(hw->hw_features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM))) + return -EINVAL; + + return 0; +} + void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset, void *dst, int length) { diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index dbb8c10aa3b1..91c5735d4dc9 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -123,6 +123,7 @@ void io_write64_twopart(u64 val, u32 *lo, u32 *hi); void ifcvf_reset(struct ifcvf_hw *hw); u64 ifcvf_get_features(struct ifcvf_hw *hw); u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); +int ifcvf_verify_min_features(struct ifcvf_hw *hw); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 25fb9dfe23f0..f624f202447d 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -179,6 +179,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) static int ifcvf_vdpa_set_features(struct vdpa_device *vdpa_dev, u64 features) { struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + int ret; + + ret = ifcvf_verify_min_features(vf); So this validate device features instead of driver which is the one we really want to check? Thanks Hi Jason, Here we check device feature bits to make sure the device support ACCESS_PLATFORM. In get_features(), it will return a intersection of device features bit and driver supported features bits(which includes ACCESS_PLATFORM). Other components like QEMU should not set features bits more than this intersection of bits. so we can make sure if this ifcvf_verify_min_features() passed, both device and driver support ACCESS_PLATFORM. Are you suggesting check driver feature bits in ifcvf_verify_min_features() in the meantime as well? Thanks! + if (ret) + return ret; vf->req_features = features; ___ Virtualization mailing list virtualizat...@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH V3 5/6] vDPA/ifcvf: fetch device feature bits when probe
This commit would read and store device feature bits when probe. rename ifcvf_get_features() to ifcvf_get_hw_features(), it reads and stores features of the probed device. new ifcvf_get_features() simply returns stored feature bits. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.c | 12 ++-- drivers/vdpa/ifcvf/ifcvf_base.h | 2 ++ drivers/vdpa/ifcvf/ifcvf_main.c | 2 ++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index f2a128e56de5..ea6a78791c9b 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -202,10 +202,11 @@ static void ifcvf_add_status(struct ifcvf_hw *hw, u8 status) ifcvf_get_status(hw); } -u64 ifcvf_get_features(struct ifcvf_hw *hw) +u64 ifcvf_get_hw_features(struct ifcvf_hw *hw) { struct virtio_pci_common_cfg __iomem *cfg = hw->common_cfg; u32 features_lo, features_hi; + u64 features; ifc_iowrite32(0, &cfg->device_feature_select); features_lo = ifc_ioread32(&cfg->device_feature); @@ -213,7 +214,14 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw) ifc_iowrite32(1, &cfg->device_feature_select); features_hi = ifc_ioread32(&cfg->device_feature); - return ((u64)features_hi << 32) | features_lo; + features = ((u64)features_hi << 32) | features_lo; + + return features; +} + +u64 ifcvf_get_features(struct ifcvf_hw *hw) +{ + return hw->hw_features; } void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset, diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 794d1505d857..dbb8c10aa3b1 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -83,6 +83,7 @@ struct ifcvf_hw { void __iomem *notify_base; u32 notify_off_multiplier; u64 req_features; + u64 hw_features; struct virtio_pci_common_cfg __iomem *common_cfg; void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; @@ -121,6 +122,7 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status); void io_write64_twopart(u64 val, u32 *lo, u32 *hi); void ifcvf_reset(struct ifcvf_hw *hw); u64 ifcvf_get_features(struct ifcvf_hw *hw); +u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index c34e1eec6b6c..25fb9dfe23f0 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -458,6 +458,8 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id) for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) vf->vring[i].irq = -EINVAL; + vf->hw_features = ifcvf_get_hw_features(vf); + ret = vdpa_register_device(&adapter->vdpa); if (ret) { IFCVF_ERR(pdev, "Failed to register ifcvf to vdpa bus"); -- 2.27.0
[PATCH V3 6/6] vDPA/ifcvf: verify mandatory feature bits for vDPA
vDPA requres VIRTIO_F_ACCESS_PLATFORM as a must, this commit examines this when set features. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.c | 8 drivers/vdpa/ifcvf/ifcvf_base.h | 1 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 3 files changed, 14 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index ea6a78791c9b..58f47fdce385 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -224,6 +224,14 @@ u64 ifcvf_get_features(struct ifcvf_hw *hw) return hw->hw_features; } +int ifcvf_verify_min_features(struct ifcvf_hw *hw) +{ + if (!(hw->hw_features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM))) + return -EINVAL; + + return 0; +} + void ifcvf_read_net_config(struct ifcvf_hw *hw, u64 offset, void *dst, int length) { diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index dbb8c10aa3b1..91c5735d4dc9 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -123,6 +123,7 @@ void io_write64_twopart(u64 val, u32 *lo, u32 *hi); void ifcvf_reset(struct ifcvf_hw *hw); u64 ifcvf_get_features(struct ifcvf_hw *hw); u64 ifcvf_get_hw_features(struct ifcvf_hw *hw); +int ifcvf_verify_min_features(struct ifcvf_hw *hw); u16 ifcvf_get_vq_state(struct ifcvf_hw *hw, u16 qid); int ifcvf_set_vq_state(struct ifcvf_hw *hw, u16 qid, u16 num); struct ifcvf_adapter *vf_to_adapter(struct ifcvf_hw *hw); diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 25fb9dfe23f0..f624f202447d 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -179,6 +179,11 @@ static u64 ifcvf_vdpa_get_features(struct vdpa_device *vdpa_dev) static int ifcvf_vdpa_set_features(struct vdpa_device *vdpa_dev, u64 features) { struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + int ret; + + ret = ifcvf_verify_min_features(vf); + if (ret) + return ret; vf->req_features = features; -- 2.27.0
[PATCH V3 4/6] vDPA/ifcvf: remove the version number string
This commit removes the version number string, using kernel version is enough. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fd5befc5cbcc..c34e1eec6b6c 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -14,7 +14,6 @@ #include #include "ifcvf_base.h" -#define VERSION_STRING "0.1" #define DRIVER_AUTHOR "Intel Corporation" #define IFCVF_DRIVER_NAME "ifcvf" @@ -503,4 +502,3 @@ static struct pci_driver ifcvf_driver = { module_pci_driver(ifcvf_driver); MODULE_LICENSE("GPL v2"); -MODULE_VERSION(VERSION_STRING); -- 2.27.0
[PATCH V3 3/6] vDPA/ifcvf: rename original IFCVF dev ids to N3000 ids
IFCVF driver probes multiple types of devices now, to distinguish the original device driven by IFCVF from others, it is renamed as "N3000". Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 8 drivers/vdpa/ifcvf/ifcvf_main.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 75d9a8052039..794d1505d857 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -18,10 +18,10 @@ #include #include -#define IFCVF_VENDOR_ID0x1AF4 -#define IFCVF_DEVICE_ID0x1041 -#define IFCVF_SUBSYS_VENDOR_ID 0x8086 -#define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define N3000_VENDOR_ID0x1AF4 +#define N3000_DEVICE_ID0x1041 +#define N3000_SUBSYS_VENDOR_ID 0x8086 +#define N3000_SUBSYS_DEVICE_ID 0x001A #define C5000X_PL_VENDOR_ID0x1AF4 #define C5000X_PL_DEVICE_ID0x1000 diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 26a2dab7ca66..fd5befc5cbcc 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -480,10 +480,10 @@ static void ifcvf_remove(struct pci_dev *pdev) } static struct pci_device_id ifcvf_pci_ids[] = { - { PCI_DEVICE_SUB(IFCVF_VENDOR_ID, - IFCVF_DEVICE_ID, - IFCVF_SUBSYS_VENDOR_ID, - IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(N3000_VENDOR_ID, +N3000_DEVICE_ID, +N3000_SUBSYS_VENDOR_ID, +N3000_SUBSYS_DEVICE_ID) }, { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, -- 2.27.0
[PATCH V3 2/6] vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA
This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-net for vDPA Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 5 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 2 files changed, 10 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 64696d63fe07..75d9a8052039 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -23,6 +23,11 @@ #define IFCVF_SUBSYS_VENDOR_ID 0x8086 #define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define C5000X_PL_VENDOR_ID0x1AF4 +#define C5000X_PL_DEVICE_ID0x1000 +#define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 + #define IFCVF_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index e501ee07de17..26a2dab7ca66 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -484,6 +484,11 @@ static struct pci_device_id ifcvf_pci_ids[] = { IFCVF_DEVICE_ID, IFCVF_SUBSYS_VENDOR_ID, IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, +C5000X_PL_DEVICE_ID, +C5000X_PL_SUBSYS_VENDOR_ID, +C5000X_PL_SUBSYS_DEVICE_ID) }, + { 0 }, }; MODULE_DEVICE_TABLE(pci, ifcvf_pci_ids); -- 2.27.0
[PATCH V3 1/6] vDPA/ifcvf: get_vendor_id returns a device specific vendor id
In this commit, ifcvf_get_vendor_id() will return a device specific vendor id of the probed pci device than a hard code. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fa1af301cf55..e501ee07de17 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) { - return IFCVF_SUBSYS_VENDOR_ID; + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + + return pdev->subsystem_vendor; } static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) -- 2.27.0
[PATCH V3 0/6] vDPA/ifcvf: enables Intel C5000X-PL virtio-net
This series enabled Intel FGPA SmartNIC C5000X-PL virtio-net for vDPA. vDPA requires VIRTIO_F_ACCESS_PLATFORM as a must, this series verify this feature bit when set features. Changes from V2: verify VIRTIO_F_ACCESS_PLATFORM when set features(Jason) Changes from V1: remove version number string(Leon) add new device ids and remove original device ids in separate patches(Jason) Zhu Lingshan (6): vDPA/ifcvf: get_vendor_id returns a device specific vendor id vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA vDPA/ifcvf: rename original IFCVF dev ids to N3000 ids vDPA/ifcvf: remove the version number string vDPA/ifcvf: fetch device feature bits when probe vDPA/ifcvf: verify mandatory feature bits for vDPA drivers/vdpa/ifcvf/ifcvf_base.c | 20 ++-- drivers/vdpa/ifcvf/ifcvf_base.h | 16 drivers/vdpa/ifcvf/ifcvf_main.c | 27 --- 3 files changed, 50 insertions(+), 13 deletions(-) -- 2.27.0
Re: [PATCH V2 2/4] vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA
On 3/9/2021 10:42 AM, Jason Wang wrote: On 2021/3/9 10:28 上午, Zhu, Lingshan wrote: On 3/9/2021 10:23 AM, Jason Wang wrote: On 2021/3/8 4:35 下午, Zhu Lingshan wrote: This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-net for vDPA Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 5 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 2 files changed, 10 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 64696d63fe07..75d9a8052039 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -23,6 +23,11 @@ #define IFCVF_SUBSYS_VENDOR_ID 0x8086 #define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define C5000X_PL_VENDOR_ID 0x1AF4 +#define C5000X_PL_DEVICE_ID 0x1000 +#define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 I just notice that the device is a transtitional one. Any reason for doing this? Note that IFCVF is a moden device anyhow (0x1041). Supporting legacy drive may bring many issues (e.g the definition is non-nomartive). One example is the support of VIRTIO_F_IOMMU_PLATFORM, legacy driver may assume the device can bypass IOMMU. Thanks Hi Jason, This device will support virtio1.0 by default, so has VIRTIO_F_IOMMU_PLATFORM by default. If you device want to force VIRTIO_F_IOMMU_PLATFORM you probably need to do what has been done by mlx5 (verify_min_features). According to the spec, if VIRTIO_F_IOMMU_PLATFORM is not mandatory, when it's not negotiated, device needs to disable or bypass IOMMU: " If this feature bit is set to 0, then the device has same access to memory addresses supplied to it as the driver has. In particular, the device will always use physical addresses matching addresses used by the driver (typically meaning physical addresses used by the CPU) and not translated further, and can access any address supplied to it by the driver. " sure, I can implement code to check the feature bits. Transitional device gives the software a chance to fall back to virtio 0.95. This only applies if you want to passthrough the card to guest directly without the help of vDPA. If we go with vDPA, it doesn't hlep. For virtio-vdpa, we know it will negotiated IOMMU_PLATFORM. For vhost-vdpa, Qemu can provide a legacy or transitional device on top of a modern vDPA device. Thanks For some cases, users may run quite out of date OS does not have vDPA nor virtio 1.0 support, transitional characters give them a chance to use the devices. Thanks Zhu Lingshan ifcvf drives this device in virtio 1.0 mode, set features VIRTIO_F_IOMMU_PLATFORM successfully. Thanks, Zhu Lingshan + #define IFCVF_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index e501ee07de17..26a2dab7ca66 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -484,6 +484,11 @@ static struct pci_device_id ifcvf_pci_ids[] = { IFCVF_DEVICE_ID, IFCVF_SUBSYS_VENDOR_ID, IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, + C5000X_PL_DEVICE_ID, + C5000X_PL_SUBSYS_VENDOR_ID, + C5000X_PL_SUBSYS_DEVICE_ID) }, + { 0 }, }; MODULE_DEVICE_TABLE(pci, ifcvf_pci_ids);
Re: [PATCH V2 2/4] vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA
On 3/9/2021 10:23 AM, Jason Wang wrote: On 2021/3/8 4:35 下午, Zhu Lingshan wrote: This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-net for vDPA Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 5 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 2 files changed, 10 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 64696d63fe07..75d9a8052039 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -23,6 +23,11 @@ #define IFCVF_SUBSYS_VENDOR_ID 0x8086 #define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define C5000X_PL_VENDOR_ID 0x1AF4 +#define C5000X_PL_DEVICE_ID 0x1000 +#define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 I just notice that the device is a transtitional one. Any reason for doing this? Note that IFCVF is a moden device anyhow (0x1041). Supporting legacy drive may bring many issues (e.g the definition is non-nomartive). One example is the support of VIRTIO_F_IOMMU_PLATFORM, legacy driver may assume the device can bypass IOMMU. Thanks Hi Jason, This device will support virtio1.0 by default, so has VIRTIO_F_IOMMU_PLATFORM by default. Transitional device gives the software a chance to fall back to virtio 0.95. ifcvf drives this device in virtio 1.0 mode, set features VIRTIO_F_IOMMU_PLATFORM successfully. Thanks, Zhu Lingshan + #define IFCVF_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index e501ee07de17..26a2dab7ca66 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -484,6 +484,11 @@ static struct pci_device_id ifcvf_pci_ids[] = { IFCVF_DEVICE_ID, IFCVF_SUBSYS_VENDOR_ID, IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, + C5000X_PL_DEVICE_ID, + C5000X_PL_SUBSYS_VENDOR_ID, + C5000X_PL_SUBSYS_DEVICE_ID) }, + { 0 }, }; MODULE_DEVICE_TABLE(pci, ifcvf_pci_ids);
[PATCH V2 4/4] vDPA/ifcvf: remove the version number string
This commit removes the version number string, using kernel version is enough. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fd5befc5cbcc..c34e1eec6b6c 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -14,7 +14,6 @@ #include #include "ifcvf_base.h" -#define VERSION_STRING "0.1" #define DRIVER_AUTHOR "Intel Corporation" #define IFCVF_DRIVER_NAME "ifcvf" @@ -503,4 +502,3 @@ static struct pci_driver ifcvf_driver = { module_pci_driver(ifcvf_driver); MODULE_LICENSE("GPL v2"); -MODULE_VERSION(VERSION_STRING); -- 2.27.0
[PATCH V2 2/4] vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA
This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-net for vDPA Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 5 + drivers/vdpa/ifcvf/ifcvf_main.c | 5 + 2 files changed, 10 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 64696d63fe07..75d9a8052039 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -23,6 +23,11 @@ #define IFCVF_SUBSYS_VENDOR_ID 0x8086 #define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define C5000X_PL_VENDOR_ID0x1AF4 +#define C5000X_PL_DEVICE_ID0x1000 +#define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 + #define IFCVF_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index e501ee07de17..26a2dab7ca66 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -484,6 +484,11 @@ static struct pci_device_id ifcvf_pci_ids[] = { IFCVF_DEVICE_ID, IFCVF_SUBSYS_VENDOR_ID, IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, +C5000X_PL_DEVICE_ID, +C5000X_PL_SUBSYS_VENDOR_ID, +C5000X_PL_SUBSYS_DEVICE_ID) }, + { 0 }, }; MODULE_DEVICE_TABLE(pci, ifcvf_pci_ids); -- 2.27.0
[PATCH V2 3/4] vDPA/ifcvf: rename original IFCVF dev ids to N3000 ids
IFCVF driver probes multiple types of devices now, to distinguish the original device driven by IFCVF from others, it is renamed as "N3000". Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 8 drivers/vdpa/ifcvf/ifcvf_main.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 75d9a8052039..794d1505d857 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -18,10 +18,10 @@ #include #include -#define IFCVF_VENDOR_ID0x1AF4 -#define IFCVF_DEVICE_ID0x1041 -#define IFCVF_SUBSYS_VENDOR_ID 0x8086 -#define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define N3000_VENDOR_ID0x1AF4 +#define N3000_DEVICE_ID0x1041 +#define N3000_SUBSYS_VENDOR_ID 0x8086 +#define N3000_SUBSYS_DEVICE_ID 0x001A #define C5000X_PL_VENDOR_ID0x1AF4 #define C5000X_PL_DEVICE_ID0x1000 diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 26a2dab7ca66..fd5befc5cbcc 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -480,10 +480,10 @@ static void ifcvf_remove(struct pci_dev *pdev) } static struct pci_device_id ifcvf_pci_ids[] = { - { PCI_DEVICE_SUB(IFCVF_VENDOR_ID, - IFCVF_DEVICE_ID, - IFCVF_SUBSYS_VENDOR_ID, - IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(N3000_VENDOR_ID, +N3000_DEVICE_ID, +N3000_SUBSYS_VENDOR_ID, +N3000_SUBSYS_DEVICE_ID) }, { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, C5000X_PL_DEVICE_ID, C5000X_PL_SUBSYS_VENDOR_ID, -- 2.27.0
[PATCH V2 1/4] vDPA/ifcvf: get_vendor_id returns a device specific vendor id
In this commit, ifcvf_get_vendor_id() will return a device specific vendor id of the probed pci device than a hard code. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fa1af301cf55..e501ee07de17 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) { - return IFCVF_SUBSYS_VENDOR_ID; + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + + return pdev->subsystem_vendor; } static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) -- 2.27.0
[PATCH V2 0/4] vDPA/ifcvf: enables Intel C5000X-PL virtio-net
This series enabled Intel FGPA SmartNIC C5000X-PL virtio-net for vDPA changes from V1: remove version number string(Leon) add new device ids and remove original device ids in separate patches(Jason) Zhu Lingshan (4): vDPA/ifcvf: get_vendor_id returns a device specific vendor id vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA vDPA/ifcvf: rename original IFCVF dev ids to N3000 ids vDPA/ifcvf: remove the version number string drivers/vdpa/ifcvf/ifcvf_base.h | 13 + drivers/vdpa/ifcvf/ifcvf_main.c | 20 +--- 2 files changed, 22 insertions(+), 11 deletions(-) -- 2.27.0
Re: [PATCH 3/3] vDPA/ifcvf: bump version string to 1.0
Hi Leon, Thanks for point this out, will send a V2 patchset delete it. Thanks Zhu Lingshan On 3/7/2021 5:01 PM, Leon Romanovsky wrote: On Fri, Mar 05, 2021 at 10:20:00PM +0800, Zhu Lingshan wrote: This commit bumps ifcvf driver version string to 1.0 Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fd5befc5cbcc..56a0974cf93c 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -14,7 +14,7 @@ #include #include "ifcvf_base.h" -#define VERSION_STRING "0.1" +#define VERSION_STRING "1.0" Please delete it instead of bumping it. We are not supposed to use in-kernel version for years already. https://lore.kernel.org/ksummit-discuss/20170625072423.GR1248@mtr-leonro.local/ Thanks
[PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA
This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-net for vDPA. C5000X-PL vendor id 0x1AF4, device id 0x1000, subvendor id 0x8086, sub device id 0x0001 To distinguish C5000X-PL from other ifcvf driven devices, the original ifcvf device is named "N3000". Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_base.h | 13 + drivers/vdpa/ifcvf/ifcvf_main.c | 13 + 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index 64696d63fe07..794d1505d857 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -18,10 +18,15 @@ #include #include -#define IFCVF_VENDOR_ID0x1AF4 -#define IFCVF_DEVICE_ID0x1041 -#define IFCVF_SUBSYS_VENDOR_ID 0x8086 -#define IFCVF_SUBSYS_DEVICE_ID 0x001A +#define N3000_VENDOR_ID0x1AF4 +#define N3000_DEVICE_ID0x1041 +#define N3000_SUBSYS_VENDOR_ID 0x8086 +#define N3000_SUBSYS_DEVICE_ID 0x001A + +#define C5000X_PL_VENDOR_ID0x1AF4 +#define C5000X_PL_DEVICE_ID0x1000 +#define C5000X_PL_SUBSYS_VENDOR_ID 0x8086 +#define C5000X_PL_SUBSYS_DEVICE_ID 0x0001 #define IFCVF_SUPPORTED_FEATURES \ ((1ULL << VIRTIO_NET_F_MAC) | \ diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index e501ee07de17..fd5befc5cbcc 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -480,10 +480,15 @@ static void ifcvf_remove(struct pci_dev *pdev) } static struct pci_device_id ifcvf_pci_ids[] = { - { PCI_DEVICE_SUB(IFCVF_VENDOR_ID, - IFCVF_DEVICE_ID, - IFCVF_SUBSYS_VENDOR_ID, - IFCVF_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(N3000_VENDOR_ID, +N3000_DEVICE_ID, +N3000_SUBSYS_VENDOR_ID, +N3000_SUBSYS_DEVICE_ID) }, + { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID, +C5000X_PL_DEVICE_ID, +C5000X_PL_SUBSYS_VENDOR_ID, +C5000X_PL_SUBSYS_DEVICE_ID) }, + { 0 }, }; MODULE_DEVICE_TABLE(pci, ifcvf_pci_ids); -- 2.27.0
[PATCH 3/3] vDPA/ifcvf: bump version string to 1.0
This commit bumps ifcvf driver version string to 1.0 Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fd5befc5cbcc..56a0974cf93c 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -14,7 +14,7 @@ #include #include "ifcvf_base.h" -#define VERSION_STRING "0.1" +#define VERSION_STRING "1.0" #define DRIVER_AUTHOR "Intel Corporation" #define IFCVF_DRIVER_NAME "ifcvf" -- 2.27.0
[PATCH 1/3] vDPA/ifcvf: get_vendor_id returns a device specific vendor id
In this commit, ifcvf_get_vendor_id() will return a device specific vendor id of the probed pci device than a hard code. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index fa1af301cf55..e501ee07de17 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev) static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev) { - return IFCVF_SUBSYS_VENDOR_ID; + struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev); + struct pci_dev *pdev = adapter->pdev; + + return pdev->subsystem_vendor; } static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev) -- 2.27.0
[PATCH 0/3] vDPA/ifcvf: enables Intel C5000X-PL virtio-net
This series enabled Intel FGPA SmartNIC C5000X-PL virtio-net for vDPA Zhu Lingshan (3): vDPA/ifcvf: get_vendor_id returns a device specific vendor id vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA vDPA/ifcvf: bump version string to 1.0 drivers/vdpa/ifcvf/ifcvf_base.h | 13 + drivers/vdpa/ifcvf/ifcvf_main.c | 20 ++-- 2 files changed, 23 insertions(+), 10 deletions(-) -- 2.27.0
[PATCH RESEND 5/5] ifcvf: implement config interrupt in IFCVF
This commit implements config interrupt support in IFC VF Signed-off-by: Zhu Lingshan Acked-by: Jason Wang --- drivers/vdpa/ifcvf/ifcvf_base.c | 3 +++ drivers/vdpa/ifcvf/ifcvf_base.h | 4 drivers/vdpa/ifcvf/ifcvf_main.c | 23 ++- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c index e24371d..94bf032 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.c +++ b/drivers/vdpa/ifcvf/ifcvf_base.c @@ -185,6 +185,9 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status) void ifcvf_reset(struct ifcvf_hw *hw) { + hw->config_cb.callback = NULL; + hw->config_cb.private = NULL; + ifcvf_set_status(hw, 0); /* flush set_status, make sure VF is stopped, reset */ ifcvf_get_status(hw); diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h index e803070..f455441 100644 --- a/drivers/vdpa/ifcvf/ifcvf_base.h +++ b/drivers/vdpa/ifcvf/ifcvf_base.h @@ -27,6 +27,7 @@ ((1ULL << VIRTIO_NET_F_MAC) | \ (1ULL << VIRTIO_F_ANY_LAYOUT) | \ (1ULL << VIRTIO_F_VERSION_1) | \ +(1ULL << VIRTIO_NET_F_STATUS) | \ (1ULL << VIRTIO_F_ORDER_PLATFORM) | \ (1ULL << VIRTIO_F_IOMMU_PLATFORM) | \ (1ULL << VIRTIO_NET_F_MRG_RXBUF)) @@ -81,6 +82,9 @@ struct ifcvf_hw { void __iomem *net_cfg; struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2]; void __iomem * const *base; + char config_msix_name[256]; + struct vdpa_callback config_cb; + }; struct ifcvf_adapter { diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 63a6366..f5a60c1 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -18,6 +18,16 @@ #define DRIVER_AUTHOR "Intel Corporation" #define IFCVF_DRIVER_NAME "ifcvf" +static irqreturn_t ifcvf_config_changed(int irq, void *arg) +{ + struct ifcvf_hw *vf = arg; + + if (vf->config_cb.callback) + return vf->config_cb.callback(vf->config_cb.private); + + return IRQ_HANDLED; +} + static irqreturn_t ifcvf_intr_handler(int irq, void *arg) { struct vring_info *vring = arg; @@ -59,6 +69,14 @@ static int ifcvf_request_irq(struct ifcvf_adapter *adapter) return ret; } + snprintf(vf->config_msix_name, 256, "ifcvf[%s]-config\n", +pci_name(pdev)); + vector = 0; + irq = pci_irq_vector(pdev, vector); + ret = devm_request_irq(&pdev->dev, irq, + ifcvf_config_changed, 0, + vf->config_msix_name, vf); + for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) { snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n", pci_name(pdev), i); @@ -328,7 +346,10 @@ static void ifcvf_vdpa_set_config(struct vdpa_device *vdpa_dev, static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev, struct vdpa_callback *cb) { - /* We don't support config interrupt */ + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev); + + vf->config_cb.callback = cb->callback; + vf->config_cb.private = cb->private; } /* -- 1.8.3.1
[PATCH RESEND 3/5] vhost_vdpa: Support config interrupt in vdpa
This commit implements config interrupt support in vhost_vdpa layer. Signed-off-by: Zhu Lingshan Acked-by: Jason Wang --- drivers/vhost/vdpa.c | 47 ++ include/uapi/linux/vhost.h | 4 2 files changed, 51 insertions(+) diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index 0968361..ffb41cf 100644 --- a/drivers/vhost/vdpa.c +++ b/drivers/vhost/vdpa.c @@ -21,6 +21,7 @@ #include #include #include +#include #include "vhost.h" @@ -70,6 +71,7 @@ struct vhost_vdpa { int nvqs; int virtio_id; int minor; + struct eventfd_ctx *config_ctx; }; static DEFINE_IDA(vhost_vdpa_ida); @@ -101,6 +103,17 @@ static irqreturn_t vhost_vdpa_virtqueue_cb(void *private) return IRQ_HANDLED; } +static irqreturn_t vhost_vdpa_config_cb(void *private) +{ + struct vhost_vdpa *v = private; + struct eventfd_ctx *config_ctx = v->config_ctx; + + if (config_ctx) + eventfd_signal(config_ctx, 1); + + return IRQ_HANDLED; +} + static void vhost_vdpa_reset(struct vhost_vdpa *v) { struct vdpa_device *vdpa = v->vdpa; @@ -288,6 +301,36 @@ static long vhost_vdpa_get_vring_num(struct vhost_vdpa *v, u16 __user *argp) return 0; } +static void vhost_vdpa_config_put(struct vhost_vdpa *v) +{ + if (v->config_ctx) + eventfd_ctx_put(v->config_ctx); +} + +static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp) +{ + struct vdpa_callback cb; + int fd; + struct eventfd_ctx *ctx; + + cb.callback = vhost_vdpa_config_cb; + cb.private = v->vdpa; + if (copy_from_user(&fd, argp, sizeof(fd))) + return -EFAULT; + + ctx = fd == VHOST_FILE_UNBIND ? NULL : eventfd_ctx_fdget(fd); + swap(ctx, v->config_ctx); + + if (!IS_ERR_OR_NULL(ctx)) + eventfd_ctx_put(ctx); + + if (IS_ERR(v->config_ctx)) + return PTR_ERR(v->config_ctx); + + v->vdpa->config->set_config_cb(v->vdpa, &cb); + + return 0; +} static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd, void __user *argp) { @@ -395,6 +438,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep, case VHOST_SET_LOG_FD: r = -ENOIOCTLCMD; break; + case VHOST_VDPA_SET_CONFIG_CALL: + r = vhost_vdpa_set_config_call(v, argp); + break; default: r = vhost_dev_ioctl(&v->vdev, cmd, argp); if (r == -ENOIOCTLCMD) @@ -729,6 +775,7 @@ static int vhost_vdpa_release(struct inode *inode, struct file *filep) vhost_dev_stop(&v->vdev); vhost_vdpa_iotlb_free(v); vhost_vdpa_free_domain(v); + vhost_vdpa_config_put(v); vhost_dev_cleanup(&v->vdev); kfree(v->vdev.vqs); mutex_unlock(&d->mutex); diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h index 9fe72e4..0c23496 100644 --- a/include/uapi/linux/vhost.h +++ b/include/uapi/linux/vhost.h @@ -15,6 +15,8 @@ #include #include +#define VHOST_FILE_UNBIND -1 + /* ioctls */ #define VHOST_VIRTIO 0xAF @@ -140,4 +142,6 @@ /* Get the max ring size. */ #define VHOST_VDPA_GET_VRING_NUM _IOR(VHOST_VIRTIO, 0x76, __u16) +/* Set event fd for config interrupt*/ +#define VHOST_VDPA_SET_CONFIG_CALL _IOW(VHOST_VIRTIO, 0x77, int) #endif -- 1.8.3.1
[PATCH RESEND 2/5] ifcvf: ignore continuous setting same staus value
User space may try to set status of same value for multiple times, this patch can handle this case more efficiently by ignoring the same value of status settings. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index d529ed6..63a6366 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -179,6 +179,9 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status) adapter = dev_get_drvdata(vdpa_dev->dev.parent); status_old = ifcvf_get_status(vf); + if (status_old == status) + return; + if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && !(status & VIRTIO_CONFIG_S_DRIVER_OK)) { ifcvf_stop_datapath(adapter); -- 1.8.3.1
[PATCH RESEND 4/5] vhost: replace -1 with VHOST_FILE_UNBIND in iotcls
This commit replaces -1 with VHOST_FILE_UNBIND in ioctls since we have added such a macro in the uapi header for vdpa_host. Signed-off-by: Zhu Lingshan Acked-by: Jason Wang --- drivers/vhost/vhost.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index d450e16..8ba3ed2 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -1574,7 +1574,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg r = -EFAULT; break; } - eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd); + eventfp = f.fd == VHOST_FILE_UNBIND ? NULL : eventfd_fget(f.fd); if (IS_ERR(eventfp)) { r = PTR_ERR(eventfp); break; @@ -1590,7 +1590,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg r = -EFAULT; break; } - ctx = f.fd == -1 ? NULL : eventfd_ctx_fdget(f.fd); + ctx = f.fd == VHOST_FILE_UNBIND ? NULL : eventfd_ctx_fdget(f.fd); if (IS_ERR(ctx)) { r = PTR_ERR(ctx); break; @@ -1602,7 +1602,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg r = -EFAULT; break; } - ctx = f.fd == -1 ? NULL : eventfd_ctx_fdget(f.fd); + ctx = f.fd == VHOST_FILE_UNBIND ? NULL : eventfd_ctx_fdget(f.fd); if (IS_ERR(ctx)) { r = PTR_ERR(ctx); break; @@ -1727,7 +1727,7 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp) r = get_user(fd, (int __user *)argp); if (r < 0) break; - ctx = fd == -1 ? NULL : eventfd_ctx_fdget(fd); + ctx = fd == VHOST_FILE_UNBIND ? NULL : eventfd_ctx_fdget(fd); if (IS_ERR(ctx)) { r = PTR_ERR(ctx); break; -- 1.8.3.1
[PATCH RESEND 1/5] ifcvf: move IRQ request/free to status change handlers
To comply with VIRTIO spec, this commit intends to move IRQ request and free operations from probe() to device status changing handler, would handle datapath start/stop in the handler as well. VIRTIO spec 1.1, section 2.1.2 Device Requirements: Device Status Field The device MUST NOT consume buffers or send any used buffer notifications to the driver before DRIVER_OK. Signed-off-by: Zhu Lingshan Acked-by: Jason Wang --- drivers/vdpa/ifcvf/ifcvf_main.c | 120 1 file changed, 73 insertions(+), 47 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index abf6a061..d529ed6 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -28,6 +28,60 @@ static irqreturn_t ifcvf_intr_handler(int irq, void *arg) return IRQ_HANDLED; } +static void ifcvf_free_irq_vectors(void *data) +{ + pci_free_irq_vectors(data); +} + +static void ifcvf_free_irq(struct ifcvf_adapter *adapter, int queues) +{ + struct pci_dev *pdev = adapter->pdev; + struct ifcvf_hw *vf = &adapter->vf; + int i; + + + for (i = 0; i < queues; i++) + devm_free_irq(&pdev->dev, vf->vring[i].irq, &vf->vring[i]); + + ifcvf_free_irq_vectors(pdev); +} + +static int ifcvf_request_irq(struct ifcvf_adapter *adapter) +{ + struct pci_dev *pdev = adapter->pdev; + struct ifcvf_hw *vf = &adapter->vf; + int vector, i, ret, irq; + + ret = pci_alloc_irq_vectors(pdev, IFCVF_MAX_INTR, + IFCVF_MAX_INTR, PCI_IRQ_MSIX); + if (ret < 0) { + IFCVF_ERR(pdev, "Failed to alloc IRQ vectors\n"); + return ret; + } + + for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) { + snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n", +pci_name(pdev), i); + vector = i + IFCVF_MSI_QUEUE_OFF; + irq = pci_irq_vector(pdev, vector); + ret = devm_request_irq(&pdev->dev, irq, + ifcvf_intr_handler, 0, + vf->vring[i].msix_name, + &vf->vring[i]); + if (ret) { + IFCVF_ERR(pdev, + "Failed to request irq for vq %d\n", i); + ifcvf_free_irq(adapter, i); + + return ret; + } + + vf->vring[i].irq = irq; + } + + return 0; +} + static int ifcvf_start_datapath(void *private) { struct ifcvf_hw *vf = ifcvf_private_to_vf(private); @@ -118,17 +172,34 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status) { struct ifcvf_adapter *adapter; struct ifcvf_hw *vf; + u8 status_old; + int ret; vf = vdpa_to_vf(vdpa_dev); adapter = dev_get_drvdata(vdpa_dev->dev.parent); + status_old = ifcvf_get_status(vf); - if (status == 0) { + if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && + !(status & VIRTIO_CONFIG_S_DRIVER_OK)) { ifcvf_stop_datapath(adapter); + ifcvf_free_irq(adapter, IFCVF_MAX_QUEUE_PAIRS * 2); + } + + if (status == 0) { ifcvf_reset_vring(adapter); return; } - if (status & VIRTIO_CONFIG_S_DRIVER_OK) { + if ((status & VIRTIO_CONFIG_S_DRIVER_OK) && + !(status_old & VIRTIO_CONFIG_S_DRIVER_OK)) { + ret = ifcvf_request_irq(adapter); + if (ret) { + status = ifcvf_get_status(vf); + status |= VIRTIO_CONFIG_S_FAILED; + ifcvf_set_status(vf, status); + return; + } + if (ifcvf_start_datapath(adapter) < 0) IFCVF_ERR(adapter->pdev, "Failed to set ifcvf vdpa status %u\n", @@ -284,38 +355,6 @@ static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev, .set_config_cb = ifcvf_vdpa_set_config_cb, }; -static int ifcvf_request_irq(struct ifcvf_adapter *adapter) -{ - struct pci_dev *pdev = adapter->pdev; - struct ifcvf_hw *vf = &adapter->vf; - int vector, i, ret, irq; - - - for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) { - snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n", -pci_name(pdev), i); - vector = i + IFCVF_MSI_QUEUE_OFF; - irq = pci_irq_vector(pdev, vector); - ret = devm_request_irq(&pdev->dev, irq, - ifcvf_intr_handler, 0, - vf-
[PATCH RESEND 0/5] vDPA:config interrupt support and IRQ improvements
This series intends to introduce: (1) config interrupt in vhost_vdpa and IFCVF. (2) handle datapath and IRQ managent in the status handler, so that it would comply to virtio spec, and more reliable Most patches already got ACKed, excepted for ifcvf: ignore continuous setting same staus value Please help review Zhu Lingshan (5): ifcvf: move IRQ request/free to status change handlers ifcvf: ignore continuous setting same staus value vhost_vdpa: Support config interrupt in vhost_vdpa vhost: replace -1 with VHOST_FILE_UNBIND in iotcls ifcvf: implement config interrupt in IFCVF drivers/vdpa/ifcvf/ifcvf_base.c | 3 + drivers/vdpa/ifcvf/ifcvf_base.h | 4 ++ drivers/vdpa/ifcvf/ifcvf_main.c | 146 +++- drivers/vhost/vdpa.c| 47 + drivers/vhost/vhost.c | 8 +-- include/uapi/linux/vhost.h | 4 ++ 6 files changed, 160 insertions(+), 52 deletions(-) -- 1.8.3.1
[PATCH] vdpa: bypass waking up vhost_woker for vdpa vq kick
Standard vhost devices rely on waking up a vhost_worker to kick a virtquque. However vdpa devices have hardware backends, so it does not need this waking up routin. In this commit, vdpa device will kick a virtqueue directly, reduce the performance overhead caused by waking up a vhost_woker. Signed-off-by: Zhu Lingshan Suggested-by: Jason Wang --- drivers/vhost/vdpa.c | 100 +++ 1 file changed, 100 insertions(+) diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index 0968361..d3a2aca 100644 --- a/drivers/vhost/vdpa.c +++ b/drivers/vhost/vdpa.c @@ -287,6 +287,66 @@ static long vhost_vdpa_get_vring_num(struct vhost_vdpa *v, u16 __user *argp) return 0; } +void vhost_vdpa_poll_stop(struct vhost_virtqueue *vq) +{ + vhost_poll_stop(&vq->poll); +} + +int vhost_vdpa_poll_start(struct vhost_virtqueue *vq) +{ + struct vhost_poll *poll = &vq->poll; + struct file *file = vq->kick; + __poll_t mask; + + + if (poll->wqh) + return 0; + + mask = vfs_poll(file, &poll->table); + if (mask) + vq->handle_kick(&vq->poll.work); + if (mask & EPOLLERR) { + vhost_poll_stop(poll); + return -EINVAL; + } + + return 0; +} + +static long vhost_vdpa_set_vring_kick(struct vhost_virtqueue *vq, + void __user *argp) +{ + bool pollstart = false, pollstop = false; + struct file *eventfp, *filep = NULL; + struct vhost_vring_file f; + long r; + + if (copy_from_user(&f, argp, sizeof(f))) + return -EFAULT; + + eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd); + if (IS_ERR(eventfp)) { + r = PTR_ERR(eventfp); + return r; + } + + if (eventfp != vq->kick) { + pollstop = (filep = vq->kick) != NULL; + pollstart = (vq->kick = eventfp) != NULL; + } else + filep = eventfp; + + if (pollstop && vq->handle_kick) + vhost_vdpa_poll_stop(vq); + + if (filep) + fput(filep); + + if (pollstart && vq->handle_kick) + r = vhost_vdpa_poll_start(vq); + + return r; +} static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd, void __user *argp) @@ -316,6 +376,11 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd, return 0; } + if (cmd == VHOST_SET_VRING_KICK) { + r = vhost_vdpa_set_vring_kick(vq, argp); + return r; + } + if (cmd == VHOST_GET_VRING_BASE) vq->last_avail_idx = ops->get_vq_state(v->vdpa, idx); @@ -667,6 +732,39 @@ static void vhost_vdpa_free_domain(struct vhost_vdpa *v) v->domain = NULL; } +static int vhost_vdpa_poll_worker(wait_queue_entry_t *wait, unsigned int mode, + int sync, void *key) +{ + struct vhost_poll *poll = container_of(wait, struct vhost_poll, wait); + struct vhost_virtqueue *vq = container_of(poll, struct vhost_virtqueue, + poll); + + if (!(key_to_poll(key) & poll->mask)) + return 0; + + vq->handle_kick(&vq->poll.work); + + return 0; +} + +void vhost_vdpa_poll_init(struct vhost_dev *dev) +{ + struct vhost_virtqueue *vq; + struct vhost_poll *poll; + int i; + + for (i = 0; i < dev->nvqs; i++) { + vq = dev->vqs[i]; + poll = &vq->poll; + if (vq->handle_kick) { + init_waitqueue_func_entry(&poll->wait, + vhost_vdpa_poll_worker); + poll->work.fn = vq->handle_kick; + } + + } +} + static int vhost_vdpa_open(struct inode *inode, struct file *filep) { struct vhost_vdpa *v; @@ -697,6 +795,8 @@ static int vhost_vdpa_open(struct inode *inode, struct file *filep) vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, vhost_vdpa_process_iotlb_msg); + vhost_vdpa_poll_init(dev); + dev->iotlb = vhost_iotlb_alloc(0, 0); if (!dev->iotlb) { r = -ENOMEM; -- 1.8.3.1
[PATCH V2] ifcvf: move IRQ request/free to status change handlers
This commit move IRQ request and free operations from probe() to VIRTIO status change handler to comply with VIRTIO spec. VIRTIO spec 1.1, section 2.1.2 Device Requirements: Device Status Field The device MUST NOT consume buffers or send any used buffer notifications to the driver before DRIVER_OK. Signed-off-by: Zhu Lingshan --- changes from V1: remove ifcvf_stop_datapath() in status == 0 handler, we don't need to do this twice; handle status == 0 after DRIVER_OK -> !DRIVER_OK handler (Jason Wang) drivers/vdpa/ifcvf/ifcvf_main.c | 120 1 file changed, 73 insertions(+), 47 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index abf6a061..d529ed6 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -28,6 +28,60 @@ static irqreturn_t ifcvf_intr_handler(int irq, void *arg) return IRQ_HANDLED; } +static void ifcvf_free_irq_vectors(void *data) +{ + pci_free_irq_vectors(data); +} + +static void ifcvf_free_irq(struct ifcvf_adapter *adapter, int queues) +{ + struct pci_dev *pdev = adapter->pdev; + struct ifcvf_hw *vf = &adapter->vf; + int i; + + + for (i = 0; i < queues; i++) + devm_free_irq(&pdev->dev, vf->vring[i].irq, &vf->vring[i]); + + ifcvf_free_irq_vectors(pdev); +} + +static int ifcvf_request_irq(struct ifcvf_adapter *adapter) +{ + struct pci_dev *pdev = adapter->pdev; + struct ifcvf_hw *vf = &adapter->vf; + int vector, i, ret, irq; + + ret = pci_alloc_irq_vectors(pdev, IFCVF_MAX_INTR, + IFCVF_MAX_INTR, PCI_IRQ_MSIX); + if (ret < 0) { + IFCVF_ERR(pdev, "Failed to alloc IRQ vectors\n"); + return ret; + } + + for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) { + snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n", +pci_name(pdev), i); + vector = i + IFCVF_MSI_QUEUE_OFF; + irq = pci_irq_vector(pdev, vector); + ret = devm_request_irq(&pdev->dev, irq, + ifcvf_intr_handler, 0, + vf->vring[i].msix_name, + &vf->vring[i]); + if (ret) { + IFCVF_ERR(pdev, + "Failed to request irq for vq %d\n", i); + ifcvf_free_irq(adapter, i); + + return ret; + } + + vf->vring[i].irq = irq; + } + + return 0; +} + static int ifcvf_start_datapath(void *private) { struct ifcvf_hw *vf = ifcvf_private_to_vf(private); @@ -118,17 +172,34 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status) { struct ifcvf_adapter *adapter; struct ifcvf_hw *vf; + u8 status_old; + int ret; vf = vdpa_to_vf(vdpa_dev); adapter = dev_get_drvdata(vdpa_dev->dev.parent); + status_old = ifcvf_get_status(vf); - if (status == 0) { + if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && + !(status & VIRTIO_CONFIG_S_DRIVER_OK)) { ifcvf_stop_datapath(adapter); + ifcvf_free_irq(adapter, IFCVF_MAX_QUEUE_PAIRS * 2); + } + + if (status == 0) { ifcvf_reset_vring(adapter); return; } - if (status & VIRTIO_CONFIG_S_DRIVER_OK) { + if ((status & VIRTIO_CONFIG_S_DRIVER_OK) && + !(status_old & VIRTIO_CONFIG_S_DRIVER_OK)) { + ret = ifcvf_request_irq(adapter); + if (ret) { + status = ifcvf_get_status(vf); + status |= VIRTIO_CONFIG_S_FAILED; + ifcvf_set_status(vf, status); + return; + } + if (ifcvf_start_datapath(adapter) < 0) IFCVF_ERR(adapter->pdev, "Failed to set ifcvf vdpa status %u\n", @@ -284,38 +355,6 @@ static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev, .set_config_cb = ifcvf_vdpa_set_config_cb, }; -static int ifcvf_request_irq(struct ifcvf_adapter *adapter) -{ - struct pci_dev *pdev = adapter->pdev; - struct ifcvf_hw *vf = &adapter->vf; - int vector, i, ret, irq; - - - for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) { - snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n", -pci_name(pdev), i); - vector = i + IFCVF_MSI_QUEUE_OFF; - irq = pci_irq_vector(pdev, vector); - ret = devm_request_irq(&pdev->dev, irq, -
[PATCH] ifcvf: move IRQ request/free to status change handlers
This commit move IRQ request and free operations from probe() to VIRTIO status change handler to comply with VIRTIO spec. VIRTIO spec 1.1, section 2.1.2 Device Requirements: Device Status Field The device MUST NOT consume buffers or send any used buffer notifications to the driver before DRIVER_OK. Signed-off-by: Zhu Lingshan --- drivers/vdpa/ifcvf/ifcvf_main.c | 119 1 file changed, 73 insertions(+), 46 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index abf6a061..4d58bf2 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -28,6 +28,60 @@ static irqreturn_t ifcvf_intr_handler(int irq, void *arg) return IRQ_HANDLED; } +static void ifcvf_free_irq_vectors(void *data) +{ + pci_free_irq_vectors(data); +} + +static void ifcvf_free_irq(struct ifcvf_adapter *adapter, int queues) +{ + struct pci_dev *pdev = adapter->pdev; + struct ifcvf_hw *vf = &adapter->vf; + int i; + + + for (i = 0; i < queues; i++) + devm_free_irq(&pdev->dev, vf->vring[i].irq, &vf->vring[i]); + + ifcvf_free_irq_vectors(pdev); +} + +static int ifcvf_request_irq(struct ifcvf_adapter *adapter) +{ + struct pci_dev *pdev = adapter->pdev; + struct ifcvf_hw *vf = &adapter->vf; + int vector, i, ret, irq; + + ret = pci_alloc_irq_vectors(pdev, IFCVF_MAX_INTR, + IFCVF_MAX_INTR, PCI_IRQ_MSIX); + if (ret < 0) { + IFCVF_ERR(pdev, "Failed to alloc IRQ vectors\n"); + return ret; + } + + for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) { + snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n", +pci_name(pdev), i); + vector = i + IFCVF_MSI_QUEUE_OFF; + irq = pci_irq_vector(pdev, vector); + ret = devm_request_irq(&pdev->dev, irq, + ifcvf_intr_handler, 0, + vf->vring[i].msix_name, + &vf->vring[i]); + if (ret) { + IFCVF_ERR(pdev, + "Failed to request irq for vq %d\n", i); + ifcvf_free_irq(adapter, i); + + return ret; + } + + vf->vring[i].irq = irq; + } + + return 0; +} + static int ifcvf_start_datapath(void *private) { struct ifcvf_hw *vf = ifcvf_private_to_vf(private); @@ -118,9 +172,12 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status) { struct ifcvf_adapter *adapter; struct ifcvf_hw *vf; + u8 status_old; + int ret; vf = vdpa_to_vf(vdpa_dev); adapter = dev_get_drvdata(vdpa_dev->dev.parent); + status_old = ifcvf_get_status(vf); if (status == 0) { ifcvf_stop_datapath(adapter); @@ -128,7 +185,22 @@ static void ifcvf_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status) return; } - if (status & VIRTIO_CONFIG_S_DRIVER_OK) { + if ((status_old & VIRTIO_CONFIG_S_DRIVER_OK) && + !(status & VIRTIO_CONFIG_S_DRIVER_OK)) { + ifcvf_stop_datapath(adapter); + ifcvf_free_irq(adapter, IFCVF_MAX_QUEUE_PAIRS * 2); + } + + if ((status & VIRTIO_CONFIG_S_DRIVER_OK) && + !(status_old & VIRTIO_CONFIG_S_DRIVER_OK)) { + ret = ifcvf_request_irq(adapter); + if (ret) { + status = ifcvf_get_status(vf); + status |= VIRTIO_CONFIG_S_FAILED; + ifcvf_set_status(vf, status); + return; + } + if (ifcvf_start_datapath(adapter) < 0) IFCVF_ERR(adapter->pdev, "Failed to set ifcvf vdpa status %u\n", @@ -284,38 +356,6 @@ static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev, .set_config_cb = ifcvf_vdpa_set_config_cb, }; -static int ifcvf_request_irq(struct ifcvf_adapter *adapter) -{ - struct pci_dev *pdev = adapter->pdev; - struct ifcvf_hw *vf = &adapter->vf; - int vector, i, ret, irq; - - - for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) { - snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n", -pci_name(pdev), i); - vector = i + IFCVF_MSI_QUEUE_OFF; - irq = pci_irq_vector(pdev, vector); - ret = devm_request_irq(&pdev->dev, irq, - ifcvf_intr_handler, 0, - vf->vring[i].msix_name
Re: [RFC 2/2] vhost: IFC VF vdpa layer
On 10/22/2019 2:53 PM, Zhu Lingshan wrote: On 10/21/2019 6:19 PM, Jason Wang wrote: On 2019/10/21 下午5:53, Zhu, Lingshan wrote: On 10/16/2019 6:19 PM, Jason Wang wrote: On 2019/10/16 上午9:30, Zhu Lingshan wrote: This commit introduced IFC VF operations for vdpa, which complys to vhost_mdev interfaces, handles IFC VF initialization, configuration and removal. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 1 file changed, 541 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c diff --git a/drivers/vhost/ifcvf/ifcvf_main.c b/drivers/vhost/ifcvf/ifcvf_main.c new file mode 100644 index ..c48a29969a85 --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_main.c @@ -0,0 +1,541 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include +#include +#include +#include +#include + +#include "ifcvf_base.h" + +#define VERSION_STRING "0.1" +#define DRIVER_AUTHOR "Intel Corporation" +#define IFCVF_DRIVER_NAME "ifcvf" + +static irqreturn_t ifcvf_intr_handler(int irq, void *arg) +{ + struct vring_info *vring = arg; + + if (vring->cb.callback) + return vring->cb.callback(vring->cb.private); + + return IRQ_HANDLED; +} + +static u64 ifcvf_mdev_get_features(struct mdev_device *mdev) +{ + return IFC_SUPPORTED_FEATURES; I would expect this should be done by querying the hw. Or IFC VF can't get any update through its firmware? Hi Jason, Thanks for your comments, for now driver just support these features. Ok, it should work but less flexible, we can change it in the future. sure! +} + +static int ifcvf_mdev_set_features(struct mdev_device *mdev, u64 features) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->req_features = features; + + return 0; +} + +static u64 ifcvf_mdev_get_vq_state(struct mdev_device *mdev, u16 qid) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].last_avail_idx; Does this really work? I'd expect it should be fetched from hw since it's an internal state. for now, it's working, we intend to support LM in next version drivers. I'm not sure I understand here, I don't see any synchronization between the hardware and last_avail_idx, so last_avail_idx should not change. Btw, what did "LM" mean :) ? I can add bar IO operations here, LM = live migration, sorry for the abbreviation. +} + +static int ifcvf_mdev_set_vq_state(struct mdev_device *mdev, u16 qid, u64 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].last_used_idx = num; I fail to understand why last_used_idx is needed. It looks to me the used idx in the used ring is sufficient. I will remove it. + vf->vring[qid].last_avail_idx = num; Do we need a synchronization with hw immediately here? + + return 0; +} + +static int ifcvf_mdev_set_vq_address(struct mdev_device *mdev, u16 idx, + u64 desc_area, u64 driver_area, + u64 device_area) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].desc = desc_area; + vf->vring[idx].avail = driver_area; + vf->vring[idx].used = device_area; + + return 0; +} + +static void ifcvf_mdev_set_vq_num(struct mdev_device *mdev, u16 qid, u32 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].size = num; +} + +static void ifcvf_mdev_set_vq_ready(struct mdev_device *mdev, + u16 qid, bool ready) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].ready = ready; +} + +static bool ifcvf_mdev_get_vq_ready(struct mdev_device *mdev, u16 qid) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].ready; +} + +static void ifcvf_mdev_set_vq_cb(struct mdev_device *mdev, u16 idx, + struct virtio_mdev_callback *cb) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].cb = *cb; +} + +static void ifcvf_mdev_kick_vq(struct mdev_device *mdev, u16 idx) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + ifcvf_notify_queue(vf, idx); +} + +static u8 ifcvf_mdev_get_status(struct mdev_device *mdev) +{ + st
Re: [RFC 2/2] vhost: IFC VF vdpa layer
On 10/22/2019 9:05 PM, Jason Wang wrote: On 2019/10/22 下午2:53, Zhu Lingshan wrote: On 10/21/2019 6:19 PM, Jason Wang wrote: On 2019/10/21 下午5:53, Zhu, Lingshan wrote: On 10/16/2019 6:19 PM, Jason Wang wrote: On 2019/10/16 上午9:30, Zhu Lingshan wrote: This commit introduced IFC VF operations for vdpa, which complys to vhost_mdev interfaces, handles IFC VF initialization, configuration and removal. Signed-off-by: Zhu Lingshan --- [...] +} + +static int ifcvf_mdev_set_features(struct mdev_device *mdev, u64 features) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->req_features = features; + + return 0; +} + +static u64 ifcvf_mdev_get_vq_state(struct mdev_device *mdev, u16 qid) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].last_avail_idx; Does this really work? I'd expect it should be fetched from hw since it's an internal state. for now, it's working, we intend to support LM in next version drivers. I'm not sure I understand here, I don't see any synchronization between the hardware and last_avail_idx, so last_avail_idx should not change. Btw, what did "LM" mean :) ? I can add bar IO operations here, LM = live migration, sorry for the abbreviation. Just make sure I understand here, I believe you mean reading last_avail_idx through IO bar here? Thanks Hi Jason, Yes, I mean last_avail_idx. is that correct? THanks
Re: [RFC 2/2] vhost: IFC VF vdpa layer
On 10/21/2019 6:19 PM, Jason Wang wrote: On 2019/10/21 下午5:53, Zhu, Lingshan wrote: On 10/16/2019 6:19 PM, Jason Wang wrote: On 2019/10/16 上午9:30, Zhu Lingshan wrote: This commit introduced IFC VF operations for vdpa, which complys to vhost_mdev interfaces, handles IFC VF initialization, configuration and removal. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 1 file changed, 541 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c diff --git a/drivers/vhost/ifcvf/ifcvf_main.c b/drivers/vhost/ifcvf/ifcvf_main.c new file mode 100644 index ..c48a29969a85 --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_main.c @@ -0,0 +1,541 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include +#include +#include +#include +#include + +#include "ifcvf_base.h" + +#define VERSION_STRING "0.1" +#define DRIVER_AUTHOR "Intel Corporation" +#define IFCVF_DRIVER_NAME "ifcvf" + +static irqreturn_t ifcvf_intr_handler(int irq, void *arg) +{ + struct vring_info *vring = arg; + + if (vring->cb.callback) + return vring->cb.callback(vring->cb.private); + + return IRQ_HANDLED; +} + +static u64 ifcvf_mdev_get_features(struct mdev_device *mdev) +{ + return IFC_SUPPORTED_FEATURES; I would expect this should be done by querying the hw. Or IFC VF can't get any update through its firmware? Hi Jason, Thanks for your comments, for now driver just support these features. Ok, it should work but less flexible, we can change it in the future. sure! +} + +static int ifcvf_mdev_set_features(struct mdev_device *mdev, u64 features) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->req_features = features; + + return 0; +} + +static u64 ifcvf_mdev_get_vq_state(struct mdev_device *mdev, u16 qid) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].last_avail_idx; Does this really work? I'd expect it should be fetched from hw since it's an internal state. for now, it's working, we intend to support LM in next version drivers. I'm not sure I understand here, I don't see any synchronization between the hardware and last_avail_idx, so last_avail_idx should not change. Btw, what did "LM" mean :) ? I can add bar IO operations here, LM = live migration, sorry for the abbreviation. +} + +static int ifcvf_mdev_set_vq_state(struct mdev_device *mdev, u16 qid, u64 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].last_used_idx = num; I fail to understand why last_used_idx is needed. It looks to me the used idx in the used ring is sufficient. I will remove it. + vf->vring[qid].last_avail_idx = num; Do we need a synchronization with hw immediately here? + + return 0; +} + +static int ifcvf_mdev_set_vq_address(struct mdev_device *mdev, u16 idx, + u64 desc_area, u64 driver_area, + u64 device_area) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].desc = desc_area; + vf->vring[idx].avail = driver_area; + vf->vring[idx].used = device_area; + + return 0; +} + +static void ifcvf_mdev_set_vq_num(struct mdev_device *mdev, u16 qid, u32 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].size = num; +} + +static void ifcvf_mdev_set_vq_ready(struct mdev_device *mdev, + u16 qid, bool ready) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].ready = ready; +} + +static bool ifcvf_mdev_get_vq_ready(struct mdev_device *mdev, u16 qid) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].ready; +} + +static void ifcvf_mdev_set_vq_cb(struct mdev_device *mdev, u16 idx, + struct virtio_mdev_callback *cb) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].cb = *cb; +} + +static void ifcvf_mdev_kick_vq(struct mdev_device *mdev, u16 idx) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + ifcvf_notify_queue(vf, idx); +} + +static u8 ifcvf_mdev_get_status(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter =
Re: [RFC 1/2] vhost: IFC VF hardware operation layer
On 10/22/2019 9:32 AM, Jason Wang wrote: On 2019/10/22 上午12:31, Simon Horman wrote: On Mon, Oct 21, 2019 at 05:55:33PM +0800, Zhu, Lingshan wrote: On 10/16/2019 5:53 PM, Simon Horman wrote: Hi Zhu, thanks for your patch. On Wed, Oct 16, 2019 at 09:10:40AM +0800, Zhu Lingshan wrote: ... +static void ifcvf_read_dev_config(struct ifcvf_hw *hw, u64 offset, + void *dst, int length) +{ + int i; + u8 *p; + u8 old_gen, new_gen; + + do { + old_gen = ioread8(&hw->common_cfg->config_generation); + + p = dst; + for (i = 0; i < length; i++) + *p++ = ioread8((u8 *)hw->dev_cfg + offset + i); + + new_gen = ioread8(&hw->common_cfg->config_generation); + } while (old_gen != new_gen); Would it be wise to limit the number of iterations of the loop above? Thanks but I don't quite get it. This is used to make sure the function would get the latest config. I am worried about the possibility that it will loop forever. Could that happen? ... My understanding is that the function here is similar to virtio config generation [1]. So this can only happen for a buggy hardware. Thanks [1] https://docs.oasis-open.org/virtio/virtio/v1.1/csprd01/virtio-v1.1-csprd01.html Section 2.4.1 Yes! +static void io_write64_twopart(u64 val, u32 *lo, u32 *hi) +{ + iowrite32(val & ((1ULL << 32) - 1), lo); + iowrite32(val >> 32, hi); +} I see this macro is also in virtio_pci_modern.c Assuming lo and hi aren't guaranteed to be sequential and thus iowrite64_hi_lo() cannot be used perhaps it would be good to add a common helper somewhere. Thanks, I will try after this IFC patchwork, I will cc you. Thanks. ...
Re: [RFC 1/2] vhost: IFC VF hardware operation layer
On 10/16/2019 4:40 PM, Jason Wang wrote: On 2019/10/16 上午9:30, Zhu Lingshan wrote: This commit introduced ifcvf_base layer, which handles IFC VF NIC hardware operations and configurations. It's better to describe the difference between ifc vf and virtio in the commit log or is there a open doc for this? Hi Jason, Sure, I will split these code into small patches with detailed commit logs in v1 patchset. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_base.c | 390 +++ drivers/vhost/ifcvf/ifcvf_base.h | 137 ++ 2 files changed, 527 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_base.c create mode 100644 drivers/vhost/ifcvf/ifcvf_base.h diff --git a/drivers/vhost/ifcvf/ifcvf_base.c b/drivers/vhost/ifcvf/ifcvf_base.c new file mode 100644 index ..b85e14c9bdcf --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_base.c @@ -0,0 +1,390 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include "ifcvf_base.h" + +static void *get_cap_addr(struct ifcvf_hw *hw, struct virtio_pci_cap *cap) +{ + u8 bar = cap->bar; + u32 length = cap->length; + u32 offset = cap->offset; + struct ifcvf_adapter *ifcvf = + container_of(hw, struct ifcvf_adapter, vf); + + if (bar >= IFCVF_PCI_MAX_RESOURCE) { + IFC_ERR(ifcvf->dev, + "Invalid bar number %u to get capabilities.\n", bar); + return NULL; + } + + if (offset + length < offset) { + IFC_ERR(ifcvf->dev, "offset(%u) + length(%u) overflows\n", + offset, length); + return NULL; + } + + if (offset + length > hw->mem_resource[cap->bar].len) { + IFC_ERR(ifcvf->dev, + "offset(%u) + len(%u) overflows bar%u to get capabilities.\n", + offset, length, bar); + return NULL; + } + + return hw->mem_resource[bar].addr + offset; +} + +int ifcvf_read_config_range(struct pci_dev *dev, + uint32_t *val, int size, int where) +{ + int i; + + for (i = 0; i < size; i += 4) { + if (pci_read_config_dword(dev, where + i, val + i / 4) < 0) + return -1; + } + return 0; +} + +int ifcvf_init_hw(struct ifcvf_hw *hw, struct pci_dev *dev) +{ + int ret; + u8 pos; + struct virtio_pci_cap cap; + u32 i; + u16 notify_off; + + ret = pci_read_config_byte(dev, PCI_CAPABILITY_LIST, &pos); + + if (ret < 0) { + IFC_ERR(&dev->dev, "failed to read PCI capability list.\n"); + return -EIO; + } + + while (pos) { + ret = ifcvf_read_config_range(dev, (u32 *)&cap, + sizeof(cap), pos); + + if (ret < 0) { + IFC_ERR(&dev->dev, "failed to get PCI capability at %x", + pos); + break; + } + + if (cap.cap_vndr != PCI_CAP_ID_VNDR) + goto next; + + IFC_INFO(&dev->dev, "read PCI config:\n" + "config type: %u.\n" + "PCI bar: %u.\n" + "PCI bar offset: %u.\n" + "PCI config len: %u.\n", + cap.cfg_type, cap.bar, + cap.offset, cap.length); + + switch (cap.cfg_type) { + case VIRTIO_PCI_CAP_COMMON_CFG: + hw->common_cfg = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->common_cfg = %p.\n", + hw->common_cfg); + break; + case VIRTIO_PCI_CAP_NOTIFY_CFG: + pci_read_config_dword(dev, pos + sizeof(cap), + &hw->notify_off_multiplier); + hw->notify_bar = cap.bar; + hw->notify_base = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->notify_base = %p.\n", + hw->notify_base); + break; + case VIRTIO_PCI_CAP_ISR_CFG: + hw->isr = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->isr = %p.\n", hw->isr); + break; + case VIRTIO_PCI_CAP_DEVICE_CFG: + hw->dev_cfg = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->dev_cfg = %p.\n", hw->dev_cfg); + break; + } +next: + pos = cap.cap_next; + } + + if (hw->common_cfg == NULL || hw->notify_base == NULL || + hw->isr == NULL || hw->dev_cfg == NULL) { + IFC_ERR(&dev->dev, "Incomplete PCI capabilities.\n"); + return -1; + } + + for (i = 0; i < (IFCVF_MAX_QUEUE_PAIRS * 2); i++) { Any reason for using hard coded queue pairs limit other than the max_queue_pairs in the net config? Hi Jason, Thanks for your kindly comments
Re: [RFC 1/2] vhost: IFC VF hardware operation layer
On 10/16/2019 4:45 PM, Jason Wang wrote: On 2019/10/16 上午9:30, Zhu Lingshan wrote: + */ +#define IFCVF_TRANSPORT_F_START 28 +#define IFCVF_TRANSPORT_F_END 34 + +#define IFC_SUPPORTED_FEATURES \ + ((1ULL << VIRTIO_NET_F_MAC) | \ + (1ULL << VIRTIO_F_ANY_LAYOUT) | \ + (1ULL << VIRTIO_F_VERSION_1) | \ + (1ULL << VHOST_F_LOG_ALL) | \ Let's avoid using VHOST_F_LOG_ALL, using the get_mdev_features() instead. Thanks, I will remove VHOST_F_LOG_ALL + (1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE) | \ + (1ULL << VIRTIO_NET_F_CTRL_VQ) | \ + (1ULL << VIRTIO_NET_F_STATUS) | \ + (1ULL << VIRTIO_NET_F_MRG_RXBUF)) /* not fully supported */ Why not having VIRTIO_F_IOMMU_PLATFORM and VIRTIO_F_ORDER_PLATFORM? I will add VIRTIO_F_ORDER_PLATFORM, for VIRTIO_F_IOMMU_PLATFORM, if we add this bit, QEMU may enable viommu, can cause troubles in LM (through we don't support LM in this version driver) Thanks, BR Zhu Lingshan Thanks
Re: [RFC 1/2] vhost: IFC VF hardware operation layer
On 10/16/2019 5:53 PM, Simon Horman wrote: Hi Zhu, thanks for your patch. On Wed, Oct 16, 2019 at 09:10:40AM +0800, Zhu Lingshan wrote: This commit introduced ifcvf_base layer, which handles IFC VF NIC hardware operations and configurations. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_base.c | 390 +++ drivers/vhost/ifcvf/ifcvf_base.h | 137 ++ 2 files changed, 527 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_base.c create mode 100644 drivers/vhost/ifcvf/ifcvf_base.h diff --git a/drivers/vhost/ifcvf/ifcvf_base.c b/drivers/vhost/ifcvf/ifcvf_base.c new file mode 100644 index ..b85e14c9bdcf --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_base.c @@ -0,0 +1,390 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include "ifcvf_base.h" + +static void *get_cap_addr(struct ifcvf_hw *hw, struct virtio_pci_cap *cap) +{ + u8 bar = cap->bar; + u32 length = cap->length; + u32 offset = cap->offset; The type of the length and offset fields of virtio_pci_cap is __le32. A conversion, such as le32_to_cpu(), is required in order to access these fields in host byte order. Hi Simon, Thanks for your comments, will do this. + struct ifcvf_adapter *ifcvf = + container_of(hw, struct ifcvf_adapter, vf); As mentioned elsewhere, please use reverse Christmas tree to order local variables throughout the patch. Please consider declaring and assigning ifcvf on separate lines if combining these stretches over more than one line. sure, will do. + + if (bar >= IFCVF_PCI_MAX_RESOURCE) { + IFC_ERR(ifcvf->dev, + "Invalid bar number %u to get capabilities.\n", bar); + return NULL; + } + + if (offset + length < offset) { + IFC_ERR(ifcvf->dev, "offset(%u) + length(%u) overflows\n", + offset, length); + return NULL; + } + + if (offset + length > hw->mem_resource[cap->bar].len) { + IFC_ERR(ifcvf->dev, + "offset(%u) + len(%u) overflows bar%u to get capabilities.\n", + offset, length, bar); + return NULL; + } + + return hw->mem_resource[bar].addr + offset; +} + +int ifcvf_read_config_range(struct pci_dev *dev, + uint32_t *val, int size, int where) +{ + int i; + + for (i = 0; i < size; i += 4) { + if (pci_read_config_dword(dev, where + i, val + i / 4) < 0) + return -1; + } I don't think the {} is needed here. An error code, such as -EINVAL, should be returned rather than -1. Likewise elsewhere. sure, will do + return 0; +} + +int ifcvf_init_hw(struct ifcvf_hw *hw, struct pci_dev *dev) +{ + int ret; + u8 pos; + struct virtio_pci_cap cap; + u32 i; + u16 notify_off; + + ret = pci_read_config_byte(dev, PCI_CAPABILITY_LIST, &pos); + + if (ret < 0) { + IFC_ERR(&dev->dev, "failed to read PCI capability list.\n"); + return -EIO; + } + + while (pos) { + ret = ifcvf_read_config_range(dev, (u32 *)&cap, + sizeof(cap), pos); sizeof should be white-space aligned vertically with dev. Likewise elsewhere. this is to avoid 80 chars limitation, I will try a better indent for this and the followings. + + if (ret < 0) { + IFC_ERR(&dev->dev, "failed to get PCI capability at %x", + pos); + break; + } + + if (cap.cap_vndr != PCI_CAP_ID_VNDR) + goto next; + + IFC_INFO(&dev->dev, "read PCI config:\n" + "config type: %u.\n" + "PCI bar: %u.\n" + "PCI bar offset: %u.\n" + "PCI config len: %u.\n", + cap.cfg_type, cap.bar, + cap.offset, cap.length); + + switch (cap.cfg_type) { + case VIRTIO_PCI_CAP_COMMON_CFG: + hw->common_cfg = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->common_cfg = %p.\n", + hw->common_cfg); + break; + case VIRTIO_PCI_CAP_NOTIFY_CFG: + pci_read_config_dword(dev, pos + sizeof(cap), + &hw->notify_off_multiplier); +
Re: [RFC 2/2] vhost: IFC VF vdpa layer
On 10/16/2019 6:19 PM, Jason Wang wrote: On 2019/10/16 上午9:30, Zhu Lingshan wrote: This commit introduced IFC VF operations for vdpa, which complys to vhost_mdev interfaces, handles IFC VF initialization, configuration and removal. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 1 file changed, 541 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c diff --git a/drivers/vhost/ifcvf/ifcvf_main.c b/drivers/vhost/ifcvf/ifcvf_main.c new file mode 100644 index ..c48a29969a85 --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_main.c @@ -0,0 +1,541 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include +#include +#include +#include +#include + +#include "ifcvf_base.h" + +#define VERSION_STRING "0.1" +#define DRIVER_AUTHOR "Intel Corporation" +#define IFCVF_DRIVER_NAME "ifcvf" + +static irqreturn_t ifcvf_intr_handler(int irq, void *arg) +{ + struct vring_info *vring = arg; + + if (vring->cb.callback) + return vring->cb.callback(vring->cb.private); + + return IRQ_HANDLED; +} + +static u64 ifcvf_mdev_get_features(struct mdev_device *mdev) +{ + return IFC_SUPPORTED_FEATURES; I would expect this should be done by querying the hw. Or IFC VF can't get any update through its firmware? Hi Jason, Thanks for your comments, for now driver just support these features. +} + +static int ifcvf_mdev_set_features(struct mdev_device *mdev, u64 features) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->req_features = features; + + return 0; +} + +static u64 ifcvf_mdev_get_vq_state(struct mdev_device *mdev, u16 qid) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].last_avail_idx; Does this really work? I'd expect it should be fetched from hw since it's an internal state. for now, it's working, we intend to support LM in next version drivers. +} + +static int ifcvf_mdev_set_vq_state(struct mdev_device *mdev, u16 qid, u64 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].last_used_idx = num; I fail to understand why last_used_idx is needed. It looks to me the used idx in the used ring is sufficient. I will remove it. + vf->vring[qid].last_avail_idx = num; Do we need a synchronization with hw immediately here? + + return 0; +} + +static int ifcvf_mdev_set_vq_address(struct mdev_device *mdev, u16 idx, + u64 desc_area, u64 driver_area, + u64 device_area) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].desc = desc_area; + vf->vring[idx].avail = driver_area; + vf->vring[idx].used = device_area; + + return 0; +} + +static void ifcvf_mdev_set_vq_num(struct mdev_device *mdev, u16 qid, u32 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].size = num; +} + +static void ifcvf_mdev_set_vq_ready(struct mdev_device *mdev, + u16 qid, bool ready) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].ready = ready; +} + +static bool ifcvf_mdev_get_vq_ready(struct mdev_device *mdev, u16 qid) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].ready; +} + +static void ifcvf_mdev_set_vq_cb(struct mdev_device *mdev, u16 idx, + struct virtio_mdev_callback *cb) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].cb = *cb; +} + +static void ifcvf_mdev_kick_vq(struct mdev_device *mdev, u16 idx) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + ifcvf_notify_queue(vf, idx); +} + +static u8 ifcvf_mdev_get_status(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->status; +} + +static u32 ifcvf_mdev_get_generation(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->generation; +} + +static int ifcvf_mdev_get_version(struct mdev_device *mdev) +{ + return VIRTIO_MDEV_VERSION; +} + +static u32 ifcvf_mdev_get_device_i
Re: [RFC 2/2] vhost: IFC VF vdpa layer
On 10/16/2019 5:53 PM, Simon Horman wrote: Hi Zhu, thanks for your patch. On Wed, Oct 16, 2019 at 09:03:18AM +0800, Zhu Lingshan wrote: This commit introduced IFC VF operations for vdpa, which complys to vhost_mdev interfaces, handles IFC VF initialization, configuration and removal. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 1 file changed, 541 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c diff --git a/drivers/vhost/ifcvf/ifcvf_main.c b/drivers/vhost/ifcvf/ifcvf_main.c new file mode 100644 index ..c48a29969a85 --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_main.c @@ -0,0 +1,541 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include +#include +#include +#include +#include + +#include "ifcvf_base.h" + +#define VERSION_STRING "0.1" +#define DRIVER_AUTHOR "Intel Corporation" +#define IFCVF_DRIVER_NAME "ifcvf" + +static irqreturn_t ifcvf_intr_handler(int irq, void *arg) +{ + struct vring_info *vring = arg; + + if (vring->cb.callback) + return vring->cb.callback(vring->cb.private); + + return IRQ_HANDLED; +} + +static u64 ifcvf_mdev_get_features(struct mdev_device *mdev) +{ + return IFC_SUPPORTED_FEATURES; +} + +static int ifcvf_mdev_set_features(struct mdev_device *mdev, u64 features) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); Perhaps a helper that takes a struct mdev_device would be useful. The pattern above seems to be repeated many times. Hi Simon, thanks a lot for your comments. sure, can do + + vf->req_features = features; + + return 0; +} + +static u64 ifcvf_mdev_get_vq_state(struct mdev_device *mdev, u16 qid) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].last_avail_idx; +} + +static int ifcvf_mdev_set_vq_state(struct mdev_device *mdev, u16 qid, u64 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].last_used_idx = num; + vf->vring[qid].last_avail_idx = num; + + return 0; +} + +static int ifcvf_mdev_set_vq_address(struct mdev_device *mdev, u16 idx, +u64 desc_area, u64 driver_area, +u64 device_area) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].desc = desc_area; + vf->vring[idx].avail = driver_area; + vf->vring[idx].used = device_area; + + return 0; +} + +static void ifcvf_mdev_set_vq_num(struct mdev_device *mdev, u16 qid, u32 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].size = num; +} + +static void ifcvf_mdev_set_vq_ready(struct mdev_device *mdev, + u16 qid, bool ready) u16 should be vertically whitespace aligned with struct mdev_device. +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].ready = ready; +} + +static bool ifcvf_mdev_get_vq_ready(struct mdev_device *mdev, u16 qid) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].ready; +} + +static void ifcvf_mdev_set_vq_cb(struct mdev_device *mdev, u16 idx, +struct virtio_mdev_callback *cb) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].cb = *cb; +} + +static void ifcvf_mdev_kick_vq(struct mdev_device *mdev, u16 idx) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + ifcvf_notify_queue(vf, idx); +} + +static u8 ifcvf_mdev_get_status(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->status; +} + +static u32 ifcvf_mdev_get_generation(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->generation; +} + +static int ifcvf_mdev_get_version(struct mdev_device *mdev) +{ + return VIRTIO_MDEV_VERSION; +} + +static u32 ifcvf_mdev_get_device_id(struct mdev_device *mdev) +{ + return IFCVF_DEVICE_ID; +} + +static u32 ifcvf_mdev_get
Re: [RFC 0/2] Intel IFC VF driver for vdpa
On 10/16/2019 4:26 PM, Jason Wang wrote: On 2019/10/16 上午9:36, Zhu Lingshan wrote: failed to send to kvm list, resend, sorry for the inconvenience. THanks, BR Zhu Lingshan On 10/16/2019 9:30 AM, Zhu Lingshan wrote: Hi all: This series intends to introduce Intel IFC VF NIC driver for Vhost Data Plane Acceleration. Here comes two main parts, one is ifcvf_base layer, which handles hardware operations. The other is ifcvf_main layer handles VF initialization, configuration and removal, which depends on and complys to vhost_mdev https://lkml.org/lkml/2019/9/26/15 This is a first RFC try, please help review. It would be helpful if yon can describe which kinds of test that has been done for this series. Thanks Hi Jason, It passed ping and netperf tests(two VMs, each has one ifc vf backed mdev nic), I will mention this in next cover letter Thanks! BR Zhu Lingshan Thanks! BR Zhu Lingshan Zhu Lingshan (2): vhost: IFC VF hardware operation layer vhost: IFC VF vdpa layer drivers/vhost/ifcvf/ifcvf_base.c | 390 drivers/vhost/ifcvf/ifcvf_base.h | 137 ++ drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 3 files changed, 1068 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_base.c create mode 100644 drivers/vhost/ifcvf/ifcvf_base.h create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c
Re: [RFC 2/2] vhost: IFC VF vdpa layer
Hello Jason, Thanks for your comments, I am on a conference travel, I will reply next Monday. Thanks, BR Zhu Lingshan On 10/16/2019 6:19 PM, Jason Wang wrote: On 2019/10/16 上午9:30, Zhu Lingshan wrote: This commit introduced IFC VF operations for vdpa, which complys to vhost_mdev interfaces, handles IFC VF initialization, configuration and removal. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 1 file changed, 541 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c diff --git a/drivers/vhost/ifcvf/ifcvf_main.c b/drivers/vhost/ifcvf/ifcvf_main.c new file mode 100644 index ..c48a29969a85 --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_main.c @@ -0,0 +1,541 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include +#include +#include +#include +#include + +#include "ifcvf_base.h" + +#define VERSION_STRING "0.1" +#define DRIVER_AUTHOR "Intel Corporation" +#define IFCVF_DRIVER_NAME "ifcvf" + +static irqreturn_t ifcvf_intr_handler(int irq, void *arg) +{ + struct vring_info *vring = arg; + + if (vring->cb.callback) + return vring->cb.callback(vring->cb.private); + + return IRQ_HANDLED; +} + +static u64 ifcvf_mdev_get_features(struct mdev_device *mdev) +{ + return IFC_SUPPORTED_FEATURES; I would expect this should be done by querying the hw. Or IFC VF can't get any update through its firmware? +} + +static int ifcvf_mdev_set_features(struct mdev_device *mdev, u64 features) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->req_features = features; + + return 0; +} + +static u64 ifcvf_mdev_get_vq_state(struct mdev_device *mdev, u16 qid) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].last_avail_idx; Does this really work? I'd expect it should be fetched from hw since it's an internal state. +} + +static int ifcvf_mdev_set_vq_state(struct mdev_device *mdev, u16 qid, u64 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].last_used_idx = num; I fail to understand why last_used_idx is needed. It looks to me the used idx in the used ring is sufficient. + vf->vring[qid].last_avail_idx = num; Do we need a synchronization with hw immediately here? + + return 0; +} + +static int ifcvf_mdev_set_vq_address(struct mdev_device *mdev, u16 idx, + u64 desc_area, u64 driver_area, + u64 device_area) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].desc = desc_area; + vf->vring[idx].avail = driver_area; + vf->vring[idx].used = device_area; + + return 0; +} + +static void ifcvf_mdev_set_vq_num(struct mdev_device *mdev, u16 qid, u32 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].size = num; +} + +static void ifcvf_mdev_set_vq_ready(struct mdev_device *mdev, + u16 qid, bool ready) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].ready = ready; +} + +static bool ifcvf_mdev_get_vq_ready(struct mdev_device *mdev, u16 qid) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].ready; +} + +static void ifcvf_mdev_set_vq_cb(struct mdev_device *mdev, u16 idx, + struct virtio_mdev_callback *cb) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].cb = *cb; +} + +static void ifcvf_mdev_kick_vq(struct mdev_device *mdev, u16 idx) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + ifcvf_notify_queue(vf, idx); +} + +static u8 ifcvf_mdev_get_status(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->status; +} + +static u32 ifcvf_mdev_get_generation(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->generation; +} + +static int ifcvf_mdev_get_version(struct mdev_device *mdev) +{ + return VIRTIO_MDEV_VERSION; +} + +static u32 ifcvf_mdev_get_device_id(struct mdev_device *mdev) +{ + return IFCVF_DEVICE_ID; +} + +sta
Re: [RFC 0/2] Intel IFC VF driver for vdpa
failed to send to kvm list, resend, sorry for the inconvenience. THanks, BR Zhu Lingshan On 10/16/2019 9:30 AM, Zhu Lingshan wrote: Hi all: This series intends to introduce Intel IFC VF NIC driver for Vhost Data Plane Acceleration. Here comes two main parts, one is ifcvf_base layer, which handles hardware operations. The other is ifcvf_main layer handles VF initialization, configuration and removal, which depends on and complys to vhost_mdev https://lkml.org/lkml/2019/9/26/15 This is a first RFC try, please help review. Thanks! BR Zhu Lingshan Zhu Lingshan (2): vhost: IFC VF hardware operation layer vhost: IFC VF vdpa layer drivers/vhost/ifcvf/ifcvf_base.c | 390 drivers/vhost/ifcvf/ifcvf_base.h | 137 ++ drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 3 files changed, 1068 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_base.c create mode 100644 drivers/vhost/ifcvf/ifcvf_base.h create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c
[RFC 2/2] vhost: IFC VF vdpa layer
This commit introduced IFC VF operations for vdpa, which complys to vhost_mdev interfaces, handles IFC VF initialization, configuration and removal. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 1 file changed, 541 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c diff --git a/drivers/vhost/ifcvf/ifcvf_main.c b/drivers/vhost/ifcvf/ifcvf_main.c new file mode 100644 index ..c48a29969a85 --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_main.c @@ -0,0 +1,541 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include +#include +#include +#include +#include + +#include "ifcvf_base.h" + +#define VERSION_STRING "0.1" +#define DRIVER_AUTHOR "Intel Corporation" +#define IFCVF_DRIVER_NAME "ifcvf" + +static irqreturn_t ifcvf_intr_handler(int irq, void *arg) +{ + struct vring_info *vring = arg; + + if (vring->cb.callback) + return vring->cb.callback(vring->cb.private); + + return IRQ_HANDLED; +} + +static u64 ifcvf_mdev_get_features(struct mdev_device *mdev) +{ + return IFC_SUPPORTED_FEATURES; +} + +static int ifcvf_mdev_set_features(struct mdev_device *mdev, u64 features) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->req_features = features; + + return 0; +} + +static u64 ifcvf_mdev_get_vq_state(struct mdev_device *mdev, u16 qid) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].last_avail_idx; +} + +static int ifcvf_mdev_set_vq_state(struct mdev_device *mdev, u16 qid, u64 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].last_used_idx = num; + vf->vring[qid].last_avail_idx = num; + + return 0; +} + +static int ifcvf_mdev_set_vq_address(struct mdev_device *mdev, u16 idx, +u64 desc_area, u64 driver_area, +u64 device_area) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].desc = desc_area; + vf->vring[idx].avail = driver_area; + vf->vring[idx].used = device_area; + + return 0; +} + +static void ifcvf_mdev_set_vq_num(struct mdev_device *mdev, u16 qid, u32 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].size = num; +} + +static void ifcvf_mdev_set_vq_ready(struct mdev_device *mdev, + u16 qid, bool ready) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].ready = ready; +} + +static bool ifcvf_mdev_get_vq_ready(struct mdev_device *mdev, u16 qid) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].ready; +} + +static void ifcvf_mdev_set_vq_cb(struct mdev_device *mdev, u16 idx, +struct virtio_mdev_callback *cb) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].cb = *cb; +} + +static void ifcvf_mdev_kick_vq(struct mdev_device *mdev, u16 idx) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + ifcvf_notify_queue(vf, idx); +} + +static u8 ifcvf_mdev_get_status(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->status; +} + +static u32 ifcvf_mdev_get_generation(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->generation; +} + +static int ifcvf_mdev_get_version(struct mdev_device *mdev) +{ + return VIRTIO_MDEV_VERSION; +} + +static u32 ifcvf_mdev_get_device_id(struct mdev_device *mdev) +{ + return IFCVF_DEVICE_ID; +} + +static u32 ifcvf_mdev_get_vendor_id(struct mdev_device *mdev) +{ + return IFCVF_VENDOR_ID; +} + +static u16 ifcvf_mdev_get_vq_align(struct mdev_device *mdev) +{ + return IFCVF_QUEUE_ALIGNMENT; +} + +static int ifcvf_start_datapath(void *private) +{ + int i, ret; + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(private); + + for (i = 0; i < (IFCVF_MAX_QUEUE_PAIRS * 2); i++) { + if (!
[RFC 1/2] vhost: IFC VF hardware operation layer
This commit introduced ifcvf_base layer, which handles IFC VF NIC hardware operations and configurations. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_base.c | 390 +++ drivers/vhost/ifcvf/ifcvf_base.h | 137 ++ 2 files changed, 527 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_base.c create mode 100644 drivers/vhost/ifcvf/ifcvf_base.h diff --git a/drivers/vhost/ifcvf/ifcvf_base.c b/drivers/vhost/ifcvf/ifcvf_base.c new file mode 100644 index ..b85e14c9bdcf --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_base.c @@ -0,0 +1,390 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include "ifcvf_base.h" + +static void *get_cap_addr(struct ifcvf_hw *hw, struct virtio_pci_cap *cap) +{ + u8 bar = cap->bar; + u32 length = cap->length; + u32 offset = cap->offset; + struct ifcvf_adapter *ifcvf = + container_of(hw, struct ifcvf_adapter, vf); + + if (bar >= IFCVF_PCI_MAX_RESOURCE) { + IFC_ERR(ifcvf->dev, + "Invalid bar number %u to get capabilities.\n", bar); + return NULL; + } + + if (offset + length < offset) { + IFC_ERR(ifcvf->dev, "offset(%u) + length(%u) overflows\n", + offset, length); + return NULL; + } + + if (offset + length > hw->mem_resource[cap->bar].len) { + IFC_ERR(ifcvf->dev, + "offset(%u) + len(%u) overflows bar%u to get capabilities.\n", + offset, length, bar); + return NULL; + } + + return hw->mem_resource[bar].addr + offset; +} + +int ifcvf_read_config_range(struct pci_dev *dev, + uint32_t *val, int size, int where) +{ + int i; + + for (i = 0; i < size; i += 4) { + if (pci_read_config_dword(dev, where + i, val + i / 4) < 0) + return -1; + } + return 0; +} + +int ifcvf_init_hw(struct ifcvf_hw *hw, struct pci_dev *dev) +{ + int ret; + u8 pos; + struct virtio_pci_cap cap; + u32 i; + u16 notify_off; + + ret = pci_read_config_byte(dev, PCI_CAPABILITY_LIST, &pos); + + if (ret < 0) { + IFC_ERR(&dev->dev, "failed to read PCI capability list.\n"); + return -EIO; + } + + while (pos) { + ret = ifcvf_read_config_range(dev, (u32 *)&cap, + sizeof(cap), pos); + + if (ret < 0) { + IFC_ERR(&dev->dev, "failed to get PCI capability at %x", + pos); + break; + } + + if (cap.cap_vndr != PCI_CAP_ID_VNDR) + goto next; + + IFC_INFO(&dev->dev, "read PCI config:\n" + "config type: %u.\n" + "PCI bar: %u.\n" + "PCI bar offset: %u.\n" + "PCI config len: %u.\n", + cap.cfg_type, cap.bar, + cap.offset, cap.length); + + switch (cap.cfg_type) { + case VIRTIO_PCI_CAP_COMMON_CFG: + hw->common_cfg = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->common_cfg = %p.\n", + hw->common_cfg); + break; + case VIRTIO_PCI_CAP_NOTIFY_CFG: + pci_read_config_dword(dev, pos + sizeof(cap), + &hw->notify_off_multiplier); + hw->notify_bar = cap.bar; + hw->notify_base = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->notify_base = %p.\n", + hw->notify_base); + break; + case VIRTIO_PCI_CAP_ISR_CFG: + hw->isr = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->isr = %p.\n", hw->isr); + break; + case VIRTIO_PCI_CAP_DEVICE_CFG: + hw->dev_cfg = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->dev_cfg = %p.\n", hw->dev_cfg); + break; + } +next: + pos = cap.cap_next; + } + + if (hw->common_cfg == NULL || hw->notify_base == NULL || + hw
[RFC 0/2] Intel IFC VF driver for vdpa
Hi all: This series intends to introduce Intel IFC VF NIC driver for Vhost Data Plane Acceleration. Here comes two main parts, one is ifcvf_base layer, which handles hardware operations. The other is ifcvf_main layer handles VF initialization, configuration and removal, which depends on and complys to vhost_mdev https://lkml.org/lkml/2019/9/26/15 This is a first RFC try, please help review. Thanks! BR Zhu Lingshan Zhu Lingshan (2): vhost: IFC VF hardware operation layer vhost: IFC VF vdpa layer drivers/vhost/ifcvf/ifcvf_base.c | 390 drivers/vhost/ifcvf/ifcvf_base.h | 137 ++ drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 3 files changed, 1068 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_base.c create mode 100644 drivers/vhost/ifcvf/ifcvf_base.h create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c -- 2.16.4
[RFC 1/2] vhost: IFC VF hardware operation layer
This commit introduced ifcvf_base layer, which handles IFC VF NIC hardware operations and configurations. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_base.c | 390 +++ drivers/vhost/ifcvf/ifcvf_base.h | 137 ++ 2 files changed, 527 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_base.c create mode 100644 drivers/vhost/ifcvf/ifcvf_base.h diff --git a/drivers/vhost/ifcvf/ifcvf_base.c b/drivers/vhost/ifcvf/ifcvf_base.c new file mode 100644 index ..b85e14c9bdcf --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_base.c @@ -0,0 +1,390 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include "ifcvf_base.h" + +static void *get_cap_addr(struct ifcvf_hw *hw, struct virtio_pci_cap *cap) +{ + u8 bar = cap->bar; + u32 length = cap->length; + u32 offset = cap->offset; + struct ifcvf_adapter *ifcvf = + container_of(hw, struct ifcvf_adapter, vf); + + if (bar >= IFCVF_PCI_MAX_RESOURCE) { + IFC_ERR(ifcvf->dev, + "Invalid bar number %u to get capabilities.\n", bar); + return NULL; + } + + if (offset + length < offset) { + IFC_ERR(ifcvf->dev, "offset(%u) + length(%u) overflows\n", + offset, length); + return NULL; + } + + if (offset + length > hw->mem_resource[cap->bar].len) { + IFC_ERR(ifcvf->dev, + "offset(%u) + len(%u) overflows bar%u to get capabilities.\n", + offset, length, bar); + return NULL; + } + + return hw->mem_resource[bar].addr + offset; +} + +int ifcvf_read_config_range(struct pci_dev *dev, + uint32_t *val, int size, int where) +{ + int i; + + for (i = 0; i < size; i += 4) { + if (pci_read_config_dword(dev, where + i, val + i / 4) < 0) + return -1; + } + return 0; +} + +int ifcvf_init_hw(struct ifcvf_hw *hw, struct pci_dev *dev) +{ + int ret; + u8 pos; + struct virtio_pci_cap cap; + u32 i; + u16 notify_off; + + ret = pci_read_config_byte(dev, PCI_CAPABILITY_LIST, &pos); + + if (ret < 0) { + IFC_ERR(&dev->dev, "failed to read PCI capability list.\n"); + return -EIO; + } + + while (pos) { + ret = ifcvf_read_config_range(dev, (u32 *)&cap, + sizeof(cap), pos); + + if (ret < 0) { + IFC_ERR(&dev->dev, "failed to get PCI capability at %x", + pos); + break; + } + + if (cap.cap_vndr != PCI_CAP_ID_VNDR) + goto next; + + IFC_INFO(&dev->dev, "read PCI config:\n" + "config type: %u.\n" + "PCI bar: %u.\n" + "PCI bar offset: %u.\n" + "PCI config len: %u.\n", + cap.cfg_type, cap.bar, + cap.offset, cap.length); + + switch (cap.cfg_type) { + case VIRTIO_PCI_CAP_COMMON_CFG: + hw->common_cfg = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->common_cfg = %p.\n", + hw->common_cfg); + break; + case VIRTIO_PCI_CAP_NOTIFY_CFG: + pci_read_config_dword(dev, pos + sizeof(cap), + &hw->notify_off_multiplier); + hw->notify_bar = cap.bar; + hw->notify_base = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->notify_base = %p.\n", + hw->notify_base); + break; + case VIRTIO_PCI_CAP_ISR_CFG: + hw->isr = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->isr = %p.\n", hw->isr); + break; + case VIRTIO_PCI_CAP_DEVICE_CFG: + hw->dev_cfg = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->dev_cfg = %p.\n", hw->dev_cfg); + break; + } +next: + pos = cap.cap_next; + } + + if (hw->common_cfg == NULL || hw->notify_base == NULL || + hw
[RFC 0/2] Intel IFC VF driver for vdpa
Hi all: This series intends to introduce Intel IFC VF NIC driver for Vhost Data Plane Acceleration. Here comes two main parts, one is ifcvf_base layer, which handles hardware operations. The other is ifcvf_main layer handles VF initialization, configuration and removal, which depends on and complys to vhost_mdev https://lkml.org/lkml/2019/9/26/15 This is a first RFC try, please help review. Thanks! BR Zhu Lingshan Zhu Lingshan (2): vhost: IFC VF hardware operation layer vhost: IFC VF vdpa layer drivers/vhost/ifcvf/ifcvf_base.c | 390 drivers/vhost/ifcvf/ifcvf_base.h | 137 ++ drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 3 files changed, 1068 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_base.c create mode 100644 drivers/vhost/ifcvf/ifcvf_base.h create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c -- 2.16.4
[RFC 2/2] vhost: IFC VF vdpa layer
This commit introduced IFC VF operations for vdpa, which complys to vhost_mdev interfaces, handles IFC VF initialization, configuration and removal. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 1 file changed, 541 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c diff --git a/drivers/vhost/ifcvf/ifcvf_main.c b/drivers/vhost/ifcvf/ifcvf_main.c new file mode 100644 index ..c48a29969a85 --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_main.c @@ -0,0 +1,541 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include +#include +#include +#include +#include + +#include "ifcvf_base.h" + +#define VERSION_STRING "0.1" +#define DRIVER_AUTHOR "Intel Corporation" +#define IFCVF_DRIVER_NAME "ifcvf" + +static irqreturn_t ifcvf_intr_handler(int irq, void *arg) +{ + struct vring_info *vring = arg; + + if (vring->cb.callback) + return vring->cb.callback(vring->cb.private); + + return IRQ_HANDLED; +} + +static u64 ifcvf_mdev_get_features(struct mdev_device *mdev) +{ + return IFC_SUPPORTED_FEATURES; +} + +static int ifcvf_mdev_set_features(struct mdev_device *mdev, u64 features) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->req_features = features; + + return 0; +} + +static u64 ifcvf_mdev_get_vq_state(struct mdev_device *mdev, u16 qid) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].last_avail_idx; +} + +static int ifcvf_mdev_set_vq_state(struct mdev_device *mdev, u16 qid, u64 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].last_used_idx = num; + vf->vring[qid].last_avail_idx = num; + + return 0; +} + +static int ifcvf_mdev_set_vq_address(struct mdev_device *mdev, u16 idx, +u64 desc_area, u64 driver_area, +u64 device_area) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].desc = desc_area; + vf->vring[idx].avail = driver_area; + vf->vring[idx].used = device_area; + + return 0; +} + +static void ifcvf_mdev_set_vq_num(struct mdev_device *mdev, u16 qid, u32 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].size = num; +} + +static void ifcvf_mdev_set_vq_ready(struct mdev_device *mdev, + u16 qid, bool ready) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].ready = ready; +} + +static bool ifcvf_mdev_get_vq_ready(struct mdev_device *mdev, u16 qid) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].ready; +} + +static void ifcvf_mdev_set_vq_cb(struct mdev_device *mdev, u16 idx, +struct virtio_mdev_callback *cb) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].cb = *cb; +} + +static void ifcvf_mdev_kick_vq(struct mdev_device *mdev, u16 idx) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + ifcvf_notify_queue(vf, idx); +} + +static u8 ifcvf_mdev_get_status(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->status; +} + +static u32 ifcvf_mdev_get_generation(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->generation; +} + +static int ifcvf_mdev_get_version(struct mdev_device *mdev) +{ + return VIRTIO_MDEV_VERSION; +} + +static u32 ifcvf_mdev_get_device_id(struct mdev_device *mdev) +{ + return IFCVF_DEVICE_ID; +} + +static u32 ifcvf_mdev_get_vendor_id(struct mdev_device *mdev) +{ + return IFCVF_VENDOR_ID; +} + +static u16 ifcvf_mdev_get_vq_align(struct mdev_device *mdev) +{ + return IFCVF_QUEUE_ALIGNMENT; +} + +static int ifcvf_start_datapath(void *private) +{ + int i, ret; + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(private); + + for (i = 0; i < (IFCVF_MAX_QUEUE_PAIRS * 2); i++) { + if (!
[RFC 1/2] vhost: IFC VF hardware operation layer
This commit introduced ifcvf_base layer, which handles IFC VF NIC hardware operations and configurations. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_base.c | 390 +++ drivers/vhost/ifcvf/ifcvf_base.h | 137 ++ 2 files changed, 527 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_base.c create mode 100644 drivers/vhost/ifcvf/ifcvf_base.h diff --git a/drivers/vhost/ifcvf/ifcvf_base.c b/drivers/vhost/ifcvf/ifcvf_base.c new file mode 100644 index ..b85e14c9bdcf --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_base.c @@ -0,0 +1,390 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include "ifcvf_base.h" + +static void *get_cap_addr(struct ifcvf_hw *hw, struct virtio_pci_cap *cap) +{ + u8 bar = cap->bar; + u32 length = cap->length; + u32 offset = cap->offset; + struct ifcvf_adapter *ifcvf = + container_of(hw, struct ifcvf_adapter, vf); + + if (bar >= IFCVF_PCI_MAX_RESOURCE) { + IFC_ERR(ifcvf->dev, + "Invalid bar number %u to get capabilities.\n", bar); + return NULL; + } + + if (offset + length < offset) { + IFC_ERR(ifcvf->dev, "offset(%u) + length(%u) overflows\n", + offset, length); + return NULL; + } + + if (offset + length > hw->mem_resource[cap->bar].len) { + IFC_ERR(ifcvf->dev, + "offset(%u) + len(%u) overflows bar%u to get capabilities.\n", + offset, length, bar); + return NULL; + } + + return hw->mem_resource[bar].addr + offset; +} + +int ifcvf_read_config_range(struct pci_dev *dev, + uint32_t *val, int size, int where) +{ + int i; + + for (i = 0; i < size; i += 4) { + if (pci_read_config_dword(dev, where + i, val + i / 4) < 0) + return -1; + } + return 0; +} + +int ifcvf_init_hw(struct ifcvf_hw *hw, struct pci_dev *dev) +{ + int ret; + u8 pos; + struct virtio_pci_cap cap; + u32 i; + u16 notify_off; + + ret = pci_read_config_byte(dev, PCI_CAPABILITY_LIST, &pos); + + if (ret < 0) { + IFC_ERR(&dev->dev, "failed to read PCI capability list.\n"); + return -EIO; + } + + while (pos) { + ret = ifcvf_read_config_range(dev, (u32 *)&cap, + sizeof(cap), pos); + + if (ret < 0) { + IFC_ERR(&dev->dev, "failed to get PCI capability at %x", + pos); + break; + } + + if (cap.cap_vndr != PCI_CAP_ID_VNDR) + goto next; + + IFC_INFO(&dev->dev, "read PCI config:\n" + "config type: %u.\n" + "PCI bar: %u.\n" + "PCI bar offset: %u.\n" + "PCI config len: %u.\n", + cap.cfg_type, cap.bar, + cap.offset, cap.length); + + switch (cap.cfg_type) { + case VIRTIO_PCI_CAP_COMMON_CFG: + hw->common_cfg = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->common_cfg = %p.\n", + hw->common_cfg); + break; + case VIRTIO_PCI_CAP_NOTIFY_CFG: + pci_read_config_dword(dev, pos + sizeof(cap), + &hw->notify_off_multiplier); + hw->notify_bar = cap.bar; + hw->notify_base = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->notify_base = %p.\n", + hw->notify_base); + break; + case VIRTIO_PCI_CAP_ISR_CFG: + hw->isr = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->isr = %p.\n", hw->isr); + break; + case VIRTIO_PCI_CAP_DEVICE_CFG: + hw->dev_cfg = get_cap_addr(hw, &cap); + IFC_INFO(&dev->dev, "hw->dev_cfg = %p.\n", hw->dev_cfg); + break; + } +next: + pos = cap.cap_next; + } + + if (hw->common_cfg == NULL || hw->notify_base == NULL || + hw
[RFC 2/2] vhost: IFC VF vdpa layer
This commit introduced IFC VF operations for vdpa, which complys to vhost_mdev interfaces, handles IFC VF initialization, configuration and removal. Signed-off-by: Zhu Lingshan --- drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 1 file changed, 541 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c diff --git a/drivers/vhost/ifcvf/ifcvf_main.c b/drivers/vhost/ifcvf/ifcvf_main.c new file mode 100644 index ..c48a29969a85 --- /dev/null +++ b/drivers/vhost/ifcvf/ifcvf_main.c @@ -0,0 +1,541 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2019 Intel Corporation. + */ + +#include +#include +#include +#include +#include + +#include "ifcvf_base.h" + +#define VERSION_STRING "0.1" +#define DRIVER_AUTHOR "Intel Corporation" +#define IFCVF_DRIVER_NAME "ifcvf" + +static irqreturn_t ifcvf_intr_handler(int irq, void *arg) +{ + struct vring_info *vring = arg; + + if (vring->cb.callback) + return vring->cb.callback(vring->cb.private); + + return IRQ_HANDLED; +} + +static u64 ifcvf_mdev_get_features(struct mdev_device *mdev) +{ + return IFC_SUPPORTED_FEATURES; +} + +static int ifcvf_mdev_set_features(struct mdev_device *mdev, u64 features) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->req_features = features; + + return 0; +} + +static u64 ifcvf_mdev_get_vq_state(struct mdev_device *mdev, u16 qid) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].last_avail_idx; +} + +static int ifcvf_mdev_set_vq_state(struct mdev_device *mdev, u16 qid, u64 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].last_used_idx = num; + vf->vring[qid].last_avail_idx = num; + + return 0; +} + +static int ifcvf_mdev_set_vq_address(struct mdev_device *mdev, u16 idx, +u64 desc_area, u64 driver_area, +u64 device_area) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].desc = desc_area; + vf->vring[idx].avail = driver_area; + vf->vring[idx].used = device_area; + + return 0; +} + +static void ifcvf_mdev_set_vq_num(struct mdev_device *mdev, u16 qid, u32 num) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].size = num; +} + +static void ifcvf_mdev_set_vq_ready(struct mdev_device *mdev, + u16 qid, bool ready) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[qid].ready = ready; +} + +static bool ifcvf_mdev_get_vq_ready(struct mdev_device *mdev, u16 qid) +{ + + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->vring[qid].ready; +} + +static void ifcvf_mdev_set_vq_cb(struct mdev_device *mdev, u16 idx, +struct virtio_mdev_callback *cb) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + vf->vring[idx].cb = *cb; +} + +static void ifcvf_mdev_kick_vq(struct mdev_device *mdev, u16 idx) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + ifcvf_notify_queue(vf, idx); +} + +static u8 ifcvf_mdev_get_status(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->status; +} + +static u32 ifcvf_mdev_get_generation(struct mdev_device *mdev) +{ + struct ifcvf_adapter *adapter = mdev_get_drvdata(mdev); + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(adapter); + + return vf->generation; +} + +static int ifcvf_mdev_get_version(struct mdev_device *mdev) +{ + return VIRTIO_MDEV_VERSION; +} + +static u32 ifcvf_mdev_get_device_id(struct mdev_device *mdev) +{ + return IFCVF_DEVICE_ID; +} + +static u32 ifcvf_mdev_get_vendor_id(struct mdev_device *mdev) +{ + return IFCVF_VENDOR_ID; +} + +static u16 ifcvf_mdev_get_vq_align(struct mdev_device *mdev) +{ + return IFCVF_QUEUE_ALIGNMENT; +} + +static int ifcvf_start_datapath(void *private) +{ + int i, ret; + struct ifcvf_hw *vf = IFC_PRIVATE_TO_VF(private); + + for (i = 0; i < (IFCVF_MAX_QUEUE_PAIRS * 2); i++) { + if (!
[RFC 0/2] Intel IFC VF driver for vdpa
Hi all: This series intends to introduce Intel IFC VF NIC driver for Vhost Data Plane Acceleration. Here comes two main parts, one is ifcvf_base layer, which handles hardware operations. The other is ifcvf_main layer handles VF initialization, configuration and removal, which depends on and complys to vhost_mdev https://lkml.org/lkml/2019/9/26/15 This is a first RFC try, please help review. Thanks! BR Zhu Lingshan Zhu Lingshan (2): vhost: IFC VF hardware operation layer vhost: IFC VF vdpa layer drivers/vhost/ifcvf/ifcvf_base.c | 390 drivers/vhost/ifcvf/ifcvf_base.h | 137 ++ drivers/vhost/ifcvf/ifcvf_main.c | 541 +++ 3 files changed, 1068 insertions(+) create mode 100644 drivers/vhost/ifcvf/ifcvf_base.c create mode 100644 drivers/vhost/ifcvf/ifcvf_base.h create mode 100644 drivers/vhost/ifcvf/ifcvf_main.c -- 2.16.4
[PATCH v2] .gitignore: ignore ASN.1 auto generated files
when build kernel with default configure, files: generatenet/ipv4/netfilter/nf_nat_snmp_basic-asn1.c net/ipv4/netfilter/nf_nat_snmp_basic-asn1.h will be automatically generated by ASN.1 compiler, so No need to track them in git, it's better to ignore them. Signed-off-by: Zhu Lingshan --- Changes in v2: -correct ANS.1 --> ASN.1 in title .gitignore | 4 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index 705e09913dc2..1be78fd8163b 100644 --- a/.gitignore +++ b/.gitignore @@ -127,3 +127,7 @@ all.config # Kdevelop4 *.kdev4 + +#Automatically generated by ASN.1 compiler +net/ipv4/netfilter/nf_nat_snmp_basic-asn1.c +net/ipv4/netfilter/nf_nat_snmp_basic-asn1.h -- 2.13.6