On Tue, Oct 24, 2023 at 06:27:04PM +0800, Zhu, Lingshan wrote:
>
>
> On 10/23/2023 7:32 PM, Michael S. Tsirkin wrote:
>
> On Mon, Oct 23, 2023 at 06:03:10PM +0800, Zhu, Lingshan wrote:
>
> config space, MMIO, registers work for years, what is wrong with them?
>
> Nothing as such. They don't seem to be appropriate for all use-case
> where people want to utilize virtio. I think a new transport
> will be needed to address these.
>
> New transport for new type of devices for sure, like transport vq for SIOV.
>
> I agree admin vq or admin cmds are useful in some use cases, that is
> another story, should be case by case.
>
> For now, let's don't talk about all-use cases, just for current task, for live
> migration.
>
> So IMHO, I still think we should use config space registers to control live
> migration process.
>
>
No because it forces integrating migration process with device driver.
Which is ok for some use-cases but not all of them. Find some other
control plane for this.
> Config space is control path, DMA is data-path, let's better not mix them,
> we never expect to use config space to transfer data.
>
> So we need DMA to transfer data, for example I take advantages of device DMA
> to logging dirty pages, This also applies to in-flight descriptors.
>
> As long as you do, I personally see little benefit to retrieve parts of
> state with memory mapped accesses.
>
> registers only control, and I personally believe a single register is much
> better
> than processing admin commands, more light-weight, more reliable, working
> for years.
>
> Yea. It would be, if we could do everything through that register.
> But we can't really. Migration has too much data to pass around
> for that to be reasonable.
>
> data are not transferred by registers, they only control.
>
> We transfer data by DMA, the device writes DMA dirty pages information(bitmap)
> to host isolated memory region.
>
If you do that then I don't see any reason not to use admin
commands for that - either through a vq or a simpler
interface.
>
> Config space interfaces are fundamental for virtio-pci.
>
>
> They are in fact fundamental to virtio. Multiple transports to
> use config space are also fundamental.
>
> I agree. So I also agree to build admin vq live migration solution based on our
> basic facilities, as Jason ever proposed.
I'm not sure it's even a vq. I suggest a minimal interface to send
admin commands. Could be used by migration, as transport, and more.
>
>
>
> And we are implementing virito live migration, not only for PCI.
>
> So both me and Jason keep repeating: We are implementing basic facilities,
> and the implementation is transport specific.
>
> But the register based facilities you proposed are extremely limited and
> seem to only work for migration. For example, it seems mostly useless for
> debugging because retrieving state is rather complex and would
> interfere with normal working of the device.
>
> If you want to prove the register controlling interfaces are extremely
> limited than admin vq or admin cmds,
> you are also proving config space registers are extremely limited than
> admin vq.
>
> Yes. Migration needs ability to pass large amounts of data around, and
> is too complex a functionality to work reliably without ability to
> report errors.
>
> what errors? when device DMA?
> missing some dirty pages? If the device can detect such errors, it can recover
> by itself,
> or how can driver fix this?
Not just pages, there's a lot of internal device state.
You fix for example by reporting that state does not work
for a current device, and guest can be restarted on migration
source.
> for control path, virtio uses re-read for many years and it works well.
Let's not even get started with how live migration currently "works
well". I happen to be familiar with it intimately. We tried to
maintain migration compatiblity as best we could and we tend to break it
every second release.
> I
> believe we have
> went through this issue before.
>
>
>
>
> So the question still here: do you want to replace current virtio-pci common
> cfg
> with admin vq or admin cmds?
>
> I think we need to add a new transport that will use admin commands.
> Which one to use would be up to a specific device.
>
> For new device type like SIOV, yes we need a new transport, transport vq.
>
> Let's focus on this live migration feature, if there are new features in the
> future
> requires admin vq, let's discuss when they proposed.
>
>
>
>
> And debug what? If you want to introduce more functionalities, we should
> discuss
> case by case.
>
> If debugging vq state, it is as easy as read queue_size, I don't see the
> limitations
> as queue_size work for years.
>
> No one reads queue_size. In fact for years we didn't have any debugging
> functionality and we are fine. If we are adding it, it really needs to
> be accessible when driver and device are wedged.
>
> OK, I don't disagree to implement new device debugging features.
>
> But let's focus on current live migration task.
>
>
>
>
> I still believe our goal is to do our best, with our capabilities, to build
> the most optimal virtio spec
> as we can do. Not other goals.
>
> Thanks
> Zhu Lingshan
>
>
>
> We have proposed to build admin vq based on our register solution, this can
> somehow even help tp resolve the nested issue.
>
> But I see the proposed has been rejected.
>
> I still believe the goal is to build a best spec, not "just can work" with
> limitations.
>
>
>
>
>
