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

[Petre Ovidiu PIRCALABU <ppircalabu@xxxxxxxxxxxxxxx>]

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.
> 
> You definitely need to do step 2 (which is to undo the respective
> part of share_xen_page_with_guest(), and you _also_ need to clear
> the page owner (undoing the other part of what that function has
> done). And then, as said above, you need to check that the page
> has no references left on it, and correctly deal with the case when
> it still has some.
> 
> On the whole gnttab_unpopulate_status_frames() is very unfortunate
> to have the way it is, including the various domain_crash()-es. It
> was merely the only way we could see when dealing with XSA-255. I
> would strongly recommend against new code trying to go this same
> route, unless we introduce a generic and clean/safe function
> inverting share_xen_page_with_guest() (which I've tried in the past
> and failed).
> 
> Jan

Thank-you very much for explaining this to me. I will update the
cleanup procedure and let you know the outcome.

Best regards,
Petre

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