diff mbox series

[2/3] vDPA/ifcvf: enable Intel C5000X-PL virtio-block for vDPA

Message ID 20210414091832.5132-3-lingshan.zhu@intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series vDPA/ifcvf: enables Intel C5000X-PL virtio-blk | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Zhu, Lingshan April 14, 2021, 9:18 a.m. UTC
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 | 17 ++++++++++++++++-
 drivers/vdpa/ifcvf/ifcvf_main.c | 10 +++++++++-
 2 files changed, 25 insertions(+), 2 deletions(-)

Comments

Jason Wang April 15, 2021, 3:34 a.m. UTC | #1
在 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@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


> +
>   /* 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 },
>   };
Zhu Lingshan April 15, 2021, 5:55 a.m. UTC | #2
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@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.

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 April 15, 2021, 6:31 a.m. UTC | #3
在 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@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 },
>>>   };
>>
>
Zhu Lingshan April 15, 2021, 6:41 a.m. UTC | #4
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 <lingshan.zhu@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
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 },
>>>>   };
>>>
>>
>
Jason Wang April 15, 2021, 7:17 a.m. UTC | #5
在 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.

Thanks
Zhu Lingshan April 15, 2021, 7:23 a.m. UTC | #6
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
diff mbox series

Patch

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