Re: [PATCH v2 4/4] Add support for MSI-X vectors configuration for PCI VFs

[Jason Wang <jasowang@redhat.com>]

在 2022/1/28 上午11:30, Parav Pandit 写道:
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Friday, January 28, 2022 8:54 AM
>> vxlan part actually.
> Right.
>>> Similarly, we prefer the ability and flexibility to set more fields in one
>> command.
>>
>> I understood this motivation. But what I want to say is:
>>
>> struct virtio_cmd {
>>      u64 attr_mask;
>>      field_X;
>>      field_Y;
>>      field_Z;
>> }
>>
>> What's the default value if one of the X,Y, Z is not specified?
> If minimum default values are not provided, HV cannot proceed to do configuration and ENOSUPPORT error code is returned by kernel to user.


Do we need to document the minimum default value?


>
>> How to get those default values?
>>
> By doing identify or _get respective command.


This can only work if there's no attr_mask in get command. If the device 
can selectively provides us partial information we can't do 
compatibility check since we don't know its value.


>   
>> And suppose we add a new field
>>
>> struct virtio_cmd {
>>      u64 attr_mask;
>>      field_X;
>>      field_Y;
>>      field_Z;
>>      field_M;
>> }
>>
>> How can the driver know it can use field_M?
> If attribute mask is present for field_M, driver can use it.


So the driver need first use get then try to use set?


>
>> And if field_M is supported in src
>> but not dst, how can we keep the migration compatibility?
> This is HV level command to query and provision a VF.
> Migration compatibility is high level check where it will identify on which HV to migrate where I can provision X vectors for a VF.


By probing capability of attr_mask in each possible commands?


>
>> Would it be simple to just mandate all the fields in this case?
>>
> May be, attribute mask is basically to let expand structure without inventing new fields.


It could be done via new command.

Thanks


> For a given command few minimal attributes to be set and driver can verify, that if they are not, a given functionality is unsupported.
>