Re: [PATCH V2 6/6] virtio-pci: implement dirty page tracking

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Nov 03, 2023 at 03:47:34PM +0000, Parav Pandit wrote:
> > > [1]
> > > https://lists.oasis-open.org/archives/virtio-comment/202310/msg00475.h
> > > tml
> > you still need to explain why this does not work for pass-through. 
> It does not work for following reasons.
> 1. Because all the fields that put on the member device are not in direct control of the hypervisor.
> The device is directly controlled by the guest including the device status and when it resets the device all the things stored in the device are lost.

I think the idea is that when this gateway is in the device then
device reset has to trap. At a high level, ok. But then what?
Is a full scan of all memory required until device reset is complete?
Drivers currently tend to busy poll the reset register,
if this takes very long we might start seeing soft lockup
messages. What is the idea then? Maybe for this we need a separate
weaker reset that does not touch this capability?

>
> 2. the PCI FLR is clearing all the registers you exposed here.

Same problem, though FLR at least is expected to take a long time.


> 3. Endless expansion of config registers of dirty tracking is not scalable, as they are not init time registers not following the Appendix B guidelines.

> 4. bitmap based dirty tracking is not atomic between cpu and device.
> Hence, it is racy.

Well pcie atomics exist. Not sure whether it's reasonable to rely on
them. Any data on who common implementations are?

> 5. All the device context needed for passthrough based hypervisor for a device type specific is missing.
> All of those can be used for non-passthrough as well.
> [1] https://lists.oasis-open.org/archives/virtio-comment/202311/msg00085.html
> 
> > And I
> > remember this is a point-less topic as MST ever wants to mute another "pass-
> > through" thread.
> No. he did not say that.
> He meant to not endlessly debate which one is better.
> He clearly said, try to see if you can make multiple hypervisor model work.
> And your series shows a clear ignorance of his guidance.

I think you mean "ignoring" :)


> 
> > >
> > >>> +optionally the device is permitted to set the entire byte, which
> > >>> +encompasses
> > >> the relevant bit, to 1.
> > >>> +
> > >>> +The device MAY increase \field{gra_power} to reduce
> > \field{bitmap_length}.
> > >>> +
> > >>> +The device must ignore any writes to \field{pasid} if PASID
> > >>> +Extended Capability is absent or the PASID functionality is
> > >>> +disabled in PASID Extended Capability
> > >>
> > >> I have to say this is going to work very badly when the number of
> > >> dirty pages is
> > >> small: you will end up scanning and re-scanning all of bitmap.
> > >> And the resolution is apparently 8 pages? You have just multiplied
> > >> the migration bandwidth by a factor of 8.
> > > Yeah.
> > > And device does not even know previously reported pages are read by driver
> > or not. All guess work game for driver and device.
> > see my reply to him
> Please see above reply.