Re: [Xen-devel] [PATCH] x86/mm: switch to new APIs in arch_init_memory

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.02.2020 11:42, Hongyan Xia wrote:
> From: Wei Liu <wei.liu2@xxxxxxxxxx>
> 
> Since we now map and unmap Xen PTE pages, we would like to track the
> lifetime of mappings so that 1) we do not dereference memory through a
> variable after it is unmapped, 2) we do not unmap more than once.
> Therefore, we introduce the UNMAP_DOMAIN_PAGE macro to nullify the
> variable after unmapping, and ignore NULL in unmap_domain_page.

To me this reads as if it was a 2nd paragraph explaining what needs
doing in order to achieve the main goal of the patch (supposedly
described in a 1st paragraph).

> --- a/xen/arch/x86/domain_page.c
> +++ b/xen/arch/x86/domain_page.c
> @@ -181,7 +181,7 @@ void unmap_domain_page(const void *ptr)
>      unsigned long va = (unsigned long)ptr, mfn, flags;
>      struct vcpu_maphash_entry *hashent;
>  
> -    if ( va >= DIRECTMAP_VIRT_START )
> +    if ( !va || va >= DIRECTMAP_VIRT_START )
>          return;

If we are to go with this, then I agree with Julien that this needs
mirroring to Arm, allowing common code to assume this behavior.
However, an alternative to this is to make ...

> --- a/xen/include/xen/domain_page.h
> +++ b/xen/include/xen/domain_page.h
> @@ -72,4 +72,9 @@ static inline void unmap_domain_page_global(const void *va) 
> {};
>  
>  #endif /* !CONFIG_DOMAIN_PAGE */
>  
> +#define UNMAP_DOMAIN_PAGE(p) do {   \
> +    unmap_domain_page(p);           \
> +    (p) = NULL;                     \
> +} while ( false )

... this avoid the call, leaving the function itself untouched.

Jan

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