Re: [Xen-devel] [PATCH v2 07/10] vm_event: Add vm_event_ng interface

[Jan Beulich <JBeulich@xxxxxxxx>]

On 31.07.2019 14:53, Petre Ovidiu PIRCALABU wrote:
> On Thu, 2019-07-18 at 14:44 +0000, Jan Beulich wrote:
>> On 18.07.2019 15:59, Petre Ovidiu PIRCALABU wrote:
>>> Before using xenforeignmemory_map_resource I investigated several
>>> different approaches:
>>> - Allocate the memory in hypervisor and xc_map_foreign_pages
>>> (doesn't
>>> work because you cannot "foreignmap" pages of your own domain.
>>> - Allocate the memory in guest, and map the allocated pages in XEN.
>>> To
>>> my knowledge there is no such API in linux to do this and the
>>> monitor
>>> application, as an userspace program, is not aware of the actual
>>> gfns
>>> for an allocated memory area.
>>>
>>> So, at this point the most promising solution is allocating the
>>> memory
>>> in XEN, sharing it with ID using share_xen_page_with_guest, and
>>> mapping
>>> it with xenforeignmemory_map_resource (with the flag
>>> XENMEM_rsrc_acq_caller_owned set)
>>
>> Which is fine - that's why Paul had introduced the generic interface.
>>
>>> To my understanding the cleanup sequence from
>>> gnttab_unpopulate_status_frames basically boils down to:
>>> 1. guest_physmap_remove_page
>>> 2. if ( test_and_clear_bit(_PGC_allocated, &pg->count_info) )
>>>          put_page(pg);
>>> 3. free_xenheap_page
>>
>> You're missing the crucial part of undoing step 2 if you find
>> that there are still references left to the page.
>>
>> And then, because of your use of vzalloc(), you can't use
>> free_xenheap_pages(), as that takes a (direct map) linear address
>> as input. It has to be free_domheap_pages() in your case.
>>
>>> My current implementation uses vzalloc instead of
>>> alloc_xenheap_pages
>>> and that's why I assumed vunmap and free_domheap_pages will suffice
>>> (I
>>> would have called vfree directly, but the temporary linked list
>>> that is
>>> used to hold the page references causes free_domheap_pages to
>>> crash)
>>>
>>> Do I also have to call guest_physmap_remove_page and put_page?
>>> (steps
>>> 1. and 2.)
>>
>> guest_physmap_remove_page() needs to be called for translated page-
>> owning domains if the page was ever added to their physmap. As long
>> as you avoid adding, you also don't need to remove. I don't recall
>> though whether a translated domain can access a resource without it
>> having a representation in its GFN space.
>>
> I've traced the GFN value for the shared MFN and it's invalid. It's set
> to INVALID_M2P_ENTRY in share_xen_page_with_guest, but I was expecting
> it to be set to valid value later on (e.g. xenmem_add_to_physmap).
> Am I missing something?

By "traced" do you mean "observed" (e.g. by way of logging) or "gone
through the code to verify it can't ever become valid"? In the latter
case you'd have proven what you want/need. Thinking about it though,
seeing how xenmem_add_to_physmap_one() works and assuming you indeed
don't add the page by other means, it looks to me as if you've done
more than is needed, as I've said already that you need to remove the
page only if it had been added.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel