Skip to contentSkip to conversation

support 1gb pages in guest page table walker

PATCH In review Christoph Egger 3 participants 11 messages Jul 22, 2011 – Jul 28, 2011

lore
Share & tools
lore

Christoph Egger

author
#0 lore raw

PATCH 1/1 support 1gb pages in guest page table walker

Support 1gb pages in guest page table walker. This is needed for superpage support for nested virtualization.

Signed-off-by Christoph Egger <[email protected]>
Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
xen/arch/x86/mm/guest_walk.c +60 -11
diff -r 4f2c59fb28e6 xen/arch/x86/mm/guest_walk.c
@@ -134,7 +134,8 @@ guest_walk_tables(struct vcpu *v, struct
     guest_l4e_t *l4p;
 #endif
     uint32_t gflags, mflags, iflags, rc = 0;
-    int pse, smep;
+    int smep;
+    bool_t pse1G = 0, pse2M = 0;
 
     perfc_incr(guest_walk);
     memset(gw, 0, sizeof(*gw));

@@ -214,15 +215,56 @@ guest_walk_tables(struct vcpu *v, struct
 
 #endif /* All levels... */
 
+#if GUEST_PAGING_LEVELS >= 3 /* PAE or 64... */
+    pse1G = (guest_supports_1G_superpages(v) && 
+           (guest_l3e_get_flags(gw->l3e) & _PAGE_PSE)); 
+
+    if ( pse1G )
+    {
+        /* Special case: this guest VA is in a PSE superpage, so there's
+         * no guest l1e.  We make one up so that the propagation code
+         * can generate a shadow l1 table.  Start with the gfn of the 
+         * first 4k-page of the superpage. */
+        gfn_t start = guest_l2e_get_gfn(gw->l2e);
+        /* Grant full access in the l1e, since all the guest entry's 
+         * access controls are enforced in the shadow l2e. */
+        int flags = (_PAGE_PRESENT|_PAGE_USER|_PAGE_RW|
+                     _PAGE_ACCESSED|_PAGE_DIRTY);
+        /* Import cache-control bits. Note that _PAGE_PAT is actually
+         * _PAGE_PSE, and it is always set. We will clear it in case
+         * _PAGE_PSE_PAT (bit 12, i.e. first bit of gfn) is clear. */
+        flags |= (guest_l2e_get_flags(gw->l2e)
+                  & (_PAGE_PAT|_PAGE_PWT|_PAGE_PCD));
+        if ( !(gfn_x(start) & 1) )
+            /* _PAGE_PSE_PAT not set: remove _PAGE_PAT from flags. */
+            flags &= ~_PAGE_PAT;
+
+#define GUEST_L3_GFN_ALIGN (1 << (GUEST_L3_PAGETABLE_SHIFT - \
+                                  GUEST_L1_PAGETABLE_SHIFT))
+        if ( gfn_x(start) & (GUEST_L3_GFN_ALIGN - 1) & ~0x1 )
+        {
+            rc |= _PAGE_INVALID_BITS;
+        }
+
+        /* Increment the pfn by the right number of 4k pages.  
+         * Mask out PAT and invalid bits. */
+        start = _gfn((gfn_x(start) & ~(GUEST_L3_GFN_ALIGN - 1)) +
+                     guest_l1_table_offset(va));
+        gw->l1e = guest_l1e_from_gfn(start, flags);
+        gw->l2mfn = gw->l1mfn = _mfn(INVALID_MFN);
+        goto set_ad;
+    }
+#endif
+
     gflags = guest_l2e_get_flags(gw->l2e) ^ iflags;
     rc |= ((gflags & mflags) ^ mflags);
     if ( rc & _PAGE_PRESENT )
         goto out;
 
