Re: [Xen-devel] [PATCH 1/3] x86: nuke PV superpage option and code

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/07/17 15:02, Wei Liu wrote:
> @@ -1317,8 +1285,13 @@ static int put_page_from_l2e(l2_pgentry_t l2e, 
> unsigned long pfn)
>          return 1;
>  
>      if ( l2e_get_flags(l2e) & _PAGE_PSE )
> -        put_superpage(l2e_get_pfn(l2e));
> -    else
> +    {
> +        struct page_info *page = mfn_to_page(l2e_get_pfn(l2e));
> +        unsigned int i;
> +
> +        for ( i = 0; i < (1u << PAGETABLE_ORDER); i++, page++ )
> +            put_page_and_type(page);

With the removal of PV superpages, can this _PAGE_PSE check ever be hit?

> +    } else
>          put_page_and_type(l2e_get_page(l2e));
>  
>      return 0;
> diff --git a/xen/include/asm-x86/guest_pt.h b/xen/include/asm-x86/guest_pt.h
> index 72126d58d5..af18c90efa 100644
> --- a/xen/include/asm-x86/guest_pt.h
> +++ b/xen/include/asm-x86/guest_pt.h
> @@ -210,7 +210,7 @@ static inline bool guest_can_use_l2_superpages(const 
> struct vcpu *v)
>       * It's also used in the dummy PT for vcpus with CR0.PG cleared.
>       */
>      return (is_pv_vcpu(v)
> -            ? opt_allow_superpage
> +            ? false

This raises an interesting question.  A PV guest likely has 2M
superpages in the M2P mapping, irrespective of whether the PV guest is
permitted to create its own 2M superpages or not.

In this context, I think it is a layering violation.  The purpose of
guest_walk() is to match what hardware does, whereas this check is
superimposing PV policy.

I will submit a patch changing this in isolation, along with suitable
justification.

> diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
> index 44e86d6a1f..6dcf5e9ca8 100644
> --- a/xen/include/asm-x86/paging.h
> +++ b/xen/include/asm-x86/paging.h
> @@ -372,7 +372,7 @@ static inline unsigned int paging_max_paddr_bits(const 
> struct domain *d)
>      unsigned int bits = paging_mode_hap(d) ? hap_paddr_bits : paddr_bits;
>  
>      if ( !IS_ENABLED(BIGMEM) && paging_mode_shadow(d) &&
> -         (!is_pv_domain(d) || opt_allow_superpage) )
> +         !is_pv_domain(d) )

Reflow onto the previous line?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel