WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: "Keir Fraser" <Keir.Fraser@xxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] x86: make show_page_walk() more robust
From: "Jan Beulich" <jbeulich@xxxxxxxxxx>
Date: Thu, 24 Jan 2008 16:00:34 +0000
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Thu, 24 Jan 2008 08:00:42 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <C3BE60BA.1B618%Keir.Fraser@xxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4798B6E3.76E4.0078.0@xxxxxxxxxx> <C3BE60BA.1B618%Keir.Fraser@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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@xxxxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxxxxx 
> http://lists.xensource.com/xen-devel 



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