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

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

On 11/3/2023 10:21 PM, Zhu, Lingshan wrote:
> On 11/3/2023 6:46 PM, Michael S. Tsirkin wrote:
> > On Fri, Nov 03, 2023 at 06:34:37PM +0800, Zhu Lingshan wrote:
> > > +\begin{lstlisting}
> > > +struct virtio_pci_dity_page_track {
> > > +ÂÂÂÂÂÂÂ u8 enable;ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ /* Read-Write */
> > > +ÂÂÂÂÂÂÂ u8 gra_power;ÂÂÂÂÂÂÂÂÂÂÂ /* Read-Write */
> > > +ÂÂÂÂÂÂÂ u8 reserved[2];
> > > +ÂÂÂÂÂÂÂ le32 {
> > > +ÂÂÂÂÂÂÂÂÂÂÂ pasid: 20;ÂÂÂÂÂÂÂÂÂÂ /* Read-Write */
> > > +ÂÂÂÂÂÂÂÂÂÂÂ reserved: 12;
> > > +ÂÂÂÂÂÂÂ };
> > > +ÂÂÂÂÂÂÂ le64 bitmap_addr;ÂÂÂÂÂÂÂ /* Read-Write */
> > > +ÂÂÂÂÂÂÂ le64 bitmap_length;ÂÂÂÂÂ /* Read-Write */
> > > +};
> > > +\end{lstlisting}
> > Okay, so it's a simple mailbox in config space. Which by itself is
> > probably a very reasonable idea - more or less what I suggested.
> > However, using such a generic facility just for the dirty bitmap seems
> > too limited. Please make it accept arbitrary commands. Reusing admin
> > command structure with a special "device itself" group sounds like one
> > way to do it.
> processing admin cmds in a cap may be too complex and overkill.
> we need to handle variable length of cmds, handle async returned 
> results, and so on.
>
> This struct seems easy and simple. And shall we use platform 
> facilities like vt-d
> to track dirty pages?
To demonstrate these issues, suppose we have a struct in a bar to 
process admin cmds:

struct virtio_admin_cmd {
ÂÂÂÂÂÂÂ u64 in_data_length;
ÂÂÂÂÂÂÂ u8 cmd_in_data[];
ÂÂÂÂÂÂÂ u64 out_data_length;
ÂÂÂÂÂÂÂ u8 cmd_out_data[];
ÂÂÂÂÂÂÂ u8 ret;
};

The problems are:
1) command_in_data and command_out data have variable length, so how 
many HW resource should be reserved in the bar?
2) To process the cmds in the bar, the device MAY need to read many 
registers in cmd_in_data[] and write many registers in cmd_out_data[],
which can be ineffective, this is not DMA.
3) a bar can only process one cmd at a time, and the driver can only 
issue another cmd after received an ret.
This process has to be synchronous IO, one cmd blocks another.
4) VF implementing a bar processing admin cmds conflicts with PF's admin vq.

So I think a bar or a cap processing admin cmds is way to complex and 
overkill.

Thanks