From: Keir Fraser <Keir.Fraser@cl.cam.ac.uk>
To: Jan Beulich <jbeulich@novell.com>
Cc: xen-devel@lists.xensource.com
Subject: Re: [PATCH] x86: make show_page_walk() more robust
Date: Thu, 24 Jan 2008 16:03:27 +0000 [thread overview]
Message-ID: <C3BE674F.1B622%Keir.Fraser@cl.cam.ac.uk> (raw)
In-Reply-To: <4798C432.76E4.0078.0@novell.com>
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
>
>
prev parent reply other threads:[~2008-01-24 16:03 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-01-24 15:03 [PATCH] x86: make show_page_walk() more robust Jan Beulich
2008-01-24 15:35 ` Keir Fraser
2008-01-24 16:00 ` Jan Beulich
2008-01-24 16:03 ` Keir Fraser [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=C3BE674F.1B622%Keir.Fraser@cl.cam.ac.uk \
--to=keir.fraser@cl.cam.ac.uk \
--cc=jbeulich@novell.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.