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

[Marcel Apfelbaum <marcel@redhat.com>]

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

> 
>