On 4/15/2021 2:31 PM, Jason Wang wrote:

在 2021/4/15 下午1:55, Zhu Lingshan 写道:

quoted

On 4/15/2021 11:34 AM, Jason Wang wrote:

quoted

在 2021/4/14 下午5:18, Zhu Lingshan 写道:

quoted

This commit enabled Intel FPGA SmartNIC C5000X-PL virtio-block
for vDPA.

Signed-off-by: Zhu Lingshan <redacted>
---
  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 <linux/pci_regs.h>
  #include <linux/vdpa.h>
  #include <uapi/linux/virtio_net.h>
+#include <uapi/linux/virtio_blk.h>
  #include <uapi/linux/virtio_config.h>
  #include <uapi/linux/virtio_pci.h>
  @@ -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

quoted

Thanks!

quoted

quoted

+
  /* 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 },
  };

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization