On 01/02/2018 20:18, Eduardo Habkost wrote: > On Thu, Feb 01, 2018 at 07:58:10PM +0200, Marcel Apfelbaum wrote: >> On 01/02/2018 19:36, Eduardo Habkost wrote: >>> On Thu, Feb 01, 2018 at 07:12:45PM +0200, Michael S. Tsirkin wrote: >>>> On Thu, Feb 01, 2018 at 03:01:36PM -0200, Eduardo Habkost wrote: >>>>> On Thu, Feb 01, 2018 at 06:59:07PM +0200, Michael S. Tsirkin wrote: >>>>>> On Thu, Feb 01, 2018 at 02:57:39PM -0200, Eduardo Habkost wrote: >>>>>>> On Thu, Feb 01, 2018 at 06:48:54PM +0200, Michael S. Tsirkin wrote: >>>>>>>> On Thu, Feb 01, 2018 at 02:31:32PM -0200, Eduardo Habkost wrote: >>>>>>>>> On Thu, Feb 01, 2018 at 04:24:30PM +0200, Michael S. Tsirkin wrote: >>>>>>>>> [...] >>>>>>>>>> The full fix would be to allow QEMU to map a list of >>>>>>>>>> pages to a guest supplied IOVA. >>>>>>>>> >>>>>>>>> Thanks, that's what I expected. >>>>>>>>> >>>>>>>>> While this is not possible, the only requests I have for this >>>>>>>>> patch is that we clearly document: >>>>>>>>> * What's the only purpose of share=on on a host-memory-backend >>>>>>>>> object (due to pvrdma limitations). >>>>>>>>> * The potential undesirable side-effects of setting share=on. >>>>>>>>> * On the commit message and other comments, clearly distinguish >>>>>>>>> HVAs in the QEMU address-space from IOVAs, to avoid confusion. >>>>>>>> >>>>>>>> Looking forward, when we do support it, how will management find out >>>>>>>> it no longer needs to pass the share parameter? >>>>>>>> >>>>>>>> Further, if the side effects of the share parameter go away, >>>>>>>> how will it know these no longer hold? >>>>>>> >>>>>>> A query-host-capabilities or similar QMP command seems necessary >>>>>>> for that. >>>>>> >>>>>> Is anyone working on that? >>>>> >>>>> Not yet. >>>>> >>>>> -- >>>>> Eduardo >>>> >>>> Do these patches need to wait until we do have that command? >>> >>> I don't think so. The command will be needed only when >>> support for pvrdma without share=on gets implemented. >>> >>> Right now, all we need is clear documentation. >>> >>>> >>>> I'm thinking it's better to have "share=on required with rdma" >>>> and "hugetlbfs not supported with rdma" >>>> than the reverse, this way new hosts do not need to carry >>>> thus stuff around forever. >>> >>> What do you mean by "the reverse"? >>> >>> IIUC, the requirements/limitations are: >>> >>> * share=on required for pvrdma. Already documented and enforced >>> by pvrdma code in this series. >> >> Right. >> >>> * hugetlbfs not supported with rdma. Is this detected/reported by >>> QEMU? Is it documented? >> >> Yes, enforced by the pvrdma device initialization and documented in the >> corresponding pvrdma doc. >> >>> * side-effects of share=on. This is not detected nor documented, >>> and probably already applies to other memory backends. >>> * Nice to have: document when share=on is useful (answer: >>> because of pvrdma), when adding share=on support to >>> host-memory-backend. >>> >> >> The documentation is part of the pvrdma doc. >> What are the side-effects of share=on? I missed that. >> (share=on is new for the memory backed RAM, the file >> backed RAM already had the share parameter) >> >> One can just grep for "share=on" in the docs directory >> and can easily see the only current usage. But maybe will >> be more, maybe we don't want to limit it for now. >> >> I am planning to re-spin today/tomorrow before sending >> a pull-request, can you please point me on what documentation >> to add and what side-effects I should document? >> > > The full list of side-effects is not clear to me. For some of > them, see Documentation/vm/numa_memory_policy.txt on the kernel > tree. > > The documentation for memory backend options is at > qemu-options.hx. Maybe something like this, extending the > existing paragraph: > > The @option{share} boolean option determines whether the memory > region is marked as private to QEMU, or shared (mapped using > the MAP_SHARED flag). The latter allows a co-operating > external process to access the QEMU memory region. > > @option{share} is also required for pvrdma devices due to > limitations in the RDMA API provided by Linux. > > Setting share=on might affect the ability to configure NUMA > bindings for the memory backend under some circumstances, see > Documentation/vm/numa_memory_policy.txt on the Linux kernel > source tree for additional details. > > I hate to point users to low-level documentation on the kernel > tree, but it's better than nothing. > > We also need to list "share" as a valid option at the > "@item -object memory-backend-ram,[...]" line. >
Thanks for the help Eduardo, I'll be sure to update the docs as advised. Marcel