Re: [virtio-comment] [PATCH 5/5] virtio-pci: implement VIRTIO_F_QUEUE_STATE

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

On 9/13/2023 12:36 PM, Parav Pandit wrote:
> > From: Zhu, Lingshan <lingshan.zhu@intel.com>
> > Sent: Wednesday, September 13, 2023 9:51 AM
>
> > > VQ depth defines the VQ's limit.
> > still sounds like limitless and I will stop arguing this as you can see if there is
> > REALLY a queue can be limitless, we even don't need Multi-queue or RSS.
> If you see some value in limitless queue, please add one.
> I have not seen such construct until now and donât see the need for it.
It is you consider the admin vq is limitless, not me, and I don't agree 
with that.
And I stop arguing this, the point is clear.
> > > > > > If you say, that require multiple AQ, then how many should a vendor
> > > > provide?
> > > > > I didnât say multiple AQs must be used.
> > > > > It is same as NIC RQs.
> > > > don't you agree a single vq has its own performance limitations?
> > > For LM I donât see the limitation.
> > > The finite limit an AQ has, such limitation is no different than some register
> > write poll with one entry at a time per device.
> > see above, and we are implementing per device facilities.
> > > > In this series, it says:
> > > > +When setting SUSPEND, the driver MUST re-read \field{device status}
> > > > +to
> > > > ensure the SUSPEND bit is set.
> > > >
> > > > And this is nothing to do with scale.
> > > Hence, it is bringing same scale QOS limitation on register too that you claim
> > may be present in the AQ.
> > > And hence, I responded earlier that when most things are not done through
> > BAR, so there is no need to do suspend/resume via BAR either.
> > > And hence the mode setting command of [1] is just fine.
> > The bar registers are almost "triggers"
> > > > > On top of that once the device is SUSPENDED, it cannot accept some
> > > > > other
> > > > RESET_VQ command.
> > > > so as SiWei suggested, there will be a new feature bit introduced in
> > > > V2 for vq reset.
> > > VQ cannot be RESET after the device reset as you wrote.
> > It is device SUSPEND, not reset.
> Suspend means suspend of English language.
> It cannot accept more synchronous commands after that and not supposed to respond.
Please read the series, 2/5 patch describes SUSPEND behaviors.
> > > > > > > > It does not reside on the PF to migrate the VFs.
> > > > > > > Hence it does not scale and cannot do parallel operation within
> > > > > > > the VF,
> > > > unless
> > > > > > each register is replicated.
> > > > > > Why its not scale? It is a per device facility.
> > > > > Because the device needs to answer per device through some large
> > > > > scale
> > > > memory to fit in a response time.
> > > > Again, it is a per-device facility, and it is register based serve
> > > > the only one device itself.
> > > > And we do not plan to log the dirty pages in bar.
> > > Hence, there is no reason to wrap suspend resume on the BAR either.
> > > The mode setting admin command is just fine.
> > They are device status bits.
> And it doesn't have to be.
I don't get your comment.
Do you mean there should not be device status bits?
Challenging even DRIVER_OK is unreasonable?
> > > > > > Why do you need parallel operation against the LM facility?
> > > > > Because your downtime was 300msec for 1000 VMs.
> > > > the LM facility in this series is per-device, it only severs itself.
> > > And that single threading and single threading per VQ reset via single register
> > wont scale.
> > it is per-device facility, for example, on the VF, not the owner PF.
> And I repeatedly explained you that you never answered, is how such queue can work after device suspend.
> A weird device bifurcation is not supported by pci and not to be done in virtio.
Didn't you find the answer in my comments?

I repeated several times:
1) as described in this series, once SUSPEND, the device should present 
stabilized config space.
2) the device freeze both it's control path and data path. So we don't 
expect any queues functional since when SUSPEND ~ !SUSPEND.
3) but device status still operational because we may need to recover 
from failed LM or cancel the LM process.
4) We will introduce a new feature bit to allow reset vqs after SUSPEND.

Where so you see we expect the queue to work after SUSPEND?

Clear now?
> > see above and please feel free to reuse the basic facilities if you like in your AQ
> > LM
> The whole attitude that "We .." and use in "your" LM is just simply wrong.
why wrong?
> Please work towards collaborative design in technical committee.
This is what I am doing now, no? Or why I am talking to you?
> What you want to repeat was already posted, so take some time to review and utilize. If not, describe why it is not useful.
EOM
>