Re: [Xen-devel] [PATCH v2 for-4.9] x86/mm: Fix incorrect unmapping of 2MB and 1GB pages

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Wed, May 10, 2017 at 12:13 PM, Igor Druzhinin
<igor.druzhinin@xxxxxxxxxx> wrote:
> The same set of functions is used to set as well as to clean
> P2M entries, except that for clean operations INVALID_MFN (~0UL)
> is passed as a parameter. Unfortunately, when calculating an
> appropriate target order for a particular mapping INVALID_MFN
> is not taken into account which leads to 4K page target order
> being set each time even for 2MB and 1GB mappings. This eventually
> breaks down an EPT structure irreversibly into 4K mappings which
> prevents consecutive high order mappings to this area.
>
> Signed-off-by: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
> ---
> Changes in v2:
> * changed mistakenly used mfn_valid() to mfn_eq()
> * aggregated gfn-mfn mask into one

Acked-by: George Dunlap <george.dunlap@xxxxxxxxxx>

This will need a release-ack from Julien as well.

 -George


>
> CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> CC: Kevin Tian <kevin.tian@xxxxxxxxx>
> CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>
> Bugfix intended for 4.9 release.
> ---
>  xen/arch/x86/mm/p2m-ept.c |  3 ++-
>  xen/arch/x86/mm/p2m.c     | 11 +++++++----
>  2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index f37a1f2..f98121d 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -681,6 +681,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, 
> mfn_t mfn,
>      ept_entry_t *table, *ept_entry = NULL;
>      unsigned long gfn_remainder = gfn;
>      unsigned int i, target = order / EPT_TABLE_ORDER;
> +    unsigned long fn_mask = !mfn_eq(mfn, INVALID_MFN) ? (gfn | mfn_x(mfn)) : 
> gfn;
>      int ret, rc = 0;
>      bool_t entry_written = 0;
>      bool_t direct_mmio = (p2mt == p2m_mmio_direct);
> @@ -701,7 +702,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, 
> mfn_t mfn,
>       * 2. gfn not exceeding guest physical address width.
>       * 3. passing a valid order.
>       */
> -    if ( ((gfn | mfn_x(mfn)) & ((1UL << order) - 1)) ||
> +    if ( (fn_mask & ((1UL << order) - 1)) ||
>           ((u64)gfn >> ((ept->wl + 1) * EPT_TABLE_ORDER)) ||
>           (order % EPT_TABLE_ORDER) )
>          return -EINVAL;
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index ae70a92..e902f1a 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -543,12 +543,15 @@ int p2m_set_entry(struct p2m_domain *p2m, unsigned long 
> gfn, mfn_t mfn,
>      while ( todo )
>      {
>          if ( hap_enabled(d) )
> -            order = (!((gfn | mfn_x(mfn) | todo) &
> -                       ((1ul << PAGE_ORDER_1G) - 1)) &&
> +        {
> +            unsigned long fn_mask = !mfn_eq(mfn, INVALID_MFN) ?
> +                                     (gfn | mfn_x(mfn) | todo) : (gfn | 
> todo);
> +
> +            order = (!(fn_mask & ((1ul << PAGE_ORDER_1G) - 1)) &&
>                       hap_has_1gb) ? PAGE_ORDER_1G :
> -                    (!((gfn | mfn_x(mfn) | todo) &
> -                       ((1ul << PAGE_ORDER_2M) - 1)) &&
> +                    (!(fn_mask & ((1ul << PAGE_ORDER_2M) - 1)) &&
>                       hap_has_2mb) ? PAGE_ORDER_2M : PAGE_ORDER_4K;
> +        }
>          else
>              order = 0;
>
> --
> 2.7.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel

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