diff mbox series

[1/3] vDPA/ifcvf: deduce VIRTIO device ID when probe

Message ID 20210414091832.5132-2-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 deduces VIRTIO device ID as device type when probe,
then ifcvf_vdpa_get_device_id() can simply return the ID.
ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size()
can work properly based on the device ID.

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
 drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------
 2 files changed, 11 insertions(+), 12 deletions(-)

Comments

Jason Wang April 15, 2021, 3:30 a.m. UTC | #1
在 2021/4/14 下午5:18, Zhu Lingshan 写道:
> This commit deduces VIRTIO device ID as device type when probe,
> then ifcvf_vdpa_get_device_id() can simply return the ID.
> ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size()
> can work properly based on the device ID.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> ---
>   drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
>   drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------
>   2 files changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index b2eeb16b9c2c..1c04cd256fa7 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -84,6 +84,7 @@ struct ifcvf_hw {
>   	u32 notify_off_multiplier;
>   	u64 req_features;
>   	u64 hw_features;
> +	u32 dev_type;
>   	struct virtio_pci_common_cfg __iomem *common_cfg;
>   	void __iomem *net_cfg;
>   	struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index 44d7586019da..99b0a6b4c227 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev)
>   
>   static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev)
>   {
> -	struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);
> -	struct pci_dev *pdev = adapter->pdev;
> -	u32 ret = -ENODEV;
> -
> -	if (pdev->device < 0x1000 || pdev->device > 0x107f)
> -		return ret;
> -
> -	if (pdev->device < 0x1040)
> -		ret =  pdev->subsystem_device;
> -	else
> -		ret =  pdev->device - 0x1040;
> +	struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>   
> -	return ret;
> +	return vf->dev_type;
>   }
>   
>   static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev)
> @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	pci_set_drvdata(pdev, adapter);
>   
>   	vf = &adapter->vf;
> +	if (pdev->device < 0x1000 || pdev->device > 0x107f)
> +		return -EOPNOTSUPP;
> +
> +	if (pdev->device < 0x1040)
> +		vf->dev_type =  pdev->subsystem_device;
> +	else
> +		vf->dev_type =  pdev->device - 0x1040;


So a question here, is the device a transtional device or modern one?

If it's a transitonal one, can it swtich endianess automatically or not?

Thanks


> +
>   	vf->base = pcim_iomap_table(pdev);
>   
>   	adapter->pdev = pdev;
Zhu Lingshan April 15, 2021, 5:52 a.m. UTC | #2
On 4/15/2021 11:30 AM, Jason Wang wrote:
>
> 在 2021/4/14 下午5:18, Zhu Lingshan 写道:
>> This commit deduces VIRTIO device ID as device type when probe,
>> then ifcvf_vdpa_get_device_id() can simply return the ID.
>> ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size()
>> can work properly based on the device ID.
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>> ---
>>   drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
>>   drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------
>>   2 files changed, 11 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h 
>> b/drivers/vdpa/ifcvf/ifcvf_base.h
>> index b2eeb16b9c2c..1c04cd256fa7 100644
>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
>> @@ -84,6 +84,7 @@ struct ifcvf_hw {
>>       u32 notify_off_multiplier;
>>       u64 req_features;
>>       u64 hw_features;
>> +    u32 dev_type;
>>       struct virtio_pci_common_cfg __iomem *common_cfg;
>>       void __iomem *net_cfg;
>>       struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 
>> b/drivers/vdpa/ifcvf/ifcvf_main.c
>> index 44d7586019da..99b0a6b4c227 100644
>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
>> @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct 
>> vdpa_device *vdpa_dev)
>>     static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev)
>>   {
>> -    struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);
>> -    struct pci_dev *pdev = adapter->pdev;
>> -    u32 ret = -ENODEV;
>> -
>> -    if (pdev->device < 0x1000 || pdev->device > 0x107f)
>> -        return ret;
>> -
>> -    if (pdev->device < 0x1040)
>> -        ret =  pdev->subsystem_device;
>> -    else
>> -        ret =  pdev->device - 0x1040;
>> +    struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>   -    return ret;
>> +    return vf->dev_type;
>>   }
>>     static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev)
>> @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, 
>> const struct pci_device_id *id)
>>       pci_set_drvdata(pdev, adapter);
>>         vf = &adapter->vf;
>> +    if (pdev->device < 0x1000 || pdev->device > 0x107f)
>> +        return -EOPNOTSUPP;
>> +
>> +    if (pdev->device < 0x1040)
>> +        vf->dev_type =  pdev->subsystem_device;
>> +    else
>> +        vf->dev_type =  pdev->device - 0x1040;
>
>
> So a question here, is the device a transtional device or modern one?
>
> If it's a transitonal one, can it swtich endianess automatically or not?
>
> Thanks
Hi Jason,

