mbox series

[V2,0/3] vDPA/ifcvf: enables Intel C5000X-PL virtio-blk

Message ID 20210415095336.4792-1-lingshan.zhu@intel.com
Headers show
Series vDPA/ifcvf: enables Intel C5000X-PL virtio-blk | expand

Message

Zhu Lingshan April 15, 2021, 9:53 a.m. UTC 
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(-)

Comments

Zhu Lingshan April 16, 2021, 2:42 a.m. UTC | #1 
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