Skip to contentSkip to conversation

PV hugepages - Xen patch

PATCH In review Dave McCracken 4 participants 12 messages Oct 2, 2008 – Oct 10, 2008

lore
Share & tools
lore

Dave McCracken

author
#0 lore raw

PATCH 1/2 PV hugepages - Xen patch

This patch enables support of hugepages in a pv Xen environment. It is against the latest xen unstable tree on http://xenbits.xensource.com.

The patch assumes the guest is passing a physically aligned hugepage. It does reference counting on all the underlying pages.

Dave McCracken
Oracle Corp.

./xen/include/asm-x86/x86_32/page.h renamed from /./xen/include/asm-x86/x86_32/page.h +1 -1
@@ -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)
./xen/include/asm-x86/x86_64/page.h renamed from /./xen/include/asm-x86/x86_64/page.h +1 -1
@@ -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)
./xen/arch/x86/mm.c renamed from /./xen/arch/x86/mm.c +34 -5
@@ -759,11 +759,29 @@ get_page_from_l2e(
         MEM_LOG("Bad L2 flags %x", l2e_get_flags(l2e) & L2_DISALLOW_MASK);
         return -EINVAL;
     }
+    if ( l2e_get_flags(l2e) & _PAGE_PSE ) {
+        unsigned long mfn = l2e_get_pfn(l2e);
+        unsigned long m, me;
+        struct page_info *page = mfn_to_page(mfn);
 
-    rc = get_page_and_type_from_pagenr(
-        l2e_get_pfn(l2e), PGT_l1_page_table, d, 0);
-    if ( unlikely(rc == -EINVAL) && get_l2_linear_pagetable(l2e, pfn, d) )
-        rc = 0;
+        rc = get_page(page, d);
+        if (unlikely(!rc)) {
+            return rc;
+        }
+
+        for (m = mfn+1, me = m + (L1_PAGETABLE_ENTRIES-1); m <= me; m++) {
+            get_page_from_pagenr(m, d);
+        }
+#ifdef __x86_64__
+        map_pages_to_xen((unsigned long)mfn_to_virt(mfn), mfn, L1_PAGETABLE_ENTRIES,
+                         PAGE_HYPERVISOR | l2e_get_flags(l2e));
+#endif
+    } else {
+        rc = get_page_and_type_from_pagenr(
+            l2e_get_pfn(l2e), PGT_l1_page_table, d, 0);
+        if ( unlikely(rc == -EINVAL) && get_l2_linear_pagetable(l2e, pfn, d) )
+            rc = 0;
+    }
 
     return rc;
 }

@@ -955,7 +973,18 @@ static int put_page_from_l2e(l2_pgentry_
     if ( (l2e_get_flags(l2e) & _PAGE_PRESENT) && 
          (l2e_get_pfn(l2e) != pfn) )
     {
-        put_page_and_type(l2e_get_page(l2e));
+        if (l2e_get_flags(l2e) & _PAGE_PSE) {
+            unsigned long mfn = l2e_get_pfn(l2e);
+            unsigned long m, me;
+            struct page_info *page = mfn_to_page(mfn);
+
+            for (m = mfn+1, me = m + (L1_PAGETABLE_ENTRIES-1); m <= me; m++) {
+                put_page(mfn_to_page(m));
+            }
+            put_page(page);
+        } else {
+            put_page_and_type(l2e_get_page(l2e));
+        }
         return 0;
     }
     return 1;

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw

Some issues:
* You need to check return value of get_page_from_pagenr() on every page of
the superpage. Any one of them can fail, causing you to undo your work so far and then fail.
* You need to get_page_type(PGT_writable) on every page if the superpage
mapping asserts _PAGE_RW. Otherwise the guest is getting write access without that being asserted in the reference counts.
* Look at get_page_from_l1e() for an example of how this is done for a
single page. You need to do similar work for every page of the super-page.
* This surely breaks save/restore, since the restore code is not
superpage-aware.

-- Keir

Quoted 13 lines · Dave McCracken

This patch enables support of hugepages in a pv Xen environment. It is against the latest xen unstable tree on http://xenbits.xensource.com.

The patch assumes the guest is passing a physically aligned hugepage. It does reference counting on all the underlying pages.

Dave McCracken
Oracle Corp.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

author
#2 lore raw
Quoted 9 lines · Keir Fraser

Some issues:
 * You need to check return value of get_page_from_pagenr() on every page of the superpage. Any one of them can fail, causing you to undo your work so far and then fail.
 * You need to get_page_type(PGT_writable) on every page if the superpage mapping asserts _PAGE_RW. Otherwise the guest is getting write access without that being asserted in the reference counts.
 * Look at get_page_from_l1e() for an example of how this is done for a single page. You need to do similar work for every page of the super-page.

Ok, here's a version of the patch with all these issues addressed.

Quoted 2 lines

 * This surely breaks save/restore, since the restore code is not superpage-aware.

I don't have this one solved yet. I'm working on it.

Dave McCracken

./xen/include/asm-x86/x86_32/page.h renamed from /./xen/include/asm-x86/x86_32/page.h +1 -1
@@ -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)
./xen/include/asm-x86/x86_64/page.h renamed from /./xen/include/asm-x86/x86_64/page.h +1 -1
@@ -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)
./xen/arch/x86/mm.c renamed from /./xen/arch/x86/mm.c +71 -9

111 lines not on this page · raw message

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#3 lore raw
Quoted 11 lines · Dave McCracken
Quoted 9 lines · Keir Fraser

Some issues:
 * You need to check return value of get_page_from_pagenr() on every page of the superpage. Any one of them can fail, causing you to undo your work so far and then fail.
 * You need to get_page_type(PGT_writable) on every page if the superpage mapping asserts _PAGE_RW. Otherwise the guest is getting write access without that being asserted in the reference counts.
 * Look at get_page_from_l1e() for an example of how this is done for a single page. You need to do similar work for every page of the super-page.

Quoted 6 lines

Ok, here's a version of the patch with all these issues addressed.

Quoted 2 lines

 * This surely breaks save/restore, since the restore code is not superpage-aware.

I don't have this one solved yet. I'm working on it.

Actually this is an interesting one. For a PV guest it may be in general unsolvable, since the target machine may not have allocatable 2MB extents. It may also screw live migration since 2MB is a very coarse granularity to do dirty-page tracking. One option: perhaps the PV kernel could shatter and then reconstruct (as best it can) superpage mappings across save/restore? I'm actually not sure what's for the best here. Perhaps just make 2MB mappings and save/restore mutually exclusive for now?

-- Keir

author
#4 lore raw
Quoted 7 lines · Keir Fraser
Quoted 5 lines · Dave McCracken
Quoted 2 lines · Keir Fraser

 * This surely breaks save/restore, since the restore code is not superpage-aware.

I don't have this one solved yet. I'm working on it.

Quoted 7 lines

Actually this is an interesting one. For a PV guest it may be in general unsolvable, since the target machine may not have allocatable 2MB extents. It may also screw live migration since 2MB is a very coarse granularity to do dirty-page tracking. One option: perhaps the PV kernel could shatter and then reconstruct (as best it can) superpage mappings across save/restore? I'm actually not sure what's for the best here. Perhaps just make 2MB mappings and save/restore mutually exclusive for now?

Yeah, that's what I'm finding. I think it's a good idea to document for now that hugepages don't work with save/restore. I'll continue to dig into it and try to figure out a scheme to make it work as a future enhancement.

Dave McCracken

#5 lore raw
Quoted 11 lines · Dave McCracken
Quoted 7 lines

