thr3ads.net - Xen devel - [Xen-devel] [PATCH] x86: make show_page

If this information is useful, please help other people find it:
Share via:

Jan Beulich

2008-Jan-24 15:03 UTC

[Xen-devel] [PATCH] x86: make show_page_walk() more robust

While adding 1Gb page support for the 1:1 mapping I noticed that
show_page_walk() crashes when used before the M2P table gets created,
and from looking at the code I realized that it would also crash if a
corrupt page table with an out of range MFN would be encountered.
While it would have been possible to make get_gpfn_from_mfn() more
robust, it seemed like keeping the overhead there low (as in the
general case proper values can be expected and would likely have been
checked for already), and hence I made the checks privates to
show_page_walk().

Signed-off-by: Jan Beulich <jbeulich@novell.com>

Index: 2008-01-18/xen/arch/x86/x86_32/mm.c
==================================================================---
2008-01-18.orig/xen/arch/x86/x86_32/mm.c	2008-01-23 11:54:09.000000000 +0100
+++ 2008-01-18/xen/arch/x86/x86_32/mm.c	2008-01-23 11:48:16.000000000 +0100
@@ -41,6 +41,7 @@ l2_pgentry_t __attribute__ ((__section__
 unsigned int PAGE_HYPERVISOR         = __PAGE_HYPERVISOR;
 unsigned int PAGE_HYPERVISOR_NOCACHE = __PAGE_HYPERVISOR_NOCACHE;
 
+int mpt_valid;
 static unsigned long mpt_size;
 
 void *alloc_xen_pagetable(void)
@@ -110,6 +111,8 @@ void __init paging_init(void)
                       pg, (__PAGE_HYPERVISOR | _PAGE_PSE) & ~_PAGE_RW));
     }
 
+    mpt_valid = 1;
+
     /* Fill with an obvious debug pattern. */
     for ( i = 0; i < (mpt_size / BYTES_PER_LONG); i++)
         set_gpfn_from_mfn(i, 0x55555555);
Index: 2008-01-18/xen/arch/x86/x86_32/traps.c
==================================================================---
2008-01-18.orig/xen/arch/x86/x86_32/traps.c	2008-01-23 11:54:09.000000000 +0100
+++ 2008-01-18/xen/arch/x86/x86_32/traps.c	2008-01-23 11:53:58.000000000 +0100
@@ -132,7 +132,8 @@ void show_page_walk(unsigned long addr)
     l3t += (cr3 & 0xFE0UL) >> 3;
     l3e = l3t[l3_table_offset(addr)];
     mfn = l3e_get_pfn(l3e);
-    pfn = get_gpfn_from_mfn(mfn);
+    pfn = mfn_valid(mfn) && mpt_valid ?
+          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
     printk(" L3[0x%03lx] = %"PRIpte" %08lx\n",
            l3_table_offset(addr), l3e_get_intpte(l3e), pfn);
     unmap_domain_page(l3t);
@@ -143,7 +144,8 @@ void show_page_walk(unsigned long addr)
     l2t = map_domain_page(mfn);
     l2e = l2t[l2_table_offset(addr)];
     mfn = l2e_get_pfn(l2e);
-    pfn = get_gpfn_from_mfn(mfn);
+    pfn = mfn_valid(mfn) && mpt_valid ?
+          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
     printk(" L2[0x%03lx] = %"PRIpte" %08lx %s\n",
            l2_table_offset(addr), l2e_get_intpte(l2e), pfn,
            (l2e_get_flags(l2e) & _PAGE_PSE) ? "(PSE)" :
"");
@@ -155,7 +157,8 @@ void show_page_walk(unsigned long addr)
     l1t = map_domain_page(mfn);
     l1e = l1t[l1_table_offset(addr)];
     mfn = l1e_get_pfn(l1e);
-    pfn = get_gpfn_from_mfn(mfn);
+    pfn = mfn_valid(mfn) && mpt_valid ?
+          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
     printk(" L1[0x%03lx] = %"PRIpte" %08lx\n",
            l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
     unmap_domain_page(l1t);
Index: 2008-01-18/xen/arch/x86/x86_64/mm.c
==================================================================---
2008-01-18.orig/xen/arch/x86/x86_64/mm.c	2008-01-23 11:54:09.000000000 +0100
+++ 2008-01-18/xen/arch/x86/x86_64/mm.c	2008-01-23 11:50:56.000000000 +0100
@@ -32,6 +32,7 @@
 #include <asm/msr.h>
 #include <public/memory.h>
 
+int mpt_valid;
 #ifdef CONFIG_COMPAT
 unsigned int m2p_compat_vstart = __HYPERVISOR_COMPAT_VIRT_START;
 #endif
@@ -144,6 +145,8 @@ void __init paging_init(void)
         l2_ro_mpt++;
     }
 