-    pse = (guest_supports_superpages(v) && 
+    pse2M = (guest_supports_superpages(v) && 
            (guest_l2e_get_flags(gw->l2e) & _PAGE_PSE)); 
 
-    if ( pse )
+    if ( pse2M )
     {
         /* Special case: this guest VA is in a PSE superpage, so there's
          * no guest l1e.  We make one up so that the propagation code

@@ -290,22 +332,29 @@ guest_walk_tables(struct vcpu *v, struct
      * success.  Although the PRMs say higher-level _PAGE_ACCESSED bits
      * get set whenever a lower-level PT is used, at least some hardware
      * walkers behave this way. */
+#if GUEST_PAGING_LEVELS >= 3 /* PAE or 64... */
+set_ad:
+#endif
     if ( rc == 0 ) 
     {
 #if GUEST_PAGING_LEVELS == 4 /* 64-bit only... */
         if ( set_ad_bits(l4p + guest_l4_table_offset(va), &gw->l4e, 0) )
             paging_mark_dirty(d, mfn_x(gw->l4mfn));
-        if ( set_ad_bits(l3p + guest_l3_table_offset(va), &gw->l3e, 0) )
+        if ( set_ad_bits(l3p + guest_l3_table_offset(va), &gw->l3e,
+                         (pse1G && (pfec & PFEC_write_access))) )
             paging_mark_dirty(d, mfn_x(gw->l3mfn));
 #endif
-        if ( set_ad_bits(l2p + guest_l2_table_offset(va), &gw->l2e,
-                         (pse && (pfec & PFEC_write_access))) )
-            paging_mark_dirty(d, mfn_x(gw->l2mfn));            
-        if ( !pse ) 
+        if ( !pse1G ) 
         {
-            if ( set_ad_bits(l1p + guest_l1_table_offset(va), &gw->l1e, 
-                             (pfec & PFEC_write_access)) )
-                paging_mark_dirty(d, mfn_x(gw->l1mfn));
+            if ( set_ad_bits(l2p + guest_l2_table_offset(va), &gw->l2e,
+                             (pse2M && (pfec & PFEC_write_access))) )
+                paging_mark_dirty(d, mfn_x(gw->l2mfn));            
+            if ( !pse2M ) 
+            {
+                if ( set_ad_bits(l1p + guest_l1_table_offset(va), &gw->l1e, 
+                                 (pfec & PFEC_write_access)) )
+                    paging_mark_dirty(d, mfn_x(gw->l1mfn));
+            }
         }
     }
xen/include/asm-x86/guest_pt.h +11 -0
diff -r 4f2c59fb28e6 xen/include/asm-x86/guest_pt.h
@@ -194,6 +194,17 @@ guest_supports_superpages(struct vcpu *v
 }
 
 static inline int
+guest_supports_1G_superpages(struct vcpu *v)
+{
+    if (!guest_supports_superpages(v))
+        return 0;
+
+    return (GUEST_PAGING_LEVELS >= 3
+           && cpu_has_page1gb
+           && hvm_long_mode_enabled(v));
+}
+
+static inline int
 guest_supports_nx(struct vcpu *v)
 {
     if ( GUEST_PAGING_LEVELS == 2 || !cpu_has_nx )

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

#1 lore raw
Quoted 2 lines
Quoted 1 line
Quoted 1 line · Christoph Egger

Support 1gb pages in guest page table walker.

Why do you do this for GUEST_PAGING_LEVELS >= 3 rather than GUEST_PAGING_LEVELS >= 4?

Jan

Quoted 4 lines

This is needed for superpage support for
nested virtualization.

Signed-off-by Christoph Egger <[email protected]>
author
#2 lore raw
Quoted 5 lines · Jan Beulich
Quoted 2 lines
Quoted 1 line
Quoted 1 line · Christoph Egger

Support 1gb pages in guest page table walker.

Why do you do this for GUEST_PAGING_LEVELS>= 3 rather than GUEST_PAGING_LEVELS>= 4?

Xen can run in 32bit PAE mode but the guest in 64bit mode. We can still change this when 32bit support is actually dropped.

Christoph

Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
#3 lore raw
Quoted 9 lines
Quoted 1 line
Quoted 1 line · Christoph Egger
Quoted 5 lines · Jan Beulich
Quoted 2 lines
Quoted 1 line
Quoted 1 line · Christoph Egger

Support 1gb pages in guest page table walker.

Why do you do this for GUEST_PAGING_LEVELS>= 3 rather than GUEST_PAGING_LEVELS>= 4?

Xen can run in 32bit PAE mode but the guest in 64bit mode.

But that is what distinguishes GUEST_PAGING_LEVELS from CONFIG_PAGING_LEVELS - you're allowing a PAE *guest* to use 1Gb mappings, no matter what host it runs on.

Jan

Quoted 4 lines

We can still change this when 32bit support is actually dropped.

Christoph

author
#4 lore raw
Quoted 5 lines · Jan Beulich
Quoted 5 lines
Quoted 1 line
Quoted 1 line · Christoph Egger
Quoted 3 lines · Jan Beulich
Quoted 2 lines
Quoted 1 line
Quoted 1 line · Christoph Egger

Support 1gb pages in guest page table walker.

Quoted 9 lines
Quoted 4 lines
Quoted 2 lines

Why do you do this for GUEST_PAGING_LEVELS>= 3 rather than GUEST_PAGING_LEVELS>= 4?

Xen can run in 32bit PAE mode but the guest in 64bit mode.

But that is what distinguishes GUEST_PAGING_LEVELS from CONFIG_PAGING_LEVELS - you're allowing a PAE *guest* to use 1Gb mappings, no matter what host it runs on.

Oh, then I mixed those two up. New fixed patch attached. Thank you for your review.

Signed-off-by Christoph Egger <[email protected]>
Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
xen/arch/x86/mm/guest_walk.c +60 -11
diff -r 4f2c59fb28e6 xen/arch/x86/mm/guest_walk.c
@@ -134,7 +134,8 @@ guest_walk_tables(struct vcpu *v, struct
     guest_l4e_t *l4p;
 #endif
     uint32_t gflags, mflags, iflags, rc = 0;
-    int pse, smep;
+    int smep;
+    bool_t pse1G = 0, pse2M = 0;
 
     perfc_incr(guest_walk);
     memset(gw, 0, sizeof(*gw));

@@ -214,15 +215,56 @@ guest_walk_tables(struct vcpu *v, struct
 
 #endif /* All levels... */
 
+#if GUEST_PAGING_LEVELS >= 4 /* 64-bit only... */
+    pse1G = (guest_supports_1G_superpages(v) && 
+           (guest_l3e_get_flags(gw->l3e) & _PAGE_PSE)); 
+
+    if ( pse1G )
+    {
+        /* Special case: this guest VA is in a PSE superpage, so there's
+         * no guest l1e.  We make one up so that the propagation code
+         * can generate a shadow l1 table.  Start with the gfn of the 
+         * first 4k-page of the superpage. */
+        gfn_t start = guest_l2e_get_gfn(gw->l2e);
+        /* Grant full access in the l1e, since all the guest entry's 
+         * access controls are enforced in the shadow l2e. */
+        int flags = (_PAGE_PRESENT|_PAGE_USER|_PAGE_RW|
+                     _PAGE_ACCESSED|_PAGE_DIRTY);
+        /* Import cache-control bits. Note that _PAGE_PAT is actually
+         * _PAGE_PSE, and it is always set. We will clear it in case
+         * _PAGE_PSE_PAT (bit 12, i.e. first bit of gfn) is clear. */
+        flags |= (guest_l2e_get_flags(gw->l2e)
+                  & (_PAGE_PAT|_PAGE_PWT|_PAGE_PCD));
+        if ( !(gfn_x(start) & 1) )
+            /* _PAGE_PSE_PAT not set: remove _PAGE_PAT from flags. */
+            flags &= ~_PAGE_PAT;
+
+#define GUEST_L3_GFN_ALIGN (1 << (GUEST_L3_PAGETABLE_SHIFT - \
+                                  GUEST_L1_PAGETABLE_SHIFT))
+        if ( gfn_x(start) & (GUEST_L3_GFN_ALIGN - 1) & ~0x1 )
+        {
+            rc |= _PAGE_INVALID_BITS;
+        }
+
+        /* Increment the pfn by the right number of 4k pages.  
+         * Mask out PAT and invalid bits. */
+        start = _gfn((gfn_x(start) & ~(GUEST_L3_GFN_ALIGN - 1)) +
+                     guest_l1_table_offset(va));
+        gw->l1e = guest_l1e_from_gfn(start, flags);
+        gw->l2mfn = gw->l1mfn = _mfn(INVALID_MFN);
+        goto set_ad;
+    }
+#endif
+
     gflags = guest_l2e_get_flags(gw->l2e) ^ iflags;
     rc |= ((gflags & mflags) ^ mflags);
     if ( rc & _PAGE_PRESENT )
         goto out;
 
-    pse = (guest_supports_superpages(v) && 
+    pse2M = (guest_supports_superpages(v) && 
            (guest_l2e_get_flags(gw->l2e) & _PAGE_PSE)); 
 
-    if ( pse )
+    if ( pse2M )
     {
         /* Special case: this guest VA is in a PSE superpage, so there's
          * no guest l1e.  We make one up so that the propagation code

@@ -290,22 +332,29 @@ guest_walk_tables(struct vcpu *v, struct
      * success.  Although the PRMs say higher-level _PAGE_ACCESSED bits
      * get set whenever a lower-level PT is used, at least some hardware
      * walkers behave this way. */
+#if GUEST_PAGING_LEVELS >= 4 /* 64-bit only... */
+set_ad:
+#endif
     if ( rc == 0 ) 
     {
 #if GUEST_PAGING_LEVELS == 4 /* 64-bit only... */
         if ( set_ad_bits(l4p + guest_l4_table_offset(va), &gw->l4e, 0) )
             paging_mark_dirty(d, mfn_x(gw->l4mfn));
-        if ( set_ad_bits(l3p + guest_l3_table_offset(va), &gw->l3e, 0) )
+        if ( set_ad_bits(l3p + guest_l3_table_offset(va), &gw->l3e,
+                         (pse1G && (pfec & PFEC_write_access))) )
             paging_mark_dirty(d, mfn_x(gw->l3mfn));
 #endif
-        if ( set_ad_bits(l2p + guest_l2_table_offset(va), &gw->l2e,
-                         (pse && (pfec & PFEC_write_access))) )
-            paging_mark_dirty(d, mfn_x(gw->l2mfn));            
-        if ( !pse ) 
+        if ( !pse1G ) 
         {
-            if ( set_ad_bits(l1p + guest_l1_table_offset(va), &gw->l1e, 
-                             (pfec & PFEC_write_access)) )
-                paging_mark_dirty(d, mfn_x(gw->l1mfn));
+            if ( set_ad_bits(l2p + guest_l2_table_offset(va), &gw->l2e,
+                             (pse2M && (pfec & PFEC_write_access))) )
+                paging_mark_dirty(d, mfn_x(gw->l2mfn));            
+            if ( !pse2M ) 
+            {
+                if ( set_ad_bits(l1p + guest_l1_table_offset(va), &gw->l1e, 
+                                 (pfec & PFEC_write_access)) )
+                    paging_mark_dirty(d, mfn_x(gw->l1mfn));
+            }
         }
     }
xen/include/asm-x86/guest_pt.h +11 -0
diff -r 4f2c59fb28e6 xen/include/asm-x86/guest_pt.h
@@ -194,6 +194,17 @@ guest_supports_superpages(struct vcpu *v
 }
 
 static inline int
+guest_supports_1G_superpages(struct vcpu *v)
+{
+    if (!guest_supports_superpages(v))
+        return 0;
+
+    return (GUEST_PAGING_LEVELS >= 4
+           && cpu_has_page1gb
+           && hvm_long_mode_enabled(v));
+}
+
+static inline int
 guest_supports_nx(struct vcpu *v)
 {
     if ( GUEST_PAGING_LEVELS == 2 || !cpu_has_nx )

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

#5 lore raw

Hi,

Thanks for this. Do you have a matching patch that enables 1GB mappings in the appropriate guests?

xen/arch/x86/mm/guest_walk.c guest_walk_tables(struct vcpu *v, struct · line 227

Quoted 10 lines · Christoph Egger
xen/arch/x86/mm/guest_walk.c +10 -0
+#if GUEST_PAGING_LEVELS >= 4 /* 64-bit only... */
+    pse1G = (guest_supports_1G_superpages(v) && 
+           (guest_l3e_get_flags(gw->l3e) & _PAGE_PSE)); 
+
+    if ( pse1G )
+    {
+        /* Special case: this guest VA is in a PSE superpage, so there's
+         * no guest l1e.  We make one up so that the propagation code
+         * can generate a shadow l1 table.  Start with the gfn of the 
+         * first 4k-page of the superpage. */

Shadow pagetables don't support 1GB mappings, so this comment needs to be updated to say why you're making the fake l1e (basically as a vehicle for the GFN of the target frame).

xen/arch/x86/mm/guest_walk.c guest_walk_tables(struct vcpu *v, struct · line 237

Quoted 10 lines
xen/arch/x86/mm/guest_walk.c +10 -0
+        gfn_t start = guest_l2e_get_gfn(gw->l2e);
+        /* Grant full access in the l1e, since all the guest entry's 
+         * access controls are enforced in the shadow l2e. */
+        int flags = (_PAGE_PRESENT|_PAGE_USER|_PAGE_RW|
+                     _PAGE_ACCESSED|_PAGE_DIRTY);
+        /* Import cache-control bits. Note that _PAGE_PAT is actually
+         * _PAGE_PSE, and it is always set. We will clear it in case
+         * _PAGE_PSE_PAT (bit 12, i.e. first bit of gfn) is clear. */
+        flags |= (guest_l2e_get_flags(gw->l2e)
+                  & (_PAGE_PAT|_PAGE_PWT|_PAGE_PCD));

You should be taking these from the l3e, not the l2e. Also, you should probably do it before the code that maps the l2 table. :)

Quoted 5 lines

diff -r 4f2c59fb28e6 xen/include/asm-x86/guest_pt.h

xen/include/asm-x86/guest_pt.h +0 -0
@@ -194,6 +194,17 @@ guest_supports_superpages(struct vcpu *v
 }

xen/include/asm-x86/guest_pt.h guest_supports_superpages(struct vcpu *v · line 204

Quoted 10 lines
xen/include/asm-x86/guest_pt.h +8 -0
 
 static inline int
+guest_supports_1G_superpages(struct vcpu *v)
+{
+    if (!guest_supports_superpages(v))
+        return 0;
+
+    return (GUEST_PAGING_LEVELS >= 4
+           && cpu_has_page1gb
+           && hvm_long_mode_enabled(v));

There needs to be some interlock to make sure this returns false for all guests except HVM HAP ones.

Also this function should be called from the CPUID trap handler to make sure we never advertise PSE1GB when we're not going to support it. (I think that involves moving it out of this header file -- it should probably go into hvm.h and be renamed to match the hvm_*_enabled macros already there.)

Cheers,

Tim.

Signature · 3 lines
Tim Deegan <[email protected]>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)
#6 lore raw
Quoted 2 lines · Tim Deegan

Also this function should be called from the CPUID trap handler to make sure we never advertise PSE1GB when we're not going to support it.

Er, not this function exactly, since CPUID should report the feature even when the guest's not in long_mode. I think it needs a hvm_pse1G_supported that can be called from CPUID, and then guest_supports_1G_superpages() boils down to "(GUEST_PAGING_LEVEL >= 4) && hvm_pse1G_supported(v)"

Tim.

Signature · 3 lines
Tim Deegan <[email protected]>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)
author
#7 lore raw
Quoted 9 lines · Tim Deegan
Quoted 2 lines · Tim Deegan

Also this function should be called from the CPUID trap handler to make sure we never advertise PSE1GB when we're not going to support it.

Er, not this function exactly, since CPUID should report the feature even when the guest's not in long_mode. I think it needs a hvm_pse1G_supported that can be called from CPUID, and then guest_supports_1G_superpages() boils down to "(GUEST_PAGING_LEVEL>= 4)&& hvm_pse1G_supported(v)"

New version attached. I removed the fake l1e calculation. Something is still wrong: The l2 guest kernel crashes with an unexpected null pointer derefencing but it boots w/o this patch.

Is it a problem when l1 guest uses 1gb pages where the host uses 4k or 2m pages?

Christoph

Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

# HG changeset patch
# User cegger
# Date 1311681725 -7200
support 1gb pages for guests

Signed-off-by Christoph Egger <[email protected]>
xen/arch/x86/hvm/hvm.c +8 -3
diff -r 4f2c59fb28e6 -r 6d15152fb59a xen/arch/x86/hvm/hvm.c
@@ -2389,6 +2389,7 @@ void hvm_cpuid(unsigned int input, unsig
                                    unsigned int *ecx, unsigned int *edx)
 {
     struct vcpu *v = current;
+    struct domain *d = v->domain;
     unsigned int count = *ecx;
 
     if ( cpuid_viridian_leaves(input, eax, ebx, ecx, edx) )

@@ -2397,7 +2398,7 @@ void hvm_cpuid(unsigned int input, unsig
     if ( cpuid_hypervisor_leaves(input, count, eax, ebx, ecx, edx) )
         return;
 
-    domain_cpuid(v->domain, input, *ecx, eax, ebx, ecx, edx);
+    domain_cpuid(d, input, *ecx, eax, ebx, ecx, edx);
 
     switch ( input )
     {

@@ -2433,7 +2434,7 @@ void hvm_cpuid(unsigned int input, unsig
             {
                 if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) )
                     continue;
-                domain_cpuid(v->domain, input, sub_leaf, &_eax, &_ebx, &_ecx, 
+                domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, 
                              &_edx);
                 if ( (_eax + _ebx) > *ebx )
                     *ebx = _eax + _ebx;

@@ -2444,9 +2445,13 @@ void hvm_cpuid(unsigned int input, unsig
     case 0x80000001:
         /* We expose RDTSCP feature to guest only when
            tsc_mode == TSC_MODE_DEFAULT and host_tsc_is_safe() returns 1 */
-        if ( v->domain->arch.tsc_mode != TSC_MODE_DEFAULT ||
+        if ( d->arch.tsc_mode != TSC_MODE_DEFAULT ||
              !host_tsc_is_safe() )
             *edx &= ~cpufeat_mask(X86_FEATURE_RDTSCP);
+        /* Expose 1gb page feature for HVM HAP guests and hw support is
+         * available. */
+        if (hvm_pse1gb_supported(d))
+            *edx |= cpufeat_mask(X86_FEATURE_PAGE1GB);
         break;
     }
 }
xen/arch/x86/mm/guest_walk.c +40 -11

86 lines not on this page · raw message

xen/include/asm-x86/guest_pt.h +11 -0
diff -r 4f2c59fb28e6 -r 6d15152fb59a xen/include/asm-x86/guest_pt.h
@@ -194,6 +194,17 @@ guest_supports_superpages(struct vcpu *v
 }
 
 static inline int
+guest_supports_1G_superpages(struct vcpu *v)
+{
+    if (!guest_supports_superpages(v))
+        return 0;
+
+    return (GUEST_PAGING_LEVELS >= 4
+           && hvm_pse1gb_supported(v->domain)
+           && hvm_long_mode_enabled(v));
+}
+
+static inline int
 guest_supports_nx(struct vcpu *v)
 {
     if ( GUEST_PAGING_LEVELS == 2 || !cpu_has_nx )
xen/include/asm-x86/hvm/hvm.h +3 -0
diff -r 4f2c59fb28e6 -r 6d15152fb59a xen/include/asm-x86/hvm/hvm.h
@@ -224,6 +224,9 @@ int hvm_girq_dest_2_vcpu_id(struct domai
 #define hvm_hap_has_2mb(d) \
     (hvm_funcs.hap_capabilities & HVM_HAP_SUPERPAGE_2MB)
 
+#define hvm_pse1gb_supported(d) \
+    (cpu_has_page1gb && hvm_hap_has_1gb(d) && paging_mode_hap(d))
+
 #ifdef __x86_64__
 #define hvm_long_mode_enabled(v) \
     ((v)->arch.hvm_vcpu.guest_efer & EFER_LMA)

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

#8 lore raw
Quoted 5 lines · Christoph Egger
Quoted 4 lines · Tim Deegan
Quoted 2 lines · Tim Deegan

Also this function should be called from the CPUID trap handler to make sure we never advertise PSE1GB when we're not going to support it.

Quoted 7 lines
Quoted 5 lines

Er, not this function exactly, since CPUID should report the feature even when the guest's not in long_mode. I think it needs a hvm_pse1G_supported that can be called from CPUID, and then guest_supports_1G_superpages() boils down to "(GUEST_PAGING_LEVEL>= 4)&& hvm_pse1G_supported(v)"

New version attached. I removed the fake l1e calculation.

If you just remove it, you need to update hap_p2m_ga_to_gfn to figure out the GFN some other way! I expect that's what's causing your problem. You should either provide the fake l1e, and say why in the comment, or audit all callers of the function to make sure they don't need it.

xen/arch/x86/hvm/hvm.c void hvm_cpuid(unsigned int input, unsig · line 2454

Quoted 12 lines
xen/arch/x86/hvm/hvm.c +5 -1
@@ -2444,9 +2445,13 @@ void hvm_cpuid(unsigned int input, unsig
     case 0x80000001:
         /* We expose RDTSCP feature to guest only when
            tsc_mode == TSC_MODE_DEFAULT and host_tsc_is_safe() returns 1 */
-        if ( v->domain->arch.tsc_mode != TSC_MODE_DEFAULT ||
+        if ( d->arch.tsc_mode != TSC_MODE_DEFAULT ||
              !host_tsc_is_safe() )
             *edx &= ~cpufeat_mask(X86_FEATURE_RDTSCP);
+        /* Expose 1gb page feature for HVM HAP guests and hw support is
+         * available. */
+        if (hvm_pse1gb_supported(d))
+            *edx |= cpufeat_mask(X86_FEATURE_PAGE1GB);

I asked for the opposite of this: the flag must be _cleared_ if the feature's _not_ supported. It's really up to the userspace tools to decide whether they want the flag set in the first place.

Quoted 5 lines

diff -r 4f2c59fb28e6 -r 6d15152fb59a xen/include/asm-x86/guest_pt.h

xen/include/asm-x86/guest_pt.h +0 -0
@@ -194,6 +194,17 @@ guest_supports_superpages(struct vcpu *v
 }

xen/include/asm-x86/guest_pt.h guest_supports_superpages(struct vcpu *v · line 204

Quoted 10 lines
xen/include/asm-x86/guest_pt.h +8 -0
 
 static inline int
+guest_supports_1G_superpages(struct vcpu *v)
+{
+    if (!guest_supports_superpages(v))
+        return 0;
+
+    return (GUEST_PAGING_LEVELS >= 4
+           && hvm_pse1gb_supported(v->domain)
+           && hvm_long_mode_enabled(v));

You don't need to check GUEST_PAGING_LEVELS and hvm_long_mode_enabled(); this file is compiled multiple times so they're always the same on any given code path. Check only GUEST_PAGING_LEVELS.

Cheers,

Tim.

Signature · 3 lines
Tim Deegan <[email protected]>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)
author
#9 lore raw
Quoted 12 lines · Tim Deegan
Quoted 11 lines · Christoph Egger
Quoted 9 lines · Tim Deegan
Quoted 2 lines · Tim Deegan

Also this function should be called from the CPUID trap handler to make sure we never advertise PSE1GB when we're not going to support it.

Er, not this function exactly, since CPUID should report the feature even when the guest's not in long_mode. I think it needs a hvm_pse1G_supported that can be called from CPUID, and then guest_supports_1G_superpages() boils down to
"(GUEST_PAGING_LEVEL>= 4)&& hvm_pse1G_supported(v)"

Quoted 7 lines
Quoted 1 line

New version attached. I removed the fake l1e calculation.

If you just remove it, you need to update hap_p2m_ga_to_gfn to figure out the GFN some other way! I expect that's what's causing your problem. You should either provide the fake l1e, and say why in the comment, or audit all callers of the function to make sure they don't need it.

Ah, I see now. I misunderstood you.

I forgot to increase the pfn by the right number of 2m pages for the l1e calculation.

Now it works for me: The l2 guest boots and passes my tests.

Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

# HG changeset patch
# User cegger
# Date 1311772079 -7200
support 1gb pages for guests

Signed-off-by Christoph Egger <[email protected]>
xen/arch/x86/hvm/hvm.c +8 -3
diff -r 4f2c59fb28e6 -r ea91116ba394 xen/arch/x86/hvm/hvm.c
@@ -2389,6 +2389,7 @@ void hvm_cpuid(unsigned int input, unsig
                                    unsigned int *ecx, unsigned int *edx)
 {
     struct vcpu *v = current;
+    struct domain *d = v->domain;
     unsigned int count = *ecx;
 
     if ( cpuid_viridian_leaves(input, eax, ebx, ecx, edx) )

@@ -2397,7 +2398,7 @@ void hvm_cpuid(unsigned int input, unsig
     if ( cpuid_hypervisor_leaves(input, count, eax, ebx, ecx, edx) )
         return;
 
-    domain_cpuid(v->domain, input, *ecx, eax, ebx, ecx, edx);
+    domain_cpuid(d, input, *ecx, eax, ebx, ecx, edx);
 
     switch ( input )
     {

@@ -2433,7 +2434,7 @@ void hvm_cpuid(unsigned int input, unsig
             {
                 if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) )
                     continue;
-                domain_cpuid(v->domain, input, sub_leaf, &_eax, &_ebx, &_ecx, 
+                domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, 
                              &_edx);
                 if ( (_eax + _ebx) > *ebx )
                     *ebx = _eax + _ebx;

@@ -2444,9 +2445,13 @@ void hvm_cpuid(unsigned int input, unsig
     case 0x80000001:
         /* We expose RDTSCP feature to guest only when
            tsc_mode == TSC_MODE_DEFAULT and host_tsc_is_safe() returns 1 */
-        if ( v->domain->arch.tsc_mode != TSC_MODE_DEFAULT ||
+        if ( d->arch.tsc_mode != TSC_MODE_DEFAULT ||
              !host_tsc_is_safe() )
             *edx &= ~cpufeat_mask(X86_FEATURE_RDTSCP);
+        /* Expose 1gb page feature for HVM HAP guests and hw support is
+         * available. */
+        if (!hvm_pse1gb_supported(d))
+            *edx &= ~cpufeat_mask(X86_FEATURE_PAGE1GB);
         break;
     }
 }
xen/arch/x86/mm/guest_walk.c +65 -13

125 lines not on this page · raw message

xen/include/asm-x86/guest_pt.h +6 -0
diff -r 4f2c59fb28e6 -r ea91116ba394 xen/include/asm-x86/guest_pt.h
@@ -194,6 +194,12 @@ guest_supports_superpages(struct vcpu *v
 }
 
 static inline int
+guest_supports_1G_superpages(struct vcpu *v)
+{
+    return (GUEST_PAGING_LEVELS >= 4 && guest_supports_superpages(v));
+}
+
+static inline int
 guest_supports_nx(struct vcpu *v)
 {
     if ( GUEST_PAGING_LEVELS == 2 || !cpu_has_nx )
xen/include/asm-x86/hvm/hvm.h +3 -0
diff -r 4f2c59fb28e6 -r ea91116ba394 xen/include/asm-x86/hvm/hvm.h
@@ -224,6 +224,9 @@ int hvm_girq_dest_2_vcpu_id(struct domai
 #define hvm_hap_has_2mb(d) \
     (hvm_funcs.hap_capabilities & HVM_HAP_SUPERPAGE_2MB)
 
+#define hvm_pse1gb_supported(d) \
+    (cpu_has_page1gb && hvm_hap_has_1gb(d) && paging_mode_hap(d))
+
 #ifdef __x86_64__
 #define hvm_long_mode_enabled(v) \
     ((v)->arch.hvm_vcpu.guest_efer & EFER_LMA)

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

#10 lore raw
Quoted 6 lines · Christoph Egger

# HG changeset patch
# User cegger
# Date 1311772079 -7200
support 1gb pages for guests

Signed-off-by Christoph Egger <[email protected]>

Thanks. Applied, with some changes, as 23755:ba78ea7784c9

Tim.

Signature · 3 lines
Tim Deegan <[email protected]>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)