Re: [Qemu-devel] [PATCH V7 3/5] docs: add pvrdma device documentation.

[Marcel Apfelbaum]

Hi Michael,

On 16/01/2018 4:06, Michael S. Tsirkin wrote:
> On Sun, Jan 14, 2018 at 11:01:45AM +0200, Marcel Apfelbaum wrote:
> > +5. Limitations
> > +==============
>
> Limitations are fine but need to cause init failures since users
> don't poke in the internal documentation.

Good point, thanks.

> > +- The device obviously is limited by the Guest Linux Driver features 
> > implementation
> > +  of the VMware device API.
> > +- Memory registration mechanism requires mremap for every page in the buffer 
> > in order
> > +  to map it to a contiguous virtual address range. Since this is not the data 
> > path
> > +  it should not matter much.
>
> Depends on the size of the region I guess. Did you try this with several
> gigabytes of memory?

On a "standard" laptop it takes about 0.3 secs for 0.5G and about 0.5 secs for 
1G.
There is no impact for memory regions less than 0.5G.

> If we are talking seconds of downtime,
> it's worth documenting so people aren't surprised. > Alternatively, limit the 
> max size of MR?

We already limited the MAX MR to ~ 134MB. (1<<27), however the user can
change the parameter at command line.
I will add to documentation to pay attention than using 1G MRs can
take half second to register.

> > +- The device requires target page size to be the same as the host page size.
>
> Pls add code to fail init when this is not the case.

Sure

> > +- QEMU cannot map guest RAM from a file descriptor if a pvrdma device is 
> > attached,
> > +  so it can't work with huge pages. The limitation will be addressed in the 
> > future,
> > +  however QEMU allocates Guest RAM with MADV_HUGEPAGE so if there are enough 
> > huge
> > +  pages available, QEMU will use them.
>
> Same here.

Will add for next re-spin.

> > +- As previously stated, migration is not supported yet, however with some 
> > hardware
> > +  support can be done.
>
> I don't see a migration blocker.

Will remove from the documentation, thanks.

Thank you for the review,
Marcel

>