diff mbox series

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

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

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 },
 };