mbox series

[V3,0/6] vDPA/ifcvf: enables Intel C5000X-PL virtio-net

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

Message

Zhu Lingshan March 10, 2021, 9 a.m. UTC 
This series enabled Intel FGPA SmartNIC C5000X-PL
virtio-net for vDPA.
vDPA requires VIRTIO_F_ACCESS_PLATFORM as a must, this series
verify this feature bit when set features.

Changes from V2:
verify VIRTIO_F_ACCESS_PLATFORM when set features(Jason)

Changes from V1:
remove version number string(Leon)
add new device ids and remove original device ids in
separate patches(Jason)

Zhu Lingshan (6):
  vDPA/ifcvf: get_vendor_id returns a device specific vendor id
  vDPA/ifcvf: enable Intel C5000X-PL virtio-net for vDPA
  vDPA/ifcvf: rename original IFCVF dev ids to N3000 ids
  vDPA/ifcvf: remove the version number string
  vDPA/ifcvf: fetch device feature bits when probe
  vDPA/ifcvf: verify mandatory feature bits for vDPA

 drivers/vdpa/ifcvf/ifcvf_base.c | 20 ++++++++++++++++++--
 drivers/vdpa/ifcvf/ifcvf_base.h | 16 ++++++++++++----
 drivers/vdpa/ifcvf/ifcvf_main.c | 27 ++++++++++++++++++++-------
 3 files changed, 50 insertions(+), 13 deletions(-)

Comments

Jason Wang March 11, 2021, 3:23 a.m. UTC | #1 
On 2021/3/10 5:00 下午, Zhu Lingshan wrote:
> In this commit, ifcvf_get_vendor_id() will return

> a device specific vendor id of the probed pci device

> than a hard code.

>

> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

> ---

>   drivers/vdpa/ifcvf/ifcvf_main.c | 5 ++++-

>   1 file changed, 4 insertions(+), 1 deletion(-)

>

> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c

> index fa1af301cf55..e501ee07de17 100644

> --- a/drivers/vdpa/ifcvf/ifcvf_main.c

> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c

> @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev)

>   

>   static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev)

>   {

> -	return IFCVF_SUBSYS_VENDOR_ID;

> +	struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);

> +	struct pci_dev *pdev = adapter->pdev;

> +

> +	return pdev->subsystem_vendor;

>   }



While at this, I wonder if we can do something similar in 
get_device_id() if it could be simple deduced from some simple math from 
the pci device id?

Thanks


>   

>   static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev)
Jason Wang March 11, 2021, 3:25 a.m. UTC | #2 
On 2021/3/10 5:00 下午, Zhu Lingshan wrote:
> IFCVF driver probes multiple types of devices now,

> to distinguish the original device driven by IFCVF

> from others, it is renamed as "N3000".

>

> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

> ---

>   drivers/vdpa/ifcvf/ifcvf_base.h | 8 ++++----

>   drivers/vdpa/ifcvf/ifcvf_main.c | 8 ++++----

>   2 files changed, 8 insertions(+), 8 deletions(-)

>

> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h

> index 75d9a8052039..794d1505d857 100644

> --- a/drivers/vdpa/ifcvf/ifcvf_base.h

> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h

> @@ -18,10 +18,10 @@

>   #include <uapi/linux/virtio_config.h>

>   #include <uapi/linux/virtio_pci.h>

>   

> -#define IFCVF_VENDOR_ID		0x1AF4

> -#define IFCVF_DEVICE_ID		0x1041

> -#define IFCVF_SUBSYS_VENDOR_ID	0x8086

> -#define IFCVF_SUBSYS_DEVICE_ID	0x001A

> +#define N3000_VENDOR_ID		0x1AF4

> +#define N3000_DEVICE_ID		0x1041

> +#define N3000_SUBSYS_VENDOR_ID	0x8086

> +#define N3000_SUBSYS_DEVICE_ID	0x001A

>   

>   #define C5000X_PL_VENDOR_ID		0x1AF4

>   #define C5000X_PL_DEVICE_ID		0x1000

> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c

> index 26a2dab7ca66..fd5befc5cbcc 100644

> --- a/drivers/vdpa/ifcvf/ifcvf_main.c

> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c