+    mpt_valid = 1;
+
     /* Create user-accessible L2 directory to map the MPT for compat guests. */
     BUILD_BUG_ON(l4_table_offset(RDWR_MPT_VIRT_START) !                 
l4_table_offset(HIRO_COMPAT_MPT_VIRT_START));
Index: 2008-01-18/xen/arch/x86/x86_64/traps.c
==================================================================---
2008-01-18.orig/xen/arch/x86/x86_64/traps.c	2008-01-23 11:54:09.000000000 +0100
+++ 2008-01-18/xen/arch/x86/x86_64/traps.c	2008-01-23 11:53:27.000000000 +0100
@@ -136,7 +136,8 @@ void show_page_walk(unsigned long addr)
     l4t = mfn_to_virt(mfn);
     l4e = l4t[l4_table_offset(addr)];
     mfn = l4e_get_pfn(l4e);
-    pfn = get_gpfn_from_mfn(mfn);
+    pfn = mfn_valid(mfn) && mpt_valid ?
+          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
     printk(" L4[0x%03lx] = %"PRIpte" %016lx\n",
            l4_table_offset(addr), l4e_get_intpte(l4e), pfn);
     if ( !(l4e_get_flags(l4e) & _PAGE_PRESENT) )
@@ -145,7 +146,8 @@ void show_page_walk(unsigned long addr)
     l3t = mfn_to_virt(mfn);
     l3e = l3t[l3_table_offset(addr)];
     mfn = l3e_get_pfn(l3e);
-    pfn = get_gpfn_from_mfn(mfn);
+    pfn = mfn_valid(mfn) && mpt_valid ?
+          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
     printk(" L3[0x%03lx] = %"PRIpte" %016lx\n",
            l3_table_offset(addr), l3e_get_intpte(l3e), pfn);
     if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
@@ -154,7 +156,8 @@ void show_page_walk(unsigned long addr)
     l2t = mfn_to_virt(mfn);
     l2e = l2t[l2_table_offset(addr)];
     mfn = l2e_get_pfn(l2e);
-    pfn = get_gpfn_from_mfn(mfn);
+    pfn = mfn_valid(mfn) && mpt_valid ?
+          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
     printk(" L2[0x%03lx] = %"PRIpte" %016lx %s\n",
            l2_table_offset(addr), l2e_get_intpte(l2e), pfn,
            (l2e_get_flags(l2e) & _PAGE_PSE) ? "(PSE)" :
"");
@@ -165,7 +168,8 @@ void show_page_walk(unsigned long addr)
     l1t = mfn_to_virt(mfn);
     l1e = l1t[l1_table_offset(addr)];
     mfn = l1e_get_pfn(l1e);
-    pfn = get_gpfn_from_mfn(mfn);
+    pfn = mfn_valid(mfn) && mpt_valid ?
+          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
     printk(" L1[0x%03lx] = %"PRIpte" %016lx\n",
            l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
 }
Index: 2008-01-18/xen/include/asm-x86/mm.h
==================================================================---
2008-01-18.orig/xen/include/asm-x86/mm.h	2008-01-23 11:54:09.000000000 +0100
+++ 2008-01-18/xen/include/asm-x86/mm.h	2008-01-23 11:55:37.000000000 +0100
@@ -267,6 +267,7 @@ TYPE_SAFE(unsigned long,mfn);
 #define machine_to_phys_mapping  ((unsigned long *)RDWR_MPT_VIRT_START)
 #define INVALID_M2P_ENTRY        (~0UL)
 #define VALID_M2P(_e)            (!((_e) & (1UL<<(BITS_PER_LONG-1))))
+extern int mpt_valid;
 
 #ifdef CONFIG_COMPAT
 #define compat_machine_to_phys_mapping ((unsigned int
*)RDWR_COMPAT_MPT_VIRT_START)



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

Keir Fraser

2008-Jan-24 15:35 UTC

head link

Re: [Xen-devel] [PATCH] x86: make show_page_walk() more robust

How can show_page_walk() be called before paging_init() sets up the M2P
table? It is invoked only from exception and interrupt handlers, which are
not installed until trap_init() has run. And trap_init() is invoked later
than paging_init() during boot.

 -- Keir

On 24/1/08 15:03, "Jan Beulich" <jbeulich@novell.com> wrote:
> While adding 1Gb page support for the 1:1 mapping I noticed that
> show_page_walk() crashes when used before the M2P table gets created,
> and from looking at the code I realized that it would also crash if a
> corrupt page table with an out of range MFN would be encountered.
> While it would have been possible to make get_gpfn_from_mfn() more
> robust, it seemed like keeping the overhead there low (as in the
> general case proper values can be expected and would likely have been
> checked for already), and hence I made the checks privates to
> show_page_walk().
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> Index: 2008-01-18/xen/arch/x86/x86_32/mm.c
> ==================================================================> ---
2008-01-18.orig/xen/arch/x86/x86_32/mm.c 2008-01-23 11:54:09.000000000
> +0100
> +++ 2008-01-18/xen/arch/x86/x86_32/mm.c 2008-01-23 11:48:16.000000000 +0100
> @@ -41,6 +41,7 @@ l2_pgentry_t __attribute__ ((__section__
>  unsigned int PAGE_HYPERVISOR         = __PAGE_HYPERVISOR;
>  unsigned int PAGE_HYPERVISOR_NOCACHE = __PAGE_HYPERVISOR_NOCACHE;
>  
> +int mpt_valid;
>  static unsigned long mpt_size;
>  
>  void *alloc_xen_pagetable(void)
> @@ -110,6 +111,8 @@ void __init paging_init(void)
>                        pg, (__PAGE_HYPERVISOR | _PAGE_PSE) &
~_PAGE_RW));
>      }
>  
> +    mpt_valid = 1;
> +
>      /* Fill with an obvious debug pattern. */
>      for ( i = 0; i < (mpt_size / BYTES_PER_LONG); i++)
>          set_gpfn_from_mfn(i, 0x55555555);
> Index: 2008-01-18/xen/arch/x86/x86_32/traps.c
> ==================================================================> ---
2008-01-18.orig/xen/arch/x86/x86_32/traps.c 2008-01-23 11:54:09.000000000
> +0100
> +++ 2008-01-18/xen/arch/x86/x86_32/traps.c 2008-01-23 11:53:58.000000000
+0100
> @@ -132,7 +132,8 @@ void show_page_walk(unsigned long addr)
>      l3t += (cr3 & 0xFE0UL) >> 3;
>      l3e = l3t[l3_table_offset(addr)];
>      mfn = l3e_get_pfn(l3e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L3[0x%03lx] = %"PRIpte" %08lx\n",
>             l3_table_offset(addr), l3e_get_intpte(l3e), pfn);
>      unmap_domain_page(l3t);
> @@ -143,7 +144,8 @@ void show_page_walk(unsigned long addr)
>      l2t = map_domain_page(mfn);
>      l2e = l2t[l2_table_offset(addr)];
>      mfn = l2e_get_pfn(l2e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L2[0x%03lx] = %"PRIpte" %08lx %s\n",
>             l2_table_offset(addr), l2e_get_intpte(l2e), pfn,
>             (l2e_get_flags(l2e) & _PAGE_PSE) ? "(PSE)" :
"");
> @@ -155,7 +157,8 @@ void show_page_walk(unsigned long addr)
>      l1t = map_domain_page(mfn);
>      l1e = l1t[l1_table_offset(addr)];
>      mfn = l1e_get_pfn(l1e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L1[0x%03lx] = %"PRIpte" %08lx\n",
>             l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
>      unmap_domain_page(l1t);
> Index: 2008-01-18/xen/arch/x86/x86_64/mm.c
> ==================================================================> ---
2008-01-18.orig/xen/arch/x86/x86_64/mm.c 2008-01-23 11:54:09.000000000
> +0100
> +++ 2008-01-18/xen/arch/x86/x86_64/mm.c 2008-01-23 11:50:56.000000000 +0100
> @@ -32,6 +32,7 @@
>  #include <asm/msr.h>
>  #include <public/memory.h>
>  
> +int mpt_valid;
>  #ifdef CONFIG_COMPAT
>  unsigned int m2p_compat_vstart = __HYPERVISOR_COMPAT_VIRT_START;
>  #endif
> @@ -144,6 +145,8 @@ void __init paging_init(void)
>          l2_ro_mpt++;
>      }
>  
> +    mpt_valid = 1;
> +
>      /* Create user-accessible L2 directory to map the MPT for compat
guests.
> */
>      BUILD_BUG_ON(l4_table_offset(RDWR_MPT_VIRT_START) !>               
l4_table_offset(HIRO_COMPAT_MPT_VIRT_START));
> Index: 2008-01-18/xen/arch/x86/x86_64/traps.c
> ==================================================================> ---
2008-01-18.orig/xen/arch/x86/x86_64/traps.c 2008-01-23 11:54:09.000000000
> +0100
> +++ 2008-01-18/xen/arch/x86/x86_64/traps.c 2008-01-23 11:53:27.000000000
+0100
> @@ -136,7 +136,8 @@ void show_page_walk(unsigned long addr)
>      l4t = mfn_to_virt(mfn);
>      l4e = l4t[l4_table_offset(addr)];
>      mfn = l4e_get_pfn(l4e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L4[0x%03lx] = %"PRIpte" %016lx\n",
>             l4_table_offset(addr), l4e_get_intpte(l4e), pfn);
>      if ( !(l4e_get_flags(l4e) & _PAGE_PRESENT) )
> @@ -145,7 +146,8 @@ void show_page_walk(unsigned long addr)
>      l3t = mfn_to_virt(mfn);
>      l3e = l3t[l3_table_offset(addr)];
>      mfn = l3e_get_pfn(l3e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L3[0x%03lx] = %"PRIpte" %016lx\n",
>             l3_table_offset(addr), l3e_get_intpte(l3e), pfn);
>      if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
> @@ -154,7 +156,8 @@ void show_page_walk(unsigned long addr)
>      l2t = mfn_to_virt(mfn);
>      l2e = l2t[l2_table_offset(addr)];
>      mfn = l2e_get_pfn(l2e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L2[0x%03lx] = %"PRIpte" %016lx %s\n",
>             l2_table_offset(addr), l2e_get_intpte(l2e), pfn,
>             (l2e_get_flags(l2e) & _PAGE_PSE) ? "(PSE)" :
"");
> @@ -165,7 +168,8 @@ void show_page_walk(unsigned long addr)
>      l1t = mfn_to_virt(mfn);
>      l1e = l1t[l1_table_offset(addr)];
>      mfn = l1e_get_pfn(l1e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L1[0x%03lx] = %"PRIpte" %016lx\n",
>             l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
>  }
> Index: 2008-01-18/xen/include/asm-x86/mm.h
> ==================================================================> ---
2008-01-18.orig/xen/include/asm-x86/mm.h 2008-01-23 11:54:09.000000000
> +0100
> +++ 2008-01-18/xen/include/asm-x86/mm.h 2008-01-23 11:55:37.000000000 +0100
> @@ -267,6 +267,7 @@ TYPE_SAFE(unsigned long,mfn);
>  #define machine_to_phys_mapping  ((unsigned long *)RDWR_MPT_VIRT_START)
>  #define INVALID_M2P_ENTRY        (~0UL)
>  #define VALID_M2P(_e)            (!((_e) &
(1UL<<(BITS_PER_LONG-1))))
> +extern int mpt_valid;
>  
>  #ifdef CONFIG_COMPAT
>  #define compat_machine_to_phys_mapping ((unsigned int
> *)RDWR_COMPAT_MPT_VIRT_START)
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


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

Jan Beulich

2008-Jan-24 16:00 UTC

head link

Re: [Xen-devel] [PATCH] x86: make show_page_walk() more robust

As I said, I found this during debugging - I had added explicit calls to
show_page_walk(), and am also considering adding it to the early page
fault handler (which provides pretty little information at present - the
stub handler for all other exceptions is doing better - and does dubious
[to me] things with a dubious [to me] comment).

And regardless of that, the mfn_valid() check is absolutely required.

Jan
>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 24.01.08 16:35
>>>How can show_page_walk() be called before paging_init() sets up the M2P
table? It is invoked only from exception and interrupt handlers, which are
not installed until trap_init() has run. And trap_init() is invoked later
than paging_init() during boot.

 -- Keir

On 24/1/08 15:03, "Jan Beulich" <jbeulich@novell.com> wrote:
> While adding 1Gb page support for the 1:1 mapping I noticed that
> show_page_walk() crashes when used before the M2P table gets created,
> and from looking at the code I realized that it would also crash if a
> corrupt page table with an out of range MFN would be encountered.
> While it would have been possible to make get_gpfn_from_mfn() more
> robust, it seemed like keeping the overhead there low (as in the
> general case proper values can be expected and would likely have been
> checked for already), and hence I made the checks privates to
> show_page_walk().
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> Index: 2008-01-18/xen/arch/x86/x86_32/mm.c
> ==================================================================> ---
2008-01-18.orig/xen/arch/x86/x86_32/mm.c 2008-01-23 11:54:09.000000000
> +0100
> +++ 2008-01-18/xen/arch/x86/x86_32/mm.c 2008-01-23 11:48:16.000000000 +0100
> @@ -41,6 +41,7 @@ l2_pgentry_t __attribute__ ((__section__
>  unsigned int PAGE_HYPERVISOR         = __PAGE_HYPERVISOR;
>  unsigned int PAGE_HYPERVISOR_NOCACHE = __PAGE_HYPERVISOR_NOCACHE;
>  
> +int mpt_valid;
>  static unsigned long mpt_size;
>  
>  void *alloc_xen_pagetable(void)
> @@ -110,6 +111,8 @@ void __init paging_init(void)
>                        pg, (__PAGE_HYPERVISOR | _PAGE_PSE) &
~_PAGE_RW));
>      }
>  
> +    mpt_valid = 1;
> +
>      /* Fill with an obvious debug pattern. */
>      for ( i = 0; i < (mpt_size / BYTES_PER_LONG); i++)
>          set_gpfn_from_mfn(i, 0x55555555);
> Index: 2008-01-18/xen/arch/x86/x86_32/traps.c
> ==================================================================> ---
2008-01-18.orig/xen/arch/x86/x86_32/traps.c 2008-01-23 11:54:09.000000000
> +0100
> +++ 2008-01-18/xen/arch/x86/x86_32/traps.c 2008-01-23 11:53:58.000000000
+0100
> @@ -132,7 +132,8 @@ void show_page_walk(unsigned long addr)
>      l3t += (cr3 & 0xFE0UL) >> 3;
>      l3e = l3t[l3_table_offset(addr)];
>      mfn = l3e_get_pfn(l3e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L3[0x%03lx] = %"PRIpte" %08lx\n",
>             l3_table_offset(addr), l3e_get_intpte(l3e), pfn);
>      unmap_domain_page(l3t);
> @@ -143,7 +144,8 @@ void show_page_walk(unsigned long addr)
>      l2t = map_domain_page(mfn);
>      l2e = l2t[l2_table_offset(addr)];
>      mfn = l2e_get_pfn(l2e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L2[0x%03lx] = %"PRIpte" %08lx %s\n",
>             l2_table_offset(addr), l2e_get_intpte(l2e), pfn,
>             (l2e_get_flags(l2e) & _PAGE_PSE) ? "(PSE)" :
"");
> @@ -155,7 +157,8 @@ void show_page_walk(unsigned long addr)
>      l1t = map_domain_page(mfn);
>      l1e = l1t[l1_table_offset(addr)];
>      mfn = l1e_get_pfn(l1e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L1[0x%03lx] = %"PRIpte" %08lx\n",
>             l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
>      unmap_domain_page(l1t);
> Index: 2008-01-18/xen/arch/x86/x86_64/mm.c
> ==================================================================> ---
2008-01-18.orig/xen/arch/x86/x86_64/mm.c 2008-01-23 11:54:09.000000000
> +0100
> +++ 2008-01-18/xen/arch/x86/x86_64/mm.c 2008-01-23 11:50:56.000000000 +0100
> @@ -32,6 +32,7 @@
>  #include <asm/msr.h>
>  #include <public/memory.h>
>  
> +int mpt_valid;
>  #ifdef CONFIG_COMPAT
>  unsigned int m2p_compat_vstart = __HYPERVISOR_COMPAT_VIRT_START;
>  #endif
> @@ -144,6 +145,8 @@ void __init paging_init(void)
>          l2_ro_mpt++;
>      }
>  
> +    mpt_valid = 1;
> +
>      /* Create user-accessible L2 directory to map the MPT for compat
guests.
> */
>      BUILD_BUG_ON(l4_table_offset(RDWR_MPT_VIRT_START) !>               
l4_table_offset(HIRO_COMPAT_MPT_VIRT_START));
> Index: 2008-01-18/xen/arch/x86/x86_64/traps.c
> ==================================================================> ---
2008-01-18.orig/xen/arch/x86/x86_64/traps.c 2008-01-23 11:54:09.000000000
> +0100
> +++ 2008-01-18/xen/arch/x86/x86_64/traps.c 2008-01-23 11:53:27.000000000
+0100
> @@ -136,7 +136,8 @@ void show_page_walk(unsigned long addr)
>      l4t = mfn_to_virt(mfn);
>      l4e = l4t[l4_table_offset(addr)];
>      mfn = l4e_get_pfn(l4e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L4[0x%03lx] = %"PRIpte" %016lx\n",
>             l4_table_offset(addr), l4e_get_intpte(l4e), pfn);
>      if ( !(l4e_get_flags(l4e) & _PAGE_PRESENT) )
> @@ -145,7 +146,8 @@ void show_page_walk(unsigned long addr)
>      l3t = mfn_to_virt(mfn);
>      l3e = l3t[l3_table_offset(addr)];
>      mfn = l3e_get_pfn(l3e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L3[0x%03lx] = %"PRIpte" %016lx\n",
>             l3_table_offset(addr), l3e_get_intpte(l3e), pfn);
>      if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
> @@ -154,7 +156,8 @@ void show_page_walk(unsigned long addr)
>      l2t = mfn_to_virt(mfn);
>      l2e = l2t[l2_table_offset(addr)];
>      mfn = l2e_get_pfn(l2e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L2[0x%03lx] = %"PRIpte" %016lx %s\n",
>             l2_table_offset(addr), l2e_get_intpte(l2e), pfn,
>             (l2e_get_flags(l2e) & _PAGE_PSE) ? "(PSE)" :
"");
> @@ -165,7 +168,8 @@ void show_page_walk(unsigned long addr)
>      l1t = mfn_to_virt(mfn);
>      l1e = l1t[l1_table_offset(addr)];
>      mfn = l1e_get_pfn(l1e);
> -    pfn = get_gpfn_from_mfn(mfn);
> +    pfn = mfn_valid(mfn) && mpt_valid ?
> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>      printk(" L1[0x%03lx] = %"PRIpte" %016lx\n",
>             l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
>  }
> Index: 2008-01-18/xen/include/asm-x86/mm.h
> ==================================================================> ---
2008-01-18.orig/xen/include/asm-x86/mm.h 2008-01-23 11:54:09.000000000
> +0100
> +++ 2008-01-18/xen/include/asm-x86/mm.h 2008-01-23 11:55:37.000000000 +0100
> @@ -267,6 +267,7 @@ TYPE_SAFE(unsigned long,mfn);
>  #define machine_to_phys_mapping  ((unsigned long *)RDWR_MPT_VIRT_START)
>  #define INVALID_M2P_ENTRY        (~0UL)
>  #define VALID_M2P(_e)            (!((_e) &
(1UL<<(BITS_PER_LONG-1))))
> +extern int mpt_valid;
>  
>  #ifdef CONFIG_COMPAT
>  #define compat_machine_to_phys_mapping ((unsigned int
> *)RDWR_COMPAT_MPT_VIRT_START)
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com 
> http://lists.xensource.com/xen-devel 


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

Keir Fraser

2008-Jan-24 16:03 UTC

head link

Re: [Xen-devel] [PATCH] x86: make show_page_walk() more robust

On 24/1/08 16:00, "Jan Beulich" <jbeulich@novell.com> wrote:
> As I said, I found this during debugging - I had added explicit calls to
> show_page_walk(), and am also considering adding it to the early page
> fault handler (which provides pretty little information at present - the
> stub handler for all other exceptions is doing better - and does dubious
> [to me] things with a dubious [to me] comment).
The problem that the early_page_fault() handler comments talks about does
actually happen on some hardware.
> And regardless of that, the mfn_valid() check is absolutely required.
Yes, I''ll take that bit.

 -- Keir
> Jan
> 
>>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 24.01.08 16:35
>>>
> How can show_page_walk() be called before paging_init() sets up the M2P
> table? It is invoked only from exception and interrupt handlers, which are
> not installed until trap_init() has run. And trap_init() is invoked later
> than paging_init() during boot.
> 
>  -- Keir
> 
> On 24/1/08 15:03, "Jan Beulich" <jbeulich@novell.com>
wrote:
> 
>> While adding 1Gb page support for the 1:1 mapping I noticed that
>> show_page_walk() crashes when used before the M2P table gets created,
>> and from looking at the code I realized that it would also crash if a
>> corrupt page table with an out of range MFN would be encountered.
>> While it would have been possible to make get_gpfn_from_mfn() more
>> robust, it seemed like keeping the overhead there low (as in the
>> general case proper values can be expected and would likely have been
>> checked for already), and hence I made the checks privates to
>> show_page_walk().
>> 
>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>> 
>> Index: 2008-01-18/xen/arch/x86/x86_32/mm.c
>>
==================================================================>> ---
2008-01-18.orig/xen/arch/x86/x86_32/mm.c 2008-01-23 11:54:09.000000000
>> +0100
>> +++ 2008-01-18/xen/arch/x86/x86_32/mm.c 2008-01-23 11:48:16.000000000
+0100
>> @@ -41,6 +41,7 @@ l2_pgentry_t __attribute__ ((__section__
>>  unsigned int PAGE_HYPERVISOR         = __PAGE_HYPERVISOR;
>>  unsigned int PAGE_HYPERVISOR_NOCACHE = __PAGE_HYPERVISOR_NOCACHE;
>>  
>> +int mpt_valid;
>>  static unsigned long mpt_size;
>>  
>>  void *alloc_xen_pagetable(void)
>> @@ -110,6 +111,8 @@ void __init paging_init(void)
>>                        pg, (__PAGE_HYPERVISOR | _PAGE_PSE) &
~_PAGE_RW));
>>      }
>>  
>> +    mpt_valid = 1;
>> +
>>      /* Fill with an obvious debug pattern. */
>>      for ( i = 0; i < (mpt_size / BYTES_PER_LONG); i++)
>>          set_gpfn_from_mfn(i, 0x55555555);
>> Index: 2008-01-18/xen/arch/x86/x86_32/traps.c
>>
==================================================================>> ---
2008-01-18.orig/xen/arch/x86/x86_32/traps.c 2008-01-23 11:54:09.000000000
>> +0100
>> +++ 2008-01-18/xen/arch/x86/x86_32/traps.c 2008-01-23
11:53:58.000000000
>> +0100
>> @@ -132,7 +132,8 @@ void show_page_walk(unsigned long addr)
>>      l3t += (cr3 & 0xFE0UL) >> 3;
>>      l3e = l3t[l3_table_offset(addr)];
>>      mfn = l3e_get_pfn(l3e);
>> -    pfn = get_gpfn_from_mfn(mfn);
>> +    pfn = mfn_valid(mfn) && mpt_valid ?
>> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>>      printk(" L3[0x%03lx] = %"PRIpte" %08lx\n",
>>             l3_table_offset(addr), l3e_get_intpte(l3e), pfn);
>>      unmap_domain_page(l3t);
>> @@ -143,7 +144,8 @@ void show_page_walk(unsigned long addr)
>>      l2t = map_domain_page(mfn);
>>      l2e = l2t[l2_table_offset(addr)];
>>      mfn = l2e_get_pfn(l2e);
>> -    pfn = get_gpfn_from_mfn(mfn);
>> +    pfn = mfn_valid(mfn) && mpt_valid ?
>> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>>      printk(" L2[0x%03lx] = %"PRIpte" %08lx %s\n",
>>             l2_table_offset(addr), l2e_get_intpte(l2e), pfn,
>>             (l2e_get_flags(l2e) & _PAGE_PSE) ? "(PSE)" :
"");
>> @@ -155,7 +157,8 @@ void show_page_walk(unsigned long addr)
>>      l1t = map_domain_page(mfn);
>>      l1e = l1t[l1_table_offset(addr)];
>>      mfn = l1e_get_pfn(l1e);
>> -    pfn = get_gpfn_from_mfn(mfn);
>> +    pfn = mfn_valid(mfn) && mpt_valid ?
>> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>>      printk(" L1[0x%03lx] = %"PRIpte" %08lx\n",
>>             l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
>>      unmap_domain_page(l1t);
>> Index: 2008-01-18/xen/arch/x86/x86_64/mm.c
>>
==================================================================>> ---
2008-01-18.orig/xen/arch/x86/x86_64/mm.c 2008-01-23 11:54:09.000000000
>> +0100
>> +++ 2008-01-18/xen/arch/x86/x86_64/mm.c 2008-01-23 11:50:56.000000000
+0100
>> @@ -32,6 +32,7 @@
>>  #include <asm/msr.h>
>>  #include <public/memory.h>
>>  
>> +int mpt_valid;
>>  #ifdef CONFIG_COMPAT
>>  unsigned int m2p_compat_vstart = __HYPERVISOR_COMPAT_VIRT_START;
>>  #endif
>> @@ -144,6 +145,8 @@ void __init paging_init(void)
>>          l2_ro_mpt++;
>>      }
>>  
>> +    mpt_valid = 1;
>> +
>>      /* Create user-accessible L2 directory to map the MPT for compat
guests.
>> */
>>      BUILD_BUG_ON(l4_table_offset(RDWR_MPT_VIRT_START) !>>       
l4_table_offset(HIRO_COMPAT_MPT_VIRT_START));
>> Index: 2008-01-18/xen/arch/x86/x86_64/traps.c
>>
==================================================================>> ---
2008-01-18.orig/xen/arch/x86/x86_64/traps.c 2008-01-23 11:54:09.000000000
>> +0100
>> +++ 2008-01-18/xen/arch/x86/x86_64/traps.c 2008-01-23
11:53:27.000000000
>> +0100
>> @@ -136,7 +136,8 @@ void show_page_walk(unsigned long addr)
>>      l4t = mfn_to_virt(mfn);
>>      l4e = l4t[l4_table_offset(addr)];
>>      mfn = l4e_get_pfn(l4e);
>> -    pfn = get_gpfn_from_mfn(mfn);
>> +    pfn = mfn_valid(mfn) && mpt_valid ?
>> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>>      printk(" L4[0x%03lx] = %"PRIpte" %016lx\n",
>>             l4_table_offset(addr), l4e_get_intpte(l4e), pfn);
>>      if ( !(l4e_get_flags(l4e) & _PAGE_PRESENT) )
>> @@ -145,7 +146,8 @@ void show_page_walk(unsigned long addr)
>>      l3t = mfn_to_virt(mfn);
>>      l3e = l3t[l3_table_offset(addr)];
>>      mfn = l3e_get_pfn(l3e);
>> -    pfn = get_gpfn_from_mfn(mfn);
>> +    pfn = mfn_valid(mfn) && mpt_valid ?
>> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>>      printk(" L3[0x%03lx] = %"PRIpte" %016lx\n",
>>             l3_table_offset(addr), l3e_get_intpte(l3e), pfn);
>>      if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
>> @@ -154,7 +156,8 @@ void show_page_walk(unsigned long addr)
>>      l2t = mfn_to_virt(mfn);
>>      l2e = l2t[l2_table_offset(addr)];
>>      mfn = l2e_get_pfn(l2e);
>> -    pfn = get_gpfn_from_mfn(mfn);
>> +    pfn = mfn_valid(mfn) && mpt_valid ?
>> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>>      printk(" L2[0x%03lx] = %"PRIpte" %016lx %s\n",
>>             l2_table_offset(addr), l2e_get_intpte(l2e), pfn,
>>             (l2e_get_flags(l2e) & _PAGE_PSE) ? "(PSE)" :
"");
>> @@ -165,7 +168,8 @@ void show_page_walk(unsigned long addr)
>>      l1t = mfn_to_virt(mfn);
>>      l1e = l1t[l1_table_offset(addr)];
>>      mfn = l1e_get_pfn(l1e);
>> -    pfn = get_gpfn_from_mfn(mfn);
>> +    pfn = mfn_valid(mfn) && mpt_valid ?
>> +          get_gpfn_from_mfn(mfn) : INVALID_M2P_ENTRY;
>>      printk(" L1[0x%03lx] = %"PRIpte" %016lx\n",
>>             l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
>>  }
>> Index: 2008-01-18/xen/include/asm-x86/mm.h
>>
==================================================================>> ---
2008-01-18.orig/xen/include/asm-x86/mm.h 2008-01-23 11:54:09.000000000
>> +0100
>> +++ 2008-01-18/xen/include/asm-x86/mm.h 2008-01-23 11:55:37.000000000
+0100
>> @@ -267,6 +267,7 @@ TYPE_SAFE(unsigned long,mfn);
>>  #define machine_to_phys_mapping  ((unsigned long
*)RDWR_MPT_VIRT_START)
>>  #define INVALID_M2P_ENTRY        (~0UL)
>>  #define VALID_M2P(_e)            (!((_e) &
(1UL<<(BITS_PER_LONG-1))))
>> +extern int mpt_valid;
>>  
>>  #ifdef CONFIG_COMPAT
>>  #define compat_machine_to_phys_mapping ((unsigned int
>> *)RDWR_COMPAT_MPT_VIRT_START)
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
> 
> 


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

Xen devel - Jan 2008 - [PATCH] x86: make show_page_walk() more robust

[Xen-devel] [PATCH] x86: make show_page_walk() more robust

Re: [Xen-devel] [PATCH] x86: make show_page_walk() more robust

Re: [Xen-devel] [PATCH] x86: make show_page_walk() more robust

Re: [Xen-devel] [PATCH] x86: make show_page_walk() more robust