This driver should drive both modern and transitional devices as we 
discussed before.
If it's a transitional one, it will act as a modern device by default, 
legacy mode is a fail-over path.
For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it must in 
modern mode.
I think we don't need to worry about endianess for legacy mode.

Thanks
Zhu Lingshan
>
>
>> +
>>       vf->base = pcim_iomap_table(pdev);
>>         adapter->pdev = pdev;
>
Jason Wang April 15, 2021, 6:30 a.m. UTC | #3
在 2021/4/15 下午1:52, Zhu Lingshan 写道:
>
>
> On 4/15/2021 11:30 AM, Jason Wang wrote:
>>
>> 在 2021/4/14 下午5:18, Zhu Lingshan 写道:
>>> This commit deduces VIRTIO device ID as device type when probe,
>>> then ifcvf_vdpa_get_device_id() can simply return the ID.
>>> ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size()
>>> can work properly based on the device ID.
>>>
>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>> ---
>>>   drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
>>>   drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------
>>>   2 files changed, 11 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h 
>>> b/drivers/vdpa/ifcvf/ifcvf_base.h
>>> index b2eeb16b9c2c..1c04cd256fa7 100644
>>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
>>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
>>> @@ -84,6 +84,7 @@ struct ifcvf_hw {
>>>       u32 notify_off_multiplier;
>>>       u64 req_features;
>>>       u64 hw_features;
>>> +    u32 dev_type;
>>>       struct virtio_pci_common_cfg __iomem *common_cfg;
>>>       void __iomem *net_cfg;
>>>       struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 
>>> b/drivers/vdpa/ifcvf/ifcvf_main.c
>>> index 44d7586019da..99b0a6b4c227 100644
>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
>>> @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct 
>>> vdpa_device *vdpa_dev)
>>>     static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev)
>>>   {
>>> -    struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);
>>> -    struct pci_dev *pdev = adapter->pdev;
>>> -    u32 ret = -ENODEV;
>>> -
>>> -    if (pdev->device < 0x1000 || pdev->device > 0x107f)
>>> -        return ret;
>>> -
>>> -    if (pdev->device < 0x1040)
>>> -        ret =  pdev->subsystem_device;
>>> -    else
>>> -        ret =  pdev->device -0x1040;
>>> +    struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>>   -    return ret;
>>> +    return vf->dev_type;
>>>   }
>>>     static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev)
>>> @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, 
>>> const struct pci_device_id *id)
>>>       pci_set_drvdata(pdev, adapter);
>>>         vf = &adapter->vf;
>>> +    if (pdev->device < 0x1000 || pdev->device > 0x107f)
>>> +        return -EOPNOTSUPP;
>>> +
>>> +    if (pdev->device < 0x1040)
>>> +        vf->dev_type =  pdev->subsystem_device;
>>> +    else
>>> +        vf->dev_type =  pdev->device - 0x1040;
>>
>>
>> So a question here, is the device a transtional device or modern one?
>>
>> If it's a transitonal one, can it swtich endianess automatically or not?
>>
>> Thanks
> Hi Jason,
>
> This driver should drive both modern and transitional devices as we 
> discussed before.
> If it's a transitional one, it will act as a modern device by default, 
> legacy mode is a fail-over path.


Note that legacy driver use native endian, support legacy driver 
requires the device to know native endian which I'm not sure your device 
can do that.

Thanks


> For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it must 
> in modern mode.
> I think we don't need to worry about endianess for legacy mode.
>
> Thanks
> Zhu Lingshan
>>
>>
>>> +
>>>       vf->base = pcim_iomap_table(pdev);
>>>         adapter->pdev = pdev;
>>
>
Zhu Lingshan April 15, 2021, 6:36 a.m. UTC | #4
On 4/15/2021 2:30 PM, Jason Wang wrote:
>
> 在 2021/4/15 下午1:52, Zhu Lingshan 写道:
>>
>>
>> On 4/15/2021 11:30 AM, Jason Wang wrote:
>>>
>>> 在 2021/4/14 下午5:18, Zhu Lingshan 写道:
>>>> This commit deduces VIRTIO device ID as device type when probe,
>>>> then ifcvf_vdpa_get_device_id() can simply return the ID.
>>>> ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size()
>>>> can work properly based on the device ID.
>>>>
>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>> ---
>>>>   drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
>>>>   drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------
>>>>   2 files changed, 11 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h 
>>>> b/drivers/vdpa/ifcvf/ifcvf_base.h
>>>> index b2eeb16b9c2c..1c04cd256fa7 100644
>>>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
>>>> @@ -84,6 +84,7 @@ struct ifcvf_hw {
>>>>       u32 notify_off_multiplier;
>>>>       u64 req_features;
>>>>       u64 hw_features;
>>>> +    u32 dev_type;
>>>>       struct virtio_pci_common_cfg __iomem *common_cfg;
>>>>       void __iomem *net_cfg;
>>>>       struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 
>>>> b/drivers/vdpa/ifcvf/ifcvf_main.c
>>>> index 44d7586019da..99b0a6b4c227 100644
>>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
>>>> @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct 
>>>> vdpa_device *vdpa_dev)
>>>>     static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev)
>>>>   {
>>>> -    struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);
>>>> -    struct pci_dev *pdev = adapter->pdev;
>>>> -    u32 ret = -ENODEV;
>>>> -
>>>> -    if (pdev->device < 0x1000 || pdev->device > 0x107f)
>>>> -        return ret;
>>>> -
>>>> -    if (pdev->device < 0x1040)
>>>> -        ret =  pdev->subsystem_device;
>>>> -    else
>>>> -        ret =  pdev->device -0x1040;
>>>> +    struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>>>   -    return ret;
>>>> +    return vf->dev_type;
>>>>   }
>>>>     static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev)
>>>> @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, 
>>>> const struct pci_device_id *id)
>>>>       pci_set_drvdata(pdev, adapter);
>>>>         vf = &adapter->vf;
>>>> +    if (pdev->device < 0x1000 || pdev->device > 0x107f)
>>>> +        return -EOPNOTSUPP;
>>>> +
>>>> +    if (pdev->device < 0x1040)
>>>> +        vf->dev_type =  pdev->subsystem_device;
>>>> +    else
>>>> +        vf->dev_type =  pdev->device - 0x1040;
>>>
>>>
>>> So a question here, is the device a transtional device or modern one?
>>>
>>> If it's a transitonal one, can it swtich endianess automatically or 
>>> not?
>>>
>>> Thanks
>> Hi Jason,
>>
>> This driver should drive both modern and transitional devices as we 
>> discussed before.
>> If it's a transitional one, it will act as a modern device by 
>> default, legacy mode is a fail-over path.
>
>
> Note that legacy driver use native endian, support legacy driver 
> requires the device to know native endian which I'm not sure your 
> device can do that.
>
> Thanks
Yes, legacy requires guest native endianess, I think we don't need to 
worry about this because our transitional device should work in modern 
mode by
default(legacy mode is the failover path we will never reach, 
get_features will fail if no ACCESS_PLATFORM), we don't support legacy 
device in vDPA.