> @@ -480,10 +480,10 @@ static void ifcvf_remove(struct pci_dev *pdev)

>   }

>   

>   static struct pci_device_id ifcvf_pci_ids[] = {

> -	{ PCI_DEVICE_SUB(IFCVF_VENDOR_ID,

> -		IFCVF_DEVICE_ID,

> -		IFCVF_SUBSYS_VENDOR_ID,

> -		IFCVF_SUBSYS_DEVICE_ID) },

> +	{ PCI_DEVICE_SUB(N3000_VENDOR_ID,

> +			 N3000_DEVICE_ID,



I am not sure the plan for Intel but I wonder if we can simply use 
PCI_ANY_ID for device id here. Otherewise you need to maintain a very 
long list of ids here.

Thanks


> +			 N3000_SUBSYS_VENDOR_ID,

> +			 N3000_SUBSYS_DEVICE_ID) },

>   	{ PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID,

>   			 C5000X_PL_DEVICE_ID,

>   			 C5000X_PL_SUBSYS_VENDOR_ID,
Zhu Lingshan March 11, 2021, 4:21 a.m. UTC | #3 
On 3/11/2021 11:23 AM, Jason Wang wrote:
>

> On 2021/3/10 5:00 下午, Zhu Lingshan wrote:

>> In this commit, ifcvf_get_vendor_id() will return

>> a device specific vendor id of the probed pci device

>> than a hard code.

>>

>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

>> ---

>>   drivers/vdpa/ifcvf/ifcvf_main.c | 5 ++++-

>>   1 file changed, 4 insertions(+), 1 deletion(-)

>>

>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 

>> b/drivers/vdpa/ifcvf/ifcvf_main.c

>> index fa1af301cf55..e501ee07de17 100644

>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c

>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c

>> @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct 

>> vdpa_device *vdpa_dev)

>>     static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev)

>>   {

>> -    return IFCVF_SUBSYS_VENDOR_ID;

>> +    struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);

>> +    struct pci_dev *pdev = adapter->pdev;

>> +

>> +    return pdev->subsystem_vendor;

>>   }

>

>

> While at this, I wonder if we can do something similar in 

> get_device_id() if it could be simple deduced from some simple math 

> from the pci device id?

>

> Thanks

Hi Jason,

IMHO, this implementation is just some memory read ops, I think other 
implementations may not save many cpu cycles, an if cost at least three 
cpu cycles.

Thanks!
>

>

>>     static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev)

>
Zhu Lingshan March 11, 2021, 4:23 a.m. UTC | #4 
On 3/11/2021 11:25 AM, Jason Wang wrote:
>

> On 2021/3/10 5:00 下午, Zhu Lingshan wrote:

>> IFCVF driver probes multiple types of devices now,

>> to distinguish the original device driven by IFCVF

>> from others, it is renamed as "N3000".

>>

>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

>> ---

>>   drivers/vdpa/ifcvf/ifcvf_base.h | 8 ++++----

>>   drivers/vdpa/ifcvf/ifcvf_main.c | 8 ++++----

>>   2 files changed, 8 insertions(+), 8 deletions(-)

>>

>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h 

>> b/drivers/vdpa/ifcvf/ifcvf_base.h

>> index 75d9a8052039..794d1505d857 100644

>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h

>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h

>> @@ -18,10 +18,10 @@

>>   #include <uapi/linux/virtio_config.h>

>>   #include <uapi/linux/virtio_pci.h>

>>   -#define IFCVF_VENDOR_ID        0x1AF4

>> -#define IFCVF_DEVICE_ID        0x1041

>> -#define IFCVF_SUBSYS_VENDOR_ID    0x8086

>> -#define IFCVF_SUBSYS_DEVICE_ID    0x001A

>> +#define N3000_VENDOR_ID        0x1AF4

>> +#define N3000_DEVICE_ID        0x1041

>> +#define N3000_SUBSYS_VENDOR_ID    0x8086

>> +#define N3000_SUBSYS_DEVICE_ID    0x001A

>>     #define C5000X_PL_VENDOR_ID        0x1AF4

>>   #define C5000X_PL_DEVICE_ID        0x1000

>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 

>> b/drivers/vdpa/ifcvf/ifcvf_main.c

>> index 26a2dab7ca66..fd5befc5cbcc 100644

