Re: [Xen-devel] [PATCH v6 3/9] x86/mm: add disallow_mask parameter to get_page_from_l1e

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.02.18 at 21:04, <wei.liu2@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c

You should have Cc-ed Tim.

> @@ -858,13 +858,21 @@ shadow_get_page_from_l1e(shadow_l1e_t sl1e, struct 
> domain *d, p2m_type_t type)
>      int res;
>      mfn_t mfn;
>      struct domain *owner;
> +    /* The disallow mask is taken from arch/x86/mm.c for HVM guest */
> +    uint32_t disallow_mask =
> +        ~(_PAGE_PRESENT | _PAGE_RW | _PAGE_USER | _PAGE_ACCESSED |
> +          _PAGE_DIRTY | _PAGE_AVAIL | _PAGE_AVAIL_HIGH | _PAGE_NX);
>  
> +    disallow_mask = (disallow_mask | _PAGE_GNTTAB) & ~_PAGE_GLOBAL;
> +    disallow_mask &= ~PAGE_CACHE_ATTRS;

If any of this is needed in the first place (see below), at least this
last line could be folded into the variable's initializer as it looks.

> +    ASSERT(is_hvm_domain(d));
>      ASSERT(!sh_l1e_is_magic(sl1e));
>  
>      if ( !shadow_mode_refcounts(d) )
>          return 1;
>  
> -    res = get_page_from_l1e(sl1e, d, d);
> +    res = get_page_from_l1e(sl1e, d, d, disallow_mask);
>  
>      // If a privileged domain is attempting to install a map of a page it 
> does
>      // not own, we let it succeed anyway.
> @@ -877,7 +885,7 @@ shadow_get_page_from_l1e(shadow_l1e_t sl1e, struct domain 
> *d, p2m_type_t type)
>      {
>          res = xsm_priv_mapping(XSM_TARGET, d, owner);
>          if ( !res ) {
> -            res = get_page_from_l1e(sl1e, d, owner);
> +            res = get_page_from_l1e(sl1e, d, owner, disallow_mask);

Since I dislike what patch 2 does I'd really like to first understand
what sense a disallow mask makes for a HVM guest (note that PV
guests won't make it here due to the shadow_mode_refcounts(d)
check just out of patch context.

And then even if this is necessary for HVM, I still don't really see
why the necessary logic can't remain in get_page_from_l1e().

Jan


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