* Re: [PATCH V2 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA [not found] ` <20210415095336.4792-3-lingshan.zhu@intel.com> @ 2021-04-15 13:41 ` Stefano Garzarella 2021-04-16 2:43 ` Zhu Lingshan 0 siblings, 1 reply; 4+ messages in thread From: Stefano Garzarella @ 2021-04-15 13:41 UTC (permalink / raw) To: Zhu Lingshan; +Cc: lulu, kvm, mst, netdev, linux-kernel, virtualization 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 <lingshan.zhu@intel.com> >--- > 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 <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) | \ >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, 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 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V2 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA 2021-04-15 13:41 ` [PATCH V2 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA Stefano Garzarella @ 2021-04-16 2:43 ` Zhu Lingshan 0 siblings, 0 replies; 4+ messages in thread From: Zhu Lingshan @ 2021-04-16 2:43 UTC (permalink / raw) To: Stefano Garzarella, Zhu Lingshan Cc: lulu, kvm, mst, netdev, linux-kernel, virtualization 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 <lingshan.zhu@intel.com> >> --- >> 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 <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) | \ >> 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 >> > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <20210415095336.4792-4-lingshan.zhu@intel.com>]
* Re: [PATCH V2 3/3] vDPA/ifcvf: get_config_size should return dev specific config size [not found] ` <20210415095336.4792-4-lingshan.zhu@intel.com> @ 2021-04-15 13:48 ` Stefano Garzarella 2021-04-16 2:42 ` Zhu Lingshan 0 siblings, 1 reply; 4+ messages in thread From: Stefano Garzarella @ 2021-04-15 13:48 UTC (permalink / raw) To: Zhu Lingshan; +Cc: lulu, kvm, mst, netdev, linux-kernel, virtualization 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 <lingshan.zhu@intel.com> >--- > 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 _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V2 3/3] vDPA/ifcvf: get_config_size should return dev specific config size 2021-04-15 13:48 ` [PATCH V2 3/3] vDPA/ifcvf: get_config_size should return dev specific config size Stefano Garzarella @ 2021-04-16 2:42 ` Zhu Lingshan 0 siblings, 0 replies; 4+ messages in thread From: Zhu Lingshan @ 2021-04-16 2:42 UTC (permalink / raw) To: Stefano Garzarella, Zhu Lingshan Cc: lulu, kvm, mst, netdev, linux-kernel, virtualization 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 <lingshan.zhu@intel.com> >> --- >> 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 > Virtualization@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/virtualization _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-16 2:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20210415095336.4792-1-lingshan.zhu@intel.com>
[not found] ` <20210415095336.4792-3-lingshan.zhu@intel.com>
2021-04-15 13:41 ` [PATCH V2 2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA Stefano Garzarella
2021-04-16 2:43 ` Zhu Lingshan
[not found] ` <20210415095336.4792-4-lingshan.zhu@intel.com>
2021-04-15 13:48 ` [PATCH V2 3/3] vDPA/ifcvf: get_config_size should return dev specific config size Stefano Garzarella
2021-04-16 2:42 ` Zhu Lingshan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox