Re: [PATCHv1 3/3] x86/ept: defer the invalidation until the p2m lock is released

["Jan Beulich" <JBeulich@suse.com>]

>>> On 06.11.15 at 18:37, <david.vrabel@citrix.com> wrote:
> --- a/xen/arch/x86/mm/mm-locks.h
> +++ b/xen/arch/x86/mm/mm-locks.h
> @@ -263,14 +263,21 @@ declare_mm_lock(altp2mlist)
>   */
>  
>  declare_mm_rwlock(altp2m);
> -#define p2m_lock(p)                         \
> -{                                           \
> -    if ( p2m_is_altp2m(p) )                 \
> -        mm_write_lock(altp2m, &(p)->lock);  \
> -    else                                    \
> -        mm_write_lock(p2m, &(p)->lock);     \
> -}
> -#define p2m_unlock(p)         mm_write_unlock(&(p)->lock);
> +#define p2m_lock(p)                             \
> +    do {                                        \
> +        if ( p2m_is_altp2m(p) )                 \
> +            mm_write_lock(altp2m, &(p)->lock);  \
> +        else                                    \
> +            mm_write_lock(p2m, &(p)->lock);     \
> +        (p)->defer_flush++;                     \
> +    } while (0)
> +#define p2m_unlock(p)                                                   \
> +    do {                                                                \
> +        bool_t need_flush = --(p)->defer_flush == 0 && (p)->need_flush; \
> +        mm_write_unlock(&(p)->lock);                                    \
> +        if (need_flush && (p)->flush)                                   \

Coding style. Also couldn't you imply (or ASSERT()) ->flush to be
non-NULL when need_flush is true?

> +            (p)->flush(p);                                              \

The p2m lock guards EPT's synced_mask afaict, and hence dropping
the lock before calling ->flush() breaks things.

> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -504,6 +504,26 @@ void p2m_free_ptp(struct p2m_domain *p2m, struct page_info *pg)
>      return;
>  }
>  
> +DEFINE_PER_CPU(struct page_list_head, p2m_deferred_free_pages);

static

Jan