>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c

>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c

>> @@ -480,10 +480,10 @@ static void ifcvf_remove(struct pci_dev *pdev)

>>   }

>>     static struct pci_device_id ifcvf_pci_ids[] = {

>> -    { PCI_DEVICE_SUB(IFCVF_VENDOR_ID,

>> -        IFCVF_DEVICE_ID,

>> -        IFCVF_SUBSYS_VENDOR_ID,

>> -        IFCVF_SUBSYS_DEVICE_ID) },

>> +    { PCI_DEVICE_SUB(N3000_VENDOR_ID,

>> +             N3000_DEVICE_ID,

>

>

> I am not sure the plan for Intel but I wonder if we can simply use 

> PCI_ANY_ID for device id here. Otherewise you need to maintain a very 

> long list of ids here.

>

> Thanks

Hi Jason,

Thanks! but maybe if we present a very simple and clear list like what 
e1000 does can help the users understand what we support easily.

Thanks!
>

>

>> + N3000_SUBSYS_VENDOR_ID,

>> +             N3000_SUBSYS_DEVICE_ID) },

>>       { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID,

>>                C5000X_PL_DEVICE_ID,

>>                C5000X_PL_SUBSYS_VENDOR_ID,

>
Jason Wang March 11, 2021, 6:13 a.m. UTC | #5 
On 2021/3/11 12:21 下午, Zhu Lingshan wrote:
>

>

> On 3/11/2021 11:23 AM, Jason Wang wrote:

>>

>> On 2021/3/10 5:00 下午, Zhu Lingshan wrote:

>>> In this commit, ifcvf_get_vendor_id() will return

>>> a device specific vendor id of the probed pci device

>>> than a hard code.

>>>

>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

>>> ---

>>>   drivers/vdpa/ifcvf/ifcvf_main.c | 5 ++++-

>>>   1 file changed, 4 insertions(+), 1 deletion(-)

>>>

>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 

>>> b/drivers/vdpa/ifcvf/ifcvf_main.c

>>> index fa1af301cf55..e501ee07de17 100644

>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c

>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c

>>> @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct 

>>> vdpa_device *vdpa_dev)

>>>     static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev)

>>>   {

>>> -    return IFCVF_SUBSYS_VENDOR_ID;

>>> +    struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);

>>> +    struct pci_dev *pdev = adapter->pdev;

>>> +

>>> +    return pdev->subsystem_vendor;

>>>   }

>>

>>

>> While at this, I wonder if we can do something similar in 

>> get_device_id() if it could be simple deduced from some simple math 

>> from the pci device id?

>>

>> Thanks

> Hi Jason,

>

> IMHO, this implementation is just some memory read ops, I think other 

> implementations may not save many cpu cycles, an if cost at least 

> three cpu cycles.

>

> Thanks!



Well, I meant whehter you can deduce virtio device id from 
pdev->subsystem_device.

Thanks


>>

>>

>>>     static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev)

>>

>
Jason Wang March 11, 2021, 6:14 a.m. UTC | #6 
On 2021/3/11 12:23 下午, Zhu Lingshan wrote:
>

>

> On 3/11/2021 11:25 AM, Jason Wang wrote:

>>

>> On 2021/3/10 5:00 下午, Zhu Lingshan wrote:

>>> IFCVF driver probes multiple types of devices now,

>>> to distinguish the original device driven by IFCVF

>>> from others, it is renamed as "N3000".

>>>

>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

>>> ---

>>>   drivers/vdpa/ifcvf/ifcvf_base.h | 8 ++++----

>>>   drivers/vdpa/ifcvf/ifcvf_main.c | 8 ++++----

>>>   2 files changed, 8 insertions(+), 8 deletions(-)

>>>

>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h 

>>> b/drivers/vdpa/ifcvf/ifcvf_base.h

>>> index 75d9a8052039..794d1505d857 100644

>>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h

>>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h

>>> @@ -18,10 +18,10 @@

>>>   #include <uapi/linux/virtio_config.h>

>>>   #include <uapi/linux/virtio_pci.h>

>>>   -#define IFCVF_VENDOR_ID        0x1AF4

>>> -#define IFCVF_DEVICE_ID        0x1041

>>> -#define IFCVF_SUBSYS_VENDOR_ID    0x8086

