Zhu Lingshan
2021-Apr-15 05:55 UTC
[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 <lingshan.zhu at intel.com> >> --- >> ? 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? > > ThanksThe 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 }, >> ? }; >
Jason Wang
2021-Apr-15 06:31 UTC
[PATCH 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA
? 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 <lingshan.zhu at intel.com> >>> --- >>> ? 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> > 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 }, >>> ? }; >> >