Re: [PATCH V7 2/2] xen/gnttab: Store frame GFN in struct page_info on Arm

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.07.2022 16:56, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> 
> Rework Arm implementation to store grant table frame GFN
> in struct page_info directly instead of keeping it in
> standalone status/shared arrays. This patch is based on
> the assumption that a grant table page is a xenheap page.
> 
> To cover 64-bit/40-bit IPA on Arm64/Arm32 we need the space
> to hold 52-bit/28-bit + extra bit value respectively. In order
> to not grow the size of struct page_info borrow the required
> amount of bits from type_info's count portion which current
> context won't suffer (currently only 1 bit is used on Arm).
> Please note, to minimize code changes and avoid introducing
> an extra #ifdef-s to the header, we keep the same amount of
> bits on both subarches, although the count portion on Arm64
> could be wider, so we waste some bits here.
> 
> Introduce corresponding PGT_* constructs and access macro
> page_get(set)_xenheap_gfn. Please note, all accesses to
> the GFN portion of type_info field should always be protected
> by the P2M lock. In case when it is not feasible to satisfy
> that requirement (risk of deadlock, lock inversion, etc)
> it is important to make sure that all non-protected updates
> to this field are atomic.
> As several non-protected read accesses still exist within
> current code (most calls to page_get_xenheap_gfn() are not
> protected by the P2M lock) the subsequent patch will introduce
> hardening code for p2m_remove_mapping() to be called with P2M
> lock held in order to check any difference between what is
> already mapped and what is requested to be ummapped.
> 
> Update existing gnttab macros to deal with GFN value according
> to new location. Also update the use of count portion of type_info
> field on Arm in share_xen_page_with_guest().
> 
> While at it, extend this simplified M2P-like approach for any
> xenheap pages which are proccessed in xenmem_add_to_physmap_one()
> except foreign ones. Update the code to set GFN portion after
> establishing new mapping for the xenheap page in said function
> and to clean GFN portion when putting a reference on that page
> in p2m_put_l3_page().
> 
> And for everything to work correctly introduce arch-specific
> initialization pattern PGT_TYPE_INFO_INITIALIZER to be applied
> to type_info field during initialization at alloc_heap_pages()
> and acquire_staticmem_pages(). The pattern's purpose on Arm
> is to clear the GFN portion before use, on x86 it is just
> a stub.
> 
> This patch is intended to fix the potential issue on Arm
> which might happen when remapping grant-table frame.
> A guest (or the toolstack) will unmap the grant-table frame
> using XENMEM_remove_physmap. This is a generic hypercall,
> so on x86, we are relying on the fact the M2P entry will
> be cleared on removal. For architecture without the M2P,
> the GFN would still be present in the grant frame/status
> array. So on the next call to map the page, we will end up to
> request the P2M to remove whatever mapping was the given GFN.
> This could well be another mapping.
> 
> Please note, this patch also changes the behavior how the shared_info
> page (which is xenheap RAM page) is mapped in xenmem_add_to_physmap_one().
> Now, we only allow to map the shared_info at once. The subsequent
> attempts to map it will result in -EBUSY. Doing that we mandate
> the caller to first unmap the page before mapping it again. This is
> to prevent Xen creating an unwanted hole in the P2M. For instance,
> this could happen if the firmware stole a RAM address for mapping
> the shared_info page into but forgot to unmap it afterwards.
> 
> Besides that, this patch simplifies arch code on Arm by
> removing arrays and corresponding management code and
> as the result gnttab_init_arch/gnttab_destroy_arch helpers
> and struct grant_table_arch become useless and can be
> dropped globally.
> 
> Suggested-by: Julien Grall <jgrall@xxxxxxxxxx>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>