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

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

On 11/6/2023 12:12 AM, Michael S. Tsirkin wrote:
> 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?
No, when device reset, the device is expected to forget everything and
re-intialize.
> Is a full scan of all memory required until device reset is complete?
Who scan the memory? The device tracks its own dirty pages.
> 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?
When reset, how can we expect the LM progress continue running.

For example, when the device DMA writes something, then reset before 
sending an interrupt,
the DMA-ed pages should be lost as expected, right?
> > 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.
If FLR, then whole device reset, and this is PCI, not virtio.
As Jason pointed out, do you want to audit every PCI functionality?
> > 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.