Re: [PATCH V3 1/6] vDPA/ifcvf: get_config_size should return a value no greater than dev implementation

["Zhu, Lingshan" <lingshan.zhu@intel.com>]

On 7/13/2022 1:31 PM, Michael S. Tsirkin wrote:
> On Fri, Jul 01, 2022 at 09:28:21PM +0800, Zhu Lingshan wrote:
>> ifcvf_get_config_size() should return a virtio device type specific value,
>> however the ret_value should not be greater than the onboard size of
>> the device implementation. E.g., for virtio_net, config_size should be
>> the minimum value of sizeof(struct virtio_net_config) and the onboard
>> cap size.
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>> ---
>>   drivers/vdpa/ifcvf/ifcvf_base.c | 13 +++++++++++--
>>   drivers/vdpa/ifcvf/ifcvf_base.h |  2 ++
>>   2 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
>> index 48c4dadb0c7c..fb957b57941e 100644
>> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
>> @@ -128,6 +128,7 @@ int ifcvf_init_hw(struct ifcvf_hw *hw, struct pci_dev *pdev)
>>   			break;
>>   		case VIRTIO_PCI_CAP_DEVICE_CFG:
>>   			hw->dev_cfg = get_cap_addr(hw, &cap);
>> +			hw->cap_dev_config_size = le32_to_cpu(cap.length);
>>   			IFCVF_DBG(pdev, "hw->dev_cfg = %p\n", hw->dev_cfg);
>>   			break;
>>   		}
>> @@ -233,15 +234,23 @@ int ifcvf_verify_min_features(struct ifcvf_hw *hw, u64 features)
>>   u32 ifcvf_get_config_size(struct ifcvf_hw *hw)
>>   {
>>   	struct ifcvf_adapter *adapter;
>> +	u32 net_config_size = sizeof(struct virtio_net_config);
>> +	u32 blk_config_size = sizeof(struct virtio_blk_config);
>> +	u32 cap_size = hw->cap_dev_config_size;
>>   	u32 config_size;
>>   
>>   	adapter = vf_to_adapter(hw);
>> +	/* If the onboard device config space size is greater than
>> +	 * the size of struct virtio_net/blk_config, only the spec
>> +	 * implementing contents size is returned, this is very
>> +	 * unlikely, defensive programming.
>> +	 */
>>   	switch (hw->dev_type) {
>>   	case VIRTIO_ID_NET:
>> -		config_size = sizeof(struct virtio_net_config);
>> +		config_size = cap_size >= net_config_size ? net_config_size : cap_size;
>>   		break;
>>   	case VIRTIO_ID_BLOCK:
>> -		config_size = sizeof(struct virtio_blk_config);
>> +		config_size = cap_size >= blk_config_size ? blk_config_size : cap_size;
>>   		break;
>>   	default:
>>   		config_size = 0;
> There's a min macro for this.
yes, a min macro is better.

Thanks,
Zhu Lingshan
>
>> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
>> index 115b61f4924b..f5563f665cc6 100644
>> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
>> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
>> @@ -87,6 +87,8 @@ struct ifcvf_hw {
>>   	int config_irq;
>>   	int vqs_reused_irq;
>>   	u16 nr_vring;
>> +	/* VIRTIO_PCI_CAP_DEVICE_CFG size */
>> +	u32 cap_dev_config_size;
>>   };
>>   
>>   struct ifcvf_adapter {
>> -- 
>> 2.31.1