Thanks
>
>
>> For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it must 
>> in modern mode.
>> I think we don't need to worry about endianess for legacy mode.
>>
>> Thanks
>> Zhu Lingshan
>>>
>>>
>>>> +
>>>>       vf->base = pcim_iomap_table(pdev);
>>>>         adapter->pdev = pdev;
>>>
>>
>
Jason Wang April 15, 2021, 7:16 a.m. UTC | #5
在 2021/4/15 下午2:36, Zhu Lingshan 写道:
>
>
> On 4/15/2021 2:30 PM, Jason Wang wrote:
>>
>> 在 2021/4/15 下午1:52, Zhu Lingshan 写道:
>>>
>>>
>>> On 4/15/2021 11:30 AM, Jason Wang wrote:
>>>>
>>>> 在 2021/4/14 下午5:18, Zhu Lingshan 写道:
>>>>> This commit deduces VIRTIO device ID as device type when probe,
>>>>> then ifcvf_vdpa_get_device_id() can simply return the ID.
>>>>> ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size()
>>>>> can work properly based on the device ID.
>>>>>
>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>>> ---
>>>>>   drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
>>>>>   drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------
>>>>>   2 files changed, 11 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h 
>>>>> b/drivers/vdpa/ifcvf/ifcvf_base.h
>>>>> index b2eeb16b9c2c..1c04cd256fa7 100644
>>>>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
>>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
>>>>> @@ -84,6 +84,7 @@ struct ifcvf_hw {
>>>>>       u32 notify_off_multiplier;
>>>>>       u64 req_features;
>>>>>       u64 hw_features;
>>>>> +    u32 dev_type;
>>>>>       struct virtio_pci_common_cfg __iomem *common_cfg;
>>>>>       void __iomem *net_cfg;
>>>>>       struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
>>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 
>>>>> b/drivers/vdpa/ifcvf/ifcvf_main.c
>>>>> index 44d7586019da..99b0a6b4c227 100644
>>>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
>>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
>>>>> @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct 
>>>>> vdpa_device *vdpa_dev)
>>>>>     static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev)
>>>>>   {
>>>>> -    struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);
>>>>> -    struct pci_dev *pdev = adapter->pdev;
>>>>> -    u32 ret = -ENODEV;
>>>>> -
>>>>> -    if (pdev->device < 0x1000 || pdev->device > 0x107f)
>>>>> -        return ret;
>>>>> -
>>>>> -    if (pdev->device < 0x1040)
>>>>> -        ret =  pdev->subsystem_device;
>>>>> -    else
>>>>> -        ret =  pdev->device-0x1040;
>>>>> +    struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>>>>   -    return ret;
>>>>> +    return vf->dev_type;
>>>>>   }
>>>>>     static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev)
>>>>> @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, 
>>>>> const struct pci_device_id *id)
>>>>>       pci_set_drvdata(pdev, adapter);
>>>>>         vf = &adapter->vf;
>>>>> +    if (pdev->device < 0x1000 || pdev->device > 0x107f)
>>>>> +        return -EOPNOTSUPP;
>>>>> +
>>>>> +    if (pdev->device < 0x1040)
>>>>> +        vf->dev_type =  pdev->subsystem_device;
>>>>> +    else
>>>>> +        vf->dev_type =  pdev->device - 0x1040;
>>>>
>>>>
>>>> So a question here, is the device a transtional device or modern one?
>>>>
>>>> If it's a transitonal one, can it swtich endianess automatically or 
>>>> not?
>>>>
>>>> Thanks
>>> Hi Jason,
>>>
>>> This driver should drive both modern and transitional devices as we 
>>> discussed before.
>>> If it's a transitional one, it will act as a modern device by 
>>> default, legacy mode is a fail-over path.
>>
>>
>> Note that legacy driver use native endian, support legacy driver 
>> requires the device to know native endian which I'm not sure your 
>> device can do that.
>>
>> Thanks
> Yes, legacy requires guest native endianess, I think we don't need to 
> worry about this because our transitional device should work in modern 
> mode by
> default(legacy mode is the failover path we will never reach, 
> get_features will fail if no ACCESS_PLATFORM), we don't support legacy 
> device in vDPA.
>
> Thanks


