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

[Parav Pandit <parav@nvidia.com>]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Friday, January 28, 2022 9:06 AM
> 
> å 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?
>
I guess, not because attr_mask is expected to grow and there may be unrelated attributes to it.
 
> 
> >
> >> 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.
> 
Device will follow some sane implementation.
If device which wants to support msix config for a VF, it will report right values in generic identify and get cmd of device.
A simple error check of bit mask will be enough in driver.

> 
> >
> >> 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?
>
No. the flow is:
1. AQ feature bit is negotiated
2. Create AQ
3. Do identify cmd and cache the device capabilities
A bit says that VF msix modification supported.
4. user queried all VFs current vector distribution (optimize this bit later, but consider this for now)
If attr.mask.bitX not set, fail the user request.
4. user requests to modify VF-A vectors from (8 to 4) uses attr.mask.bitX.
 
> 
> >
> >> 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?
>
No. not in each possible commands.
Identify command gives the capabilities supported in step #3 above to see if PFs on source and destination HV support it or not.
 
> 
> >
> >> 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.
Sure, it can be.
We want to avoid too many commands, if they can be clubbed in single command when starting a VF.