>>> -#define IFCVF_SUBSYS_DEVICE_ID    0x001A

>>> +#define N3000_VENDOR_ID        0x1AF4

>>> +#define N3000_DEVICE_ID        0x1041

>>> +#define N3000_SUBSYS_VENDOR_ID    0x8086

>>> +#define N3000_SUBSYS_DEVICE_ID    0x001A

>>>     #define C5000X_PL_VENDOR_ID        0x1AF4

>>>   #define C5000X_PL_DEVICE_ID        0x1000

>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 

>>> b/drivers/vdpa/ifcvf/ifcvf_main.c

>>> index 26a2dab7ca66..fd5befc5cbcc 100644

>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c

>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c

>>> @@ -480,10 +480,10 @@ static void ifcvf_remove(struct pci_dev *pdev)

>>>   }

>>>     static struct pci_device_id ifcvf_pci_ids[] = {

>>> -    { PCI_DEVICE_SUB(IFCVF_VENDOR_ID,

>>> -        IFCVF_DEVICE_ID,

>>> -        IFCVF_SUBSYS_VENDOR_ID,

>>> -        IFCVF_SUBSYS_DEVICE_ID) },

>>> +    { PCI_DEVICE_SUB(N3000_VENDOR_ID,

>>> +             N3000_DEVICE_ID,

>>

>>

>> I am not sure the plan for Intel but I wonder if we can simply use 

>> PCI_ANY_ID for device id here. Otherewise you need to maintain a very 

>> long list of ids here.

>>

>> Thanks

> Hi Jason,

>

> Thanks! but maybe if we present a very simple and clear list like what 

> e1000 does can help the users understand what we support easily.

>

> Thanks!



That's fine.

Thanks


>>

>>

>>> + N3000_SUBSYS_VENDOR_ID,

>>> +             N3000_SUBSYS_DEVICE_ID) },

>>>       { PCI_DEVICE_SUB(C5000X_PL_VENDOR_ID,

>>>                C5000X_PL_DEVICE_ID,

>>>                C5000X_PL_SUBSYS_VENDOR_ID,

>>

>
Zhu Lingshan March 11, 2021, 7:34 a.m. UTC | #7 
On 3/11/2021 2:13 PM, Jason Wang wrote:
>

> On 2021/3/11 12:21 下午, Zhu Lingshan wrote:

>>

>>

>> On 3/11/2021 11:23 AM, Jason Wang wrote:

>>>

>>> On 2021/3/10 5:00 下午, Zhu Lingshan wrote:

>>>> In this commit, ifcvf_get_vendor_id() will return

>>>> a device specific vendor id of the probed pci device

>>>> than a hard code.

>>>>

>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

>>>> ---

>>>>   drivers/vdpa/ifcvf/ifcvf_main.c | 5 ++++-

>>>>   1 file changed, 4 insertions(+), 1 deletion(-)

>>>>

>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 

>>>> b/drivers/vdpa/ifcvf/ifcvf_main.c

>>>> index fa1af301cf55..e501ee07de17 100644

>>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c

>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c

>>>> @@ -324,7 +324,10 @@ static u32 ifcvf_vdpa_get_device_id(struct 

>>>> vdpa_device *vdpa_dev)

>>>>     static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev)

>>>>   {

>>>> -    return IFCVF_SUBSYS_VENDOR_ID;

>>>> +    struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);

>>>> +    struct pci_dev *pdev = adapter->pdev;

>>>> +

>>>> +    return pdev->subsystem_vendor;

>>>>   }

>>>

>>>

>>> While at this, I wonder if we can do something similar in 

>>> get_device_id() if it could be simple deduced from some simple math 

>>> from the pci device id?

>>>

>>> Thanks

>> Hi Jason,

>>

>> IMHO, this implementation is just some memory read ops, I think other 

>> implementations may not save many cpu cycles, an if cost at least 

>> three cpu cycles.

>>

>> Thanks!

>

>

> Well, I meant whehter you can deduce virtio device id from 

> pdev->subsystem_device.

>

> Thanks

Oh, sure, I get you
>

>

>>>

>>>

>>>>     static u32 ifcvf_vdpa_get_vq_align(struct vdpa_device *vdpa_dev)

>>>

>>

>