WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 1 of 3] p2m: Allow l2 pages to be replaced by sup

To: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 1 of 3] p2m: Allow l2 pages to be replaced by superpages
From: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Date: Wed, 19 Jan 2011 12:22:04 +0000
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 19 Jan 2011 04:22:57 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <366d675630fd6ecbd622.1295274993@elijah>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <patchbomb.1295274992@elijah> <366d675630fd6ecbd622.1295274993@elijah>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.20 (2009-06-14)
Hi, 

At 14:36 +0000 on 17 Jan (1295274993), George Dunlap wrote:
> # HG changeset patch
> # User George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> # Date 1295274250 0
> # Node ID 366d675630fd6ecbd6228426b3f7723d8a9dd944
> # Parent  75b6287626ee0b852d725543568001e99b13be5b
> p2m: Allow l2 pages to be replaced by superpages
> 
> Allow second-level p2m pages to be replaced with superpage entries,
> freeing the p2m table page properly.  This allows, for example, a
> sequence of 512 singleton zero pages to be replaced with a superpage
> populate-on-demand entry.

This problem became more general under your feet - xen 4.1 supports 1GiB
superpages as well, so the same thing needs to be fixed for them.  
(I understand that PoD only uses 2MiB superpages but to half-fix it
invites future bugs.)

Also, although in the EPT code you're careful to do all the flushing &c
before freeing the old page, in the vanilla p2m code you free the page
even before writing the new l2e!

Cheers,

Tim.

> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> 
> diff -r 75b6287626ee -r 366d675630fd xen/arch/x86/mm/hap/hap.c
> --- a/xen/arch/x86/mm/hap/hap.c       Fri Jan 14 16:38:51 2011 +0000
> +++ b/xen/arch/x86/mm/hap/hap.c       Mon Jan 17 14:24:10 2011 +0000
> @@ -333,9 +333,11 @@
>  
>      ASSERT(page_get_owner(pg) == d);
>      /* Should have just the one ref we gave it in alloc_p2m_page() */
> -    if ( (pg->count_info & PGC_count_mask) != 1 )
> -        HAP_ERROR("Odd p2m page count c=%#lx t=%"PRtype_info"\n",
> -                  pg->count_info, pg->u.inuse.type_info);
> +    if ( (pg->count_info & PGC_count_mask) != 1 ) {
> +        HAP_ERROR("Odd p2m page %p count c=%#lx t=%"PRtype_info"\n",
> +                     pg, pg->count_info, pg->u.inuse.type_info);
> +        WARN();
> +    }
>      pg->count_info &= ~PGC_count_mask;
>      /* Free should not decrement domain's total allocation, since
>       * these pages were allocated without an owner. */
> diff -r 75b6287626ee -r 366d675630fd xen/arch/x86/mm/hap/p2m-ept.c
> --- a/xen/arch/x86/mm/hap/p2m-ept.c   Fri Jan 14 16:38:51 2011 +0000
> +++ b/xen/arch/x86/mm/hap/p2m-ept.c   Mon Jan 17 14:24:10 2011 +0000
> @@ -317,6 +317,7 @@
>      int vtd_pte_present = 0;
>      int needs_sync = 1;
>      struct domain *d = p2m->domain;
> +    struct page_info *intermediate_table = NULL;
>  
>      /*
>       * the caller must make sure:
> @@ -369,6 +370,12 @@
>          /* No need to flush if the old entry wasn't valid */
>          if ( !is_epte_present(ept_entry) )
>              needs_sync = 0;
> +        else if ( order == 9 && ! ept_entry->sp )
> +        {
> +            /* We're replacing a non-SP page with a superpage.  Make sure to
> +             * handle freeing the table properly. */
> +            intermediate_table = mfn_to_page(ept_entry->mfn);
> +        }
>  
>          if ( mfn_valid(mfn_x(mfn)) || direct_mmio || p2m_is_paged(p2mt) ||
>               (p2mt == p2m_ram_paging_in_start) )
> @@ -487,6 +494,17 @@
>          }
>      }
>  
> +    if ( intermediate_table )
> +    {
> +        /* Release the old intermediate table.  This has to be the
> +           last thing we do, after the ept_sync_domain() and removal
> +           from the iommu tables, so as to avoid a potential
> +           use-after-free. */
> +
> +        page_list_del(intermediate_table, &d->arch.p2m->pages);
> +        d->arch.paging.free_page(d, intermediate_table);
> +    }
> +
>      return rv;
>  }
>  
> diff -r 75b6287626ee -r 366d675630fd xen/arch/x86/mm/p2m.c
> --- a/xen/arch/x86/mm/p2m.c   Fri Jan 14 16:38:51 2011 +0000
> +++ b/xen/arch/x86/mm/p2m.c   Mon Jan 17 14:24:10 2011 +0000
> @@ -1361,9 +1361,15 @@
>          if ( (l1e_get_flags(*p2m_entry) & _PAGE_PRESENT) &&
>               !(l1e_get_flags(*p2m_entry) & _PAGE_PSE) )
>          {
> -            P2M_ERROR("configure P2M table 4KB L2 entry with large page\n");
> -            domain_crash(p2m->domain);
> -            goto out;
> +            struct page_info *pg;
> +
> +            /* We're replacing a non-SP page with a superpage.  Make sure to
> +             * handle freeing the table properly. */
> +            pg = mfn_to_page(_mfn(l1e_get_pfn(*p2m_entry)));
> +            paging_write_p2m_entry(p2m->domain, gfn, p2m_entry, table_mfn,
> +                                   l1e_empty(), 2);
> +            page_list_del(pg, &p2m->pages);
> +            p2m->domain->arch.paging.free_page(p2m->domain, pg);
>          }
>          
>          ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel