ept code fix-ups

PATCH New Patrick Colp 2 participants 7 messages Aug 5, 2009 – Aug 6, 2009

Share & tools

Patrick Colp

author

2009-08-05 14:00 UTC #0 lore raw

PATCH 1/1 ept code fix-ups

The ept code by default sets rwx to 1 instead of 0. There are also some places where unsigned long is used instead of mfn_t. Finally, the code formatting is inconsistent both with itself and the rest of Xen.

This series of patches fixes up the ept code to address these issues.

Patrick

Patrick Colp

author

2009-08-05 14:00 UTC #1 lore raw

PATCH 1/1 ept p2m: set rwx flags to 0 for invalid and mmio_dm types

# HG changeset patch
# User Patrick Colp <[email protected]>
# Date 1249480778 -3600
# Node ID 86486ebbb5f0bd4f94b42492b7e00110971a4078 # Parent 68e8b8379244e293c55875e7dc3692fc81d3d212 ept p2m: set rwx flags to 0 for invalid and mmio_dm types.

Read/write/execute flags are set to 1 before calling the type_to_flags function which sets them to their appropriate values depending on the p2m type. However, in invalid, mmio_dm, and default/unknown cases in type_to_flags just falls through, unsafely leaving full access to these pages.

Signed-off-by Patrick Colp <[email protected]>

xen/arch/x86/mm/hap/p2m-ept.c +1 -2

diff -r 68e8b8379244 -r 86486ebbb5f0 xen/arch/x86/mm/hap/p2m-ept.c
@@ -36,6 +36,7 @@
         case p2m_invalid:
         case p2m_mmio_dm:
         default:
+             entry->r = entry->w = entry->x = 0;
             return;
         case p2m_ram_rw:
         case p2m_mmio_direct:

@@ -196,7 +197,6 @@
             ept_entry->rsvd = 0;
             ept_entry->avail2 = 0;
             /* last step */
-            ept_entry->r = ept_entry->w = ept_entry->x = 1;
             ept_p2m_type_to_flags(ept_entry, p2mt);
         }
         else

@@ -242,7 +242,6 @@
             split_ept_entry->rsvd = 0;
             split_ept_entry->avail2 = 0;
             /* last step */
-            split_ept_entry->r = split_ept_entry->w = split_ept_entry->x = 1;
             ept_p2m_type_to_flags(split_ept_entry, split_p2mt);
         }

Patrick Colp

author

2009-08-05 14:00 UTC #2 lore raw

PATCH 1/1 ept p2m: replace unsigned long with mfn_t for mfns

In reply to Patrick Colp

# HG changeset patch
# User Patrick Colp <[email protected]> # Date 1249480778 -3600
# Node ID a77221fe874c17b6a4ac3dc0ca40ab91949d8afa # Parent 86486ebbb5f0bd4f94b42492b7e00110971a4078 ept p2m: replace unsigned long with mfn_t for mfns.

Signed-off-by Patrick Colp <[email protected]>

xen/arch/x86/mm/hap/p2m-ept.c +8 -8

diff -r 86486ebbb5f0 -r a77221fe874c xen/arch/x86/mm/hap/p2m-ept.c
@@ -402,11 +402,11 @@
  * return 1 to not to reset ept entry.
  */
 static int need_modify_ept_entry(struct domain *d, unsigned long gfn,
-                                    unsigned long mfn, uint8_t o_igmt,
+                                    mfn_t mfn, uint8_t o_igmt,
                                     uint8_t o_emt, p2m_type_t p2mt)
 {
     uint8_t igmt, emt;
-    emt = epte_get_entry_emt(d, gfn, mfn, &igmt, 
+    emt = epte_get_entry_emt(d, gfn, mfn_x(mfn), &igmt, 
                                 (p2mt == p2m_mmio_direct));
     if ( (emt == o_emt) && (igmt == o_igmt) )
         return 0;

@@ -420,7 +420,7 @@
     p2m_type_t p2mt;
     uint64_t epte;
     int order = 0;
-    unsigned long mfn;
+    mfn_t mfn;
     uint8_t o_igmt, o_emt;
 
     for ( gfn = start_gfn; gfn <= end_gfn; gfn++ )

@@ -428,8 +428,8 @@
         epte = ept_get_entry_content(d, gfn);
         if ( epte == 0 )
             continue;
-        mfn = (epte & EPTE_MFN_MASK) >> PAGE_SHIFT;
-        if ( !mfn_valid(mfn) )
+        mfn = _mfn((epte & EPTE_MFN_MASK) >> PAGE_SHIFT);
+        if ( !mfn_valid(mfn_x(mfn)) )
             continue;
         p2mt = (epte & EPTE_AVAIL1_MASK) >> EPTE_AVAIL1_SHIFT;
         o_igmt = (epte & EPTE_IGMT_MASK) >> EPTE_IGMT_SHIFT;

@@ -447,7 +447,7 @@
                 order = EPT_TABLE_ORDER;
                 if ( need_modify_ept_entry(d, gfn, mfn, 
                                             o_igmt, o_emt, p2mt) )
-                    ept_set_entry(d, gfn, _mfn(mfn), order, p2mt);
+                    ept_set_entry(d, gfn, mfn, order, p2mt);
                 gfn += 0x1FF;
             }
             else

@@ -455,7 +455,7 @@
                 /* change emt for partial entries of the 2m area. */
                 if ( need_modify_ept_entry(d, gfn, mfn, 
                                             o_igmt, o_emt, p2mt) )
-                    ept_set_entry(d, gfn, _mfn(mfn), order, p2mt);
+                    ept_set_entry(d, gfn, mfn, order, p2mt);
                 gfn = ((gfn >> EPT_TABLE_ORDER) << EPT_TABLE_ORDER) + 0x1FF;
             }
         }

@@ -463,7 +463,7 @@
         {
             if ( need_modify_ept_entry(d, gfn, mfn, 
                                             o_igmt, o_emt, p2mt) )
-                ept_set_entry(d, gfn, _mfn(mfn), order, p2mt);
+                ept_set_entry(d, gfn, mfn, order, p2mt);
         }
     }
 }

Patrick Colp

author

2009-08-05 14:00 UTC #3 lore raw

PATCH 1/1 ept mtrr: replace unsigned long with mfn_t for mfns

In reply to Patrick Colp

# HG changeset patch
# User Patrick Colp <[email protected]> # Date 1249480778 -3600
# Node ID 9b2a92187e3cfac7753da4e4b1944e774cb01e6f # Parent a77221fe874c17b6a4ac3dc0ca40ab91949d8afa ept mtrr: replace unsigned long with mfn_t for mfns.

Signed-off-by Patrick Colp <[email protected]>

xen/arch/x86/hvm/mtrr.c +4 -3

diff -r a77221fe874c -r 9b2a92187e3c xen/arch/x86/hvm/mtrr.c
@@ -20,6 +20,7 @@
 #include <public/hvm/e820.h>
 #include <xen/types.h>
 #include <asm/e820.h>
+#include <asm/mm.h>
 #include <asm/paging.h>
 #include <asm/p2m.h>
 #include <xen/domain_page.h>

@@ -712,7 +713,7 @@
 
 uint8_t epte_get_entry_emt(
     struct domain *d, unsigned long gfn, 
-    unsigned long mfn, uint8_t *igmt, int direct_mmio)
+    mfn_t mfn, uint8_t *igmt, int direct_mmio)
 {
     uint8_t gmtrr_mtype, hmtrr_mtype;
     uint32_t type;

@@ -730,7 +731,7 @@
     if ( (v == current) && v->domain->arch.hvm_domain.is_in_uc_mode )
         return MTRR_TYPE_UNCACHABLE;
 
-    if ( !mfn_valid(mfn) )
+    if ( !mfn_valid(mfn_x(mfn)) )
         return MTRR_TYPE_UNCACHABLE;
 
     if ( hvm_get_mem_pinned_cacheattr(d, gfn, &type) )

@@ -752,6 +753,6 @@
     }
 
     gmtrr_mtype = get_mtrr_type(&v->arch.hvm_vcpu.mtrr, (gfn << PAGE_SHIFT));
-    hmtrr_mtype = get_mtrr_type(&mtrr_state, (mfn << PAGE_SHIFT));
+    hmtrr_mtype = get_mtrr_type(&mtrr_state, (mfn_x(mfn) << PAGE_SHIFT));
     return ((gmtrr_mtype <= hmtrr_mtype) ? gmtrr_mtype : hmtrr_mtype);
 }

xen/arch/x86/mm/hap/p2m-ept.c +4 -4

diff -r a77221fe874c -r 9b2a92187e3c xen/arch/x86/mm/hap/p2m-ept.c
@@ -168,7 +168,7 @@
     {
         if ( mfn_valid(mfn_x(mfn)) || (p2mt == p2m_mmio_direct) )
         {
-            ept_entry->emt = epte_get_entry_emt(d, gfn, mfn_x(mfn),
+            ept_entry->emt = epte_get_entry_emt(d, gfn, mfn,
                                 &igmt, direct_mmio);
             ept_entry->igmt = igmt;
             ept_entry->sp_avail = walk_level ? 1 : 0;

@@ -230,7 +230,7 @@
         {
             split_ept_entry = split_table + i;
             split_ept_entry->emt = epte_get_entry_emt(d,
-                                        gfn-offset+i, split_mfn+i, 
+                                        gfn-offset+i, _mfn(split_mfn+i), 
                                         &igmt, direct_mmio);
             split_ept_entry->igmt = igmt;

@@ -247,7 +247,7 @@
 
         /* Set the destinated 4k page as normal */
         split_ept_entry = split_table + offset;
-        split_ept_entry->emt = epte_get_entry_emt(d, gfn, mfn_x(mfn), 
+        split_ept_entry->emt = epte_get_entry_emt(d, gfn, mfn,
                                                 &igmt, direct_mmio);
         split_ept_entry->igmt = igmt;

@@ -406,7 +406,7 @@
                                     uint8_t o_emt, p2m_type_t p2mt)
 {
     uint8_t igmt, emt;
-    emt = epte_get_entry_emt(d, gfn, mfn_x(mfn), &igmt, 
+    emt = epte_get_entry_emt(d, gfn, mfn, &igmt, 
                                 (p2mt == p2m_mmio_direct));
     if ( (emt == o_emt) && (igmt == o_igmt) )
         return 0;

xen/include/asm-x86/mtrr.h +2 -1

diff -r a77221fe874c -r 9b2a92187e3c xen/include/asm-x86/mtrr.h
@@ -2,6 +2,7 @@
 #define __ASM_X86_MTRR_H__
 
 #include <xen/config.h>
+#include <asm/mm.h>
 
 /* These are the region types. They match the architectural specification. */
 #define MTRR_TYPE_UNCACHABLE 0

@@ -66,7 +67,7 @@
 extern u32 get_pat_flags(struct vcpu *v, u32 gl1e_flags, paddr_t gpaddr,
                   paddr_t spaddr, uint8_t gmtrr_mtype);
 extern uint8_t epte_get_entry_emt(
-    struct domain *d, unsigned long gfn, unsigned long mfn,
+    struct domain *d, unsigned long gfn, mfn_t mfn,
     uint8_t *igmt, int direct_mmio);
 extern void ept_change_entry_emt_with_range(
     struct domain *d, unsigned long start_gfn, unsigned long end_gfn);

Patrick Colp

author

2009-08-05 14:00 UTC #4 lore raw

PATCH 1/1 ept: code clean up and formatting

In reply to Patrick Colp

# HG changeset patch
# User Patrick Colp <[email protected]> # Date 1249480778 -3600
# Node ID 91a30cffa1c4e0d52e36574159ef3a2c4f0bcfee # Parent 9b2a92187e3cfac7753da4e4b1944e774cb01e6f
ept: code clean up and formatting.

Fix alignment and comments and add and remove spaces and lines where appropriate.

Signed-off-by Patrick Colp <[email protected]>

xen/arch/x86/hvm/mtrr.c +2 -3

diff -r 9b2a92187e3c -r 91a30cffa1c4 xen/arch/x86/hvm/mtrr.c
@@ -711,9 +711,8 @@
 HVM_REGISTER_SAVE_RESTORE(MTRR, hvm_save_mtrr_msr, hvm_load_mtrr_msr,
                           1, HVMSR_PER_VCPU);
 
-uint8_t epte_get_entry_emt(
-    struct domain *d, unsigned long gfn, 
-    mfn_t mfn, uint8_t *igmt, int direct_mmio)
+uint8_t epte_get_entry_emt(struct domain *d, unsigned long gfn, mfn_t mfn,
+                           uint8_t *igmt, int direct_mmio)
 {
     uint8_t gmtrr_mtype, hmtrr_mtype;
     uint32_t type;

xen/arch/x86/mm/hap/p2m-ept.c +94 ~~-71~~

413 lines not on this page · raw message

xen/include/asm-x86/mtrr.h +2 -3

diff -r 9b2a92187e3c -r 91a30cffa1c4 xen/include/asm-x86/mtrr.h
@@ -66,9 +66,8 @@
 extern void mtrr_centaur_report_mcr(int mcr, u32 lo, u32 hi);
 extern u32 get_pat_flags(struct vcpu *v, u32 gl1e_flags, paddr_t gpaddr,
                   paddr_t spaddr, uint8_t gmtrr_mtype);
-extern uint8_t epte_get_entry_emt(
-    struct domain *d, unsigned long gfn, mfn_t mfn,
-    uint8_t *igmt, int direct_mmio);
+extern uint8_t epte_get_entry_emt(struct domain *d, unsigned long gfn,
+                                  mfn_t mfn, uint8_t *igmt, int direct_mmio);
 extern void ept_change_entry_emt_with_range(
     struct domain *d, unsigned long start_gfn, unsigned long end_gfn);
 extern unsigned char pat_type_2_pte_flags(unsigned char pat_type);

Keir Fraser

2009-08-06 08:18 UTC #5 lore raw

Re: [PATCH 4 of 4] ept: code clean up and formatting

Quoted 9 lines · Patrick Colp

# HG changeset patch
# User Patrick Colp <[email protected]> # Date 1249480778 -3600
# Node ID 91a30cffa1c4e0d52e36574159ef3a2c4f0bcfee # Parent 9b2a92187e3cfac7753da4e4b1944e774cb01e6f
ept: code clean up and formatting.
Fix alignment and comments and add and remove spaces and lines where appropriate.

This one didn't apply cleanly. Can you refresh against xen-unstable tip and maybe send as an attachment (in case it's a garbling problem)?

Also is patch 1/4 by itself safe for 3.4 and 3.3 branches?

Thanks,
Keir

Patrick Colp

author

2009-08-06 08:34 UTC #6 lore raw

Re: [PATCH 4 of 4] ept: code clean up and formatting

I just did a pull and there were no changes since I sent the patches. I'll attach it in case it got garbled.

It should be safe to apply 1/4 to 3.3 and 3.4 (I just checked and it applied fine with some offsetting in both cases).

Patrick

Quoted 12 lines · Keir Fraser

Quoted 9 lines · Patrick Colp
# HG changeset patch
# User Patrick Colp <[email protected]> # Date 1249480778 -3600
# Node ID 91a30cffa1c4e0d52e36574159ef3a2c4f0bcfee # Parent 9b2a92187e3cfac7753da4e4b1944e774cb01e6f
ept: code clean up and formatting.
Fix alignment and comments and add and remove spaces and lines where appropriate.

Quoted 9 lines

This one didn't apply cleanly. Can you refresh against xen-unstable tip and maybe send as an attachment (in case it's a garbling problem)?
Also is patch 1/4 by itself safe for 3.4 and 3.3 branches?
Thanks,
Keir

ept: code clean up and formatting.

Fix alignment and comments and add and remove spaces and lines where appropriate.

Signed-off-by Patrick Colp <[email protected]>

xen/arch/x86/hvm/mtrr.c +2 -3

diff -r 889df868a490 xen/arch/x86/hvm/mtrr.c
@@ -711,9 +711,8 @@
 HVM_REGISTER_SAVE_RESTORE(MTRR, hvm_save_mtrr_msr, hvm_load_mtrr_msr,
                           1, HVMSR_PER_VCPU);
 
-uint8_t epte_get_entry_emt(
-    struct domain *d, unsigned long gfn, 
-    mfn_t mfn, uint8_t *igmt, int direct_mmio)
+uint8_t epte_get_entry_emt(struct domain *d, unsigned long gfn, mfn_t mfn,
+                           uint8_t *igmt, int direct_mmio)
 {
     uint8_t gmtrr_mtype, hmtrr_mtype;
     uint32_t type;

xen/arch/x86/mm/hap/p2m-ept.c +94 ~~-71~~

413 lines not on this page · raw message

xen/include/asm-x86/mtrr.h +2 -3

diff -r 889df868a490 xen/include/asm-x86/mtrr.h
@@ -66,9 +66,8 @@
 extern void mtrr_centaur_report_mcr(int mcr, u32 lo, u32 hi);
 extern u32 get_pat_flags(struct vcpu *v, u32 gl1e_flags, paddr_t gpaddr,
                   paddr_t spaddr, uint8_t gmtrr_mtype);
-extern uint8_t epte_get_entry_emt(
-    struct domain *d, unsigned long gfn, mfn_t mfn,
-    uint8_t *igmt, int direct_mmio);
+extern uint8_t epte_get_entry_emt(struct domain *d, unsigned long gfn,
+                                  mfn_t mfn, uint8_t *igmt, int direct_mmio);
 extern void ept_change_entry_emt_with_range(
     struct domain *d, unsigned long start_gfn, unsigned long end_gfn);
 extern unsigned char pat_type_2_pte_flags(unsigned char pat_type);

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