Actually this is an interesting one. For a PV guest it may be in general unsolvable, since the target machine may not have allocatable 2MB extents. It may also screw live migration since 2MB is a very coarse granularity to do dirty-page tracking. One option: perhaps the PV kernel could shatter and then reconstruct (as best it can) superpage mappings across save/restore? I'm actually not sure what's for the best here. Perhaps just make 2MB mappings and save/restore mutually exclusive for now?

Yeah, that's what I'm finding. I think it's a good idea to document for now that hugepages don't work with save/restore. I'll continue to dig into it and try to figure out a scheme to make it work as a future enhancement.

Then PV superpage support must be a configuration option, and disabled by default.

-- Keir

author
#6 lore raw
Quoted 2 lines · Keir Fraser

Then PV superpage support must be a configuration option, and disabled by default.

I added a command line option to enable it. Is this what you had in mind?

Dave McCracken

./xen/include/asm-x86/x86_32/page.h renamed from /./xen/include/asm-x86/x86_32/page.h +1 -1
@@ -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)
./xen/include/asm-x86/x86_64/page.h renamed from /./xen/include/asm-x86/x86_64/page.h +1 -1
@@ -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)
./xen/arch/x86/mm.c renamed from /./xen/arch/x86/mm.c +77 -9

123 lines not on this page · raw message

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#7 lore raw
Quoted 5 lines · Dave McCracken
Quoted 2 lines · Keir Fraser

Then PV superpage support must be a configuration option, and disabled by default.

I added a command line option to enable it. Is this what you had in mind?

It'll certainly do, albeit rather coarse-grained.

-- Keir

#8 lore raw
In reply to Dave McCracken
Quoted 5 lines · Dave McCracken
Quoted 2 lines · Keir Fraser

Then PV superpage support must be a configuration option, and disabled by default.

I added a command line option to enable it. Is this what you had in mind?

Please fix the coding style (brace positions; white space around if/while/for headers; etc) and resubmit this and the Linux patch with a signed-off-by attribution.

-- Keir

#9 lore raw
In reply to Keir Fraser
Quoted 5 lines · Keir Fraser

Actually this is an interesting one. For a PV guest it may be in general unsolvable, since the target machine may not have allocatable 2MB extents. It may also screw live migration since 2MB is a very coarse granularity to do dirty-page tracking. One option: perhaps the PV kernel could shatter and then reconstruct (as best it can) superpage mappings across save/restore?

That means you need to notify the guest when you're starting a live migration, rather than just springing it on them at the last moment as we do now.

But shattering large pages all over the place is going to be pretty expensive, and possibly awkward if it suddenly needs to come up with a pile of pages for the new L1 entries.

J

#10 lore raw
Quoted 7 lines · Jeremy Fitzhardinge
Quoted 5 lines · Keir Fraser

Actually this is an interesting one. For a PV guest it may be in general unsolvable, since the target machine may not have allocatable 2MB extents. It may also screw live migration since 2MB is a very coarse granularity to do dirty-page tracking. One option: perhaps the PV kernel could shatter and then reconstruct (as best it can) superpage mappings across save/restore?

Quoted 7 lines

That means you need to notify the guest when you're starting a live migration, rather than just springing it on them at the last moment as we do now.

But shattering large pages all over the place is going to be pretty expensive, and possibly awkward if it suddenly needs to come up with a pile of pages for the new L1 entries.

Or you could just take the view this is a pre-migration capability check, and that admin (or mgmt app) must ensure sufficient free hugepages on the destination before attempting migration. If this isn't satisfied then XenD can just fail / abort the migration op and leave it running on original host.

Dainel

Signature · 4 lines
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
#11 lore raw
Quoted 5 lines · Daniel P. Berrange

Or you could just take the view this is a pre-migration capability check,
and that admin (or mgmt app) must ensure sufficient free hugepages on the destination before attempting migration. If this isn't satisfied then XenD can just fail / abort the migration op and leave it running on original
host.

And do something to prevent new hugepages from being allocated during the dirty logging phase?

J