RE: [virtio-dev] Re: [PATCH 0/5] virtio: introduce SUSPEND bit and vq state

Parav Pandit
2023-09-26T06:03:53Z

> From: Zhu, Lingshan <lingshan.zhu@intel.com>
> Sent: Tuesday, September 26, 2023 11:07 AM

> > 1. cover letter is missing the problem statement and use case
> I only reply to this section of comments, this does not mean I agree with you on
> your other statements, Instead I agree with Jason on his replies to you.
>
> In my cover letter:
> "The main usecase of these new facilities is Live Migration."
>
:)
Two letter word do not explain the use case of why is asking to mediating a native virtio device.

And yet you call is the basic facilities.
Anyways you know the misaligned response in email and cover letter is evident.

> Did you miss it?
No.
It misses the detail as I described in the theory of operation described in [1].

[1] https://lists.oasis-open.org/archives/virtio-comment/202309/msg00061.html

> > 2. why queue suspend and both device suspend are introduced, only one
> should be there. The design description is missing.
> there are no queue suspend, they are device suspend and vq state accessors.
> please read the patch if you want to comment.
> > 3. Even though it claims under some random basic facility, cover letter clearly
> states the main use case is "live migration".
> it is not random, they are precisely defined virtio basic facilities.
> > 4. Patch 4 is not needed at all. When device is suspended, it is _suspended_. It
> does not do any bifurcation.
> The device should not accept vq reset and the driver should reset vqs, please
> read previous discussions with MST and please don't ignore the conclusions
> > 5. only suspend bit of patch2 is not enough to cover P2P. One needs suspend
> and freeze both covered in series [1].
> we have discussed this for many times, P2P is out of virtio spec, do you want to
> mediate every PCI state/functionality?
> > 6. Finally the whole description of 1 to 4 need to be split in the device
> operation, so that both passthrough and medication can utilize it using admin
> cmd and otherwise.
> Do you see any reasons this solution can not be used for passthrough and
> mediation?
Right. Proposed solution does not meeting following requirements addressed in [1].

[1] https://lore.kernel.org/virtio-comment/20230906081637.32185-1-lingshan.zhu@intel.com/T/#m7efbaadbc73f033c2793d9eb1eb0afa210aae4be

[1] I replied to Jason in previous email.
I will repeat here. They are covered in [1].

1. Missing P2P support
2. Missing dirty page tracking
3. Incremental device context framework for short downtime
3.a Ability to do inflight descriptor tracking
4. Ability to do the work for multiple member devices in parallel.

> Or does features_OK work for passthrough or mediation? Any difference?
It does not work. Passthrough devices are not trapped by the hypervisor.

> > Since Zhu, told that dirty tracking and inflight descriptors will be done, I
> presume he will propose to do over admin q or command interface.
> > And since all can run over the admin commands, the plumbing done in 1 to 4
> can be made using admin commands.
> No
Such negative assertion does not help.
Explain why part, like how I explained above.

> >
> > Until now we could not establish creating yet another DMA interface that is
> better than q interface.
> > So ...
> > To me both the methods will start looking more converged to me over admin
> command and queues.
> I don't think so, again, we are introducing basic facilities and these facilities
> don't depend on or rely on admin vq.
If so, stop the work "live migration" from the cover letter.
Reliance of admin command (again not vq, be careful what you constantly claim).
Not reliance on admin queue or admin command does/does not make it basic facility.

Admin commands and queues are already in basic facilities section today.
So claiming that hey one is using admin commands that means it is non_basic facility is not correct.

> >
> > Passthrough will use them over owner device.
> > Mediation somehow need to do over member device.
> > Mediation will not use any device suspend command because it needs to
> keep bisecting everything.
> please read QEMU vhost live migration solution
Can you please share the pointer to it?

I am familiar with [2] and it does not require device suspend flow as things are bisected.

[2] https://qemu-project.gitlab.io/qemu/interop/vhost-user.html#introduction