Re: [Xen-devel] [PATCH 1/2] Xen PV support for hugepages

["Jan Beulich" <jbeulich@xxxxxxxxxx>]

>--- xen-unstable//./xen/include/asm-x86/x86_32/page.h  2008-07-17 
>09:49:27.000000000 -0500
>+++ xen-hpage/./xen/include/asm-x86/x86_32/page.h      2008-10-02 
>15:07:34.000000000 -0500
>@@ -112,7 +112,7 @@ extern unsigned int PAGE_HYPERVISOR_NOCA
>  * Disallow unused flag bits plus PAT/PSE, PCD, PWT and GLOBAL.
>  * Permit the NX bit if the hardware supports it.
>  */
>-#define BASE_DISALLOW_MASK (0xFFFFF198U & ~_PAGE_NX)
>+#define BASE_DISALLOW_MASK (0xFFFFF118U & ~_PAGE_NX)
> 
> #define L1_DISALLOW_MASK (BASE_DISALLOW_MASK | _PAGE_GNTTAB)
> #define L2_DISALLOW_MASK (BASE_DISALLOW_MASK)

You ought to or in the bit you removed from BASE_DISALLOW_MASK into
L1_DISALLOW_MASK.

>--- xen-unstable//./xen/include/asm-x86/x86_64/page.h  2008-10-02 
>14:23:17.000000000 -0500
>+++ xen-hpage/./xen/include/asm-x86/x86_64/page.h      2008-10-02 
>15:07:34.000000000 -0500
>@@ -112,7 +112,7 @@ typedef l4_pgentry_t root_pgentry_t;
>  * Permit the NX bit if the hardware supports it.
>  * Note that range [62:52] is available for software use on x86/64.
>  */
>-#define BASE_DISALLOW_MASK (0xFF800198U & ~_PAGE_NX)
>+#define BASE_DISALLOW_MASK (0xFF800118U & ~_PAGE_NX)
> 
> #define L1_DISALLOW_MASK (BASE_DISALLOW_MASK | _PAGE_GNTTAB)
> #define L2_DISALLOW_MASK (BASE_DISALLOW_MASK)

Same here, but also for L3 (unless you also support Gb pages) and L4.

>--- xen-unstable//./xen/arch/x86/mm.c  2008-10-02 14:23:17.000000000 -0500
>+++ xen-hpage/./xen/arch/x86/mm.c      2008-10-09 09:07:47.000000000 -0500
>@@ -160,6 +160,9 @@ unsigned long total_pages;
> 
> #define PAGE_CACHE_ATTRS (_PAGE_PAT|_PAGE_PCD|_PAGE_PWT)
> 
>+static int opt_allow_hugepage = 0;
>+boolean_param("allowhugepage", opt_allow_hugepage);
>+
> #define l1_disallow_mask(d)                                     \
>     ((d != dom_io) &&                                           \
>      (rangeset_is_empty((d)->iomem_caps) &&                     \

And you also need to adjust l1_disallow_mask() to honor
opt_allow_hugepage.

>+        rc = get_data_page(page, d, writeable);
>+        if ( unlikely(!rc) )
>+            return rc;
>+
>+        for ( m = mfn+1, me = m + (L1_PAGETABLE_ENTRIES-1); m <= me; m++ )

Isn't this off by one (i.e. shouldn't the condition be m < me)?

>+        map_pages_to_xen((unsigned long)mfn_to_virt(mfn), mfn, 
>L1_PAGETABLE_ENTRIES,
>+                         PAGE_HYPERVISOR | l2e_get_flags(l2e));

You need to translate flags to l1 notion here - map_pages_to_xen() expects
it this way. Even more, it's not valid to simply or in PAGE_HYPERVISOR - you 
need
to follow the same logic as used in get_page_from_l1e().

>+            for ( m = mfn+1, me = m + (L1_PAGETABLE_ENTRIES-1); m <= me; m++ )

Same as above.

Jan


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