Ok, so I think it's better to add a comment here.

Thanks


>>
>>
>>> For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it must 
>>> in modern mode.
>>> I think we don't need to worry about endianess for legacy mode.
>>>
>>> Thanks
>>> Zhu Lingshan
>>>>
>>>>
>>>>> +
>>>>>       vf->base = pcim_iomap_table(pdev);
>>>>>         adapter->pdev = pdev;
>>>>
>>>
>>
>
Zhu Lingshan April 15, 2021, 7:23 a.m. UTC | #6
On 4/15/2021 3:16 PM, Jason Wang wrote:
>
> 在 2021/4/15 下午2:36, Zhu Lingshan 写道:
>>
>>
>> On 4/15/2021 2:30 PM, Jason Wang wrote:
>>>
>>> 在 2021/4/15 下午1:52, Zhu Lingshan 写道:
>>>>
>>>>
>>>> On 4/15/2021 11:30 AM, Jason Wang wrote:
>>>>>
>>>>> 在 2021/4/14 下午5:18, Zhu Lingshan 写道:
>>>>>> This commit deduces VIRTIO device ID as device type when probe,
>>>>>> then ifcvf_vdpa_get_device_id() can simply return the ID.
>>>>>> ifcvf_vdpa_get_features() and ifcvf_vdpa_get_config_size()
>>>>>> can work properly based on the device ID.
>>>>>>
>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>>>> ---
>>>>>>   drivers/vdpa/ifcvf/ifcvf_base.h |  1 +
>>>>>>   drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++------------
>>>>>>   2 files changed, 11 insertions(+), 12 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h 
>>>>>> b/drivers/vdpa/ifcvf/ifcvf_base.h
>>>>>> index b2eeb16b9c2c..1c04cd256fa7 100644
>>>>>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
>>>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
>>>>>> @@ -84,6 +84,7 @@ struct ifcvf_hw {
>>>>>>       u32 notify_off_multiplier;
>>>>>>       u64 req_features;
>>>>>>       u64 hw_features;
>>>>>> +    u32 dev_type;
>>>>>>       struct virtio_pci_common_cfg __iomem *common_cfg;
>>>>>>       void __iomem *net_cfg;
>>>>>>       struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
>>>>>> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 
>>>>>> b/drivers/vdpa/ifcvf/ifcvf_main.c
>>>>>> index 44d7586019da..99b0a6b4c227 100644
>>>>>> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
>>>>>> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
>>>>>> @@ -323,19 +323,9 @@ static u32 ifcvf_vdpa_get_generation(struct 
>>>>>> vdpa_device *vdpa_dev)
>>>>>>     static u32 ifcvf_vdpa_get_device_id(struct vdpa_device 
>>>>>> *vdpa_dev)
>>>>>>   {
>>>>>> -    struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);
>>>>>> -    struct pci_dev *pdev = adapter->pdev;
>>>>>> -    u32 ret = -ENODEV;
>>>>>> -
>>>>>> -    if (pdev->device < 0x1000 || pdev->device > 0x107f)
>>>>>> -        return ret;
>>>>>> -
>>>>>> -    if (pdev->device < 0x1040)
>>>>>> -        ret =  pdev->subsystem_device;
>>>>>> -    else
>>>>>> -        ret =  pdev->device-0x1040;
>>>>>> +    struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
>>>>>>   -    return ret;
>>>>>> +    return vf->dev_type;
>>>>>>   }
>>>>>>     static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device 
>>>>>> *vdpa_dev)
>>>>>> @@ -466,6 +456,14 @@ static int ifcvf_probe(struct pci_dev *pdev, 
>>>>>> const struct pci_device_id *id)
>>>>>>       pci_set_drvdata(pdev, adapter);
>>>>>>         vf = &adapter->vf;
>>>>>> +    if (pdev->device < 0x1000 || pdev->device > 0x107f)
>>>>>> +        return -EOPNOTSUPP;
>>>>>> +
>>>>>> +    if (pdev->device < 0x1040)
>>>>>> +        vf->dev_type =  pdev->subsystem_device;
>>>>>> +    else
>>>>>> +        vf->dev_type =  pdev->device - 0x1040;
>>>>>
>>>>>
>>>>> So a question here, is the device a transtional device or modern one?
>>>>>
>>>>> If it's a transitonal one, can it swtich endianess automatically 
>>>>> or not?
>>>>>
>>>>> Thanks
>>>> Hi Jason,
>>>>
>>>> This driver should drive both modern and transitional devices as we 
>>>> discussed before.
>>>> If it's a transitional one, it will act as a modern device by 
>>>> default, legacy mode is a fail-over path.
>>>
>>>
>>> Note that legacy driver use native endian, support legacy driver 
>>> requires the device to know native endian which I'm not sure your 
>>> device can do that.
>>>
>>> Thanks
>> Yes, legacy requires guest native endianess, I think we don't need to 
>> worry about this because our transitional device should work in 
>> modern mode by
>> default(legacy mode is the failover path we will never reach, 
>> get_features will fail if no ACCESS_PLATFORM), we don't support 
>> legacy device in vDPA.
>>
>> Thanks
>
>
> Ok, so I think it's better to add a comment here.
sure, will add a comment in V2

Thanks
>
> Thanks
>
>
>>>
>>>
>>>> For vDPA, it has to support VIRTIO_1 and ACCESS_PLATFORM, so it 
>>>> must in modern mode.
>>>> I think we don't need to worry about endianess for legacy mode.
>>>>
>>>> Thanks
>>>> Zhu Lingshan
>>>>>
>>>>>
>>>>>> +
>>>>>>       vf->base = pcim_iomap_table(pdev);
>>>>>>         adapter->pdev = pdev;
>>>>>
>>>>
>>>
>>
>
diff mbox series

Patch

diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
index b2eeb16b9c2c..1c04cd256fa7 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.h
+++ b/drivers/vdpa/ifcvf/ifcvf_base.h
@@ -84,6 +84,7 @@  struct ifcvf_hw {
 	u32 notify_off_multiplier;
 	u64 req_features;
 	u64 hw_features;
+	u32 dev_type;
 	struct virtio_pci_common_cfg __iomem *common_cfg;
 	void __iomem *net_cfg;
 	struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index 44d7586019da..99b0a6b4c227 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -323,19 +323,9 @@  static u32 ifcvf_vdpa_get_generation(struct vdpa_device *vdpa_dev)
 
 static u32 ifcvf_vdpa_get_device_id(struct vdpa_device *vdpa_dev)
 {
-	struct ifcvf_adapter *adapter = vdpa_to_adapter(vdpa_dev);
-	struct pci_dev *pdev = adapter->pdev;
-	u32 ret = -ENODEV;
-
-	if (pdev->device < 0x1000 || pdev->device > 0x107f)
-		return ret;
-
-	if (pdev->device < 0x1040)
-		ret =  pdev->subsystem_device;
-	else
-		ret =  pdev->device - 0x1040;
+	struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
 
-	return ret;
+	return vf->dev_type;
 }
 
 static u32 ifcvf_vdpa_get_vendor_id(struct vdpa_device *vdpa_dev)
@@ -466,6 +456,14 @@  static int ifcvf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	pci_set_drvdata(pdev, adapter);
 
 	vf = &adapter->vf;
+	if (pdev->device < 0x1000 || pdev->device > 0x107f)
+		return -EOPNOTSUPP;
+
+	if (pdev->device < 0x1040)
+		vf->dev_type =  pdev->subsystem_device;
+	else
+		vf->dev_type =  pdev->device - 0x1040;
+
 	vf->base = pcim_iomap_table(pdev);
 
 	adapter->pdev = pdev;