[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH V6 1/2] xen/gnttab: Store frame GFN in struct page_info on Arm
Hi Jan, On 24/06/2022 07:45, Jan Beulich wrote: On 23.06.2022 19:50, Julien Grall wrote:On 11/05/2022 19:47, Oleksandr Tyshchenko wrote:@@ -1505,7 +1507,23 @@ int xenmem_add_to_physmap_one( }/* Map at new location. */- rc = guest_physmap_add_entry(d, gfn, mfn, 0, t);+ if ( !p2m_is_ram(t) || !is_xen_heap_mfn(mfn) ) + rc = guest_physmap_add_entry(d, gfn, mfn, 0, t);I would expand the comment above to explain why you need a different path for xenheap mapped as RAM. AFAICT, this is because we need to call page_set_xenheap_gfn().+ else + { + struct p2m_domain *p2m = p2m_get_hostp2m(d); + + p2m_write_lock(p2m); + if ( gfn_eq(page_get_xenheap_gfn(mfn_to_page(mfn)), INVALID_GFN) )Sorry to only notice it now. This check will also change the behavior for XENMAPSPACE_shared_info. Now, we are only allowed to map the shared info once. I believe this is fine because AFAICT x86 already prevents it. But this is probably something that ought to be explained in the already long commit message.If by "prevent" you mean x86 unmaps the page from its earlier GFN, then yes. But this means that Arm would better follow that model instead of returning -EBUSY in this case. Just think of kexec-ing or a boot loader wanting to map shared info or grant table: There wouldn't necessarily be an explicit unmap. I spent some time to think about this. There is a potential big issue with implicit unmapping from its earlier GFN. Imagine the boot loader decided to map the page in place of a RAM. If the boot loader didn't unmap the page then when the OS map again, we would have a hole in the RAM. The OS may not know that and it may end up to use a page as RAM (and crash). The problem is the same for kexec and AFAIU that's why we need to use soft-reset when kexec-ing. So overall, I think we should prevent implicit unmap. So it would help to enforce that the bootloader (or any other components) clean-up behind themselves (i.e. unmap the page and populate if necessary). Cheers, -- Julien Grall
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |