lkml.org 
[lkml]   [2011]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] xen/mmu: Add workaround "x86-64, mm: Put early page table high"
On Tue, May 03, 2011 at 02:55:27AM +0200, Daniel Kiper wrote:
> On Mon, May 02, 2011 at 01:22:21PM -0400, Konrad Rzeszutek Wilk wrote:
> > As a consequence of the commit:
> >
> > commit 4b239f458c229de044d6905c2b0f9fe16ed9e01e
> > Author: Yinghai Lu <yinghai@kernel.org>
> > Date: Fri Dec 17 16:58:28 2010 -0800
> >
> > x86-64, mm: Put early page table high
> >
> > it causes the Linux kernel to crash under Xen:
> >
> > mapping kernel into physical memory
> > Xen: setup ISA identity maps
> > about to get started...
> > (XEN) mm.c:2466:d0 Bad type (saw 7400000000000001 != exp 1000000000000000) for mfn b1d89 (pfn bacf7)
> > (XEN) mm.c:3027:d0 Error while pinning mfn b1d89
> > (XEN) traps.c:481:d0 Unhandled invalid opcode fault/trap [#6] on VCPU 0 [ec=0000]
> > (XEN) domain_crash_sync called from entry.S
> > (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
> > ...
>
> I was hit by this bug when I was working on memory hotplug.
> After some investigation I found myself above mentioned patch
> as a guilty and later I discovered that you are working on that
> issue. I have tested your patch and discoverd some issues with it.
> First of all it has compilation issues on gcc version 4.1.2 20061115
> (prerelease) (Debian 4.1.1-21). Details below.
>
> Additionlly, I think that your patch does not work as you expected.
> I found that git commit 24bdb0b62cc82120924762ae6bc85afc8c3f2b26
> (xen: do not create the extra e820 region at an addr lower than 4G)
> do this work (to some extent). When this patch is removed domU
> is crashing with following error:

Which is "this patch" ? The 24bdb0b62cc82120924762ae6bc85afc8c3f2b26?

>
> (early) Linux version 2.6.39-rc5-x86_64.xenU.all.r0+ (root@dev-00) (gcc version 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)) #5 SMP Tue May 3 01:43:26 CEST 2011
> (early) Command line: root=/dev/xvda debug earlyprintk=xen noapic nolapic console=hvc0
> (early) ACPI in unprivileged domain disabled
> (early) released 0 pages of unused memory
> (early) Set 0 page(s) to 1-1 mapping.
> (early) BIOS-provided physical RAM map:
> (early) Xen: 0000000000000000 - 00000000000a0000 (usable)
> (early) Xen: 00000000000a0000 - 0000000000100000 (reserved)
> (early) Xen: 0000000000100000 - 0000000026000000 (usable)
> (early) bootconsole [xenboot0] enabled
> (early) NX (Execute Disable) protection: active
> (early) DMI not present or invalid.
> (early) e820 update range: 0000000000000000 - 0000000000010000 (early) (usable)(early) ==> (early) (reserved)(early)
> (early) e820 remove range: 00000000000a0000 - 0000000000100000 (early) (usable)(early)
> (early) No AGP bridge found
> (early) last_pfn = 0x26000 max_arch_pfn = 0x400000000
> (early) initial memory mapped : 0 - 01693000
> (early) Base memory trampoline at [ffff88000009e000] 9e000 size 8192
> (early) init_memory_mapping: 0000000000000000-0000000026000000
> (early) 0000000000 - 0026000000 page 4k
> (early) kernel direct mapping tables up to 26000000 @ 256ce000-25800000
> (early) BUG: unable to handle kernel (early) NULL pointer dereference(early) at (null)
> (early) IP:(early) [<ffffffff814a33f2>] find_range_array+0x4e/0x57
> (early) PGD 0 (early)
> (early) Oops: 0003 [#1] (early) SMP (early)
> (early) last sysfs file:
> (early) CPU 0 (early)
> (early) Modules linked in:(early)
> (early)
> (early) Pid: 0, comm: swapper Not tainted 2.6.39-rc5-x86_64.xenU.all.r0+ #5(early) (early)
> (early) RIP: e030:[<ffffffff814a33f2>] (early) [<ffffffff814a33f2>] find_range_array+0x4e/0x57
> (early) RSP: e02b:ffffffff81427e58 EFLAGS: 00010046
> (early) RAX: 0000000000000000 RBX: 00000000000000e0 RCX: 00000000000000e0
> (early) RDX: ffff8800257fff20 RSI: 00000000257fff20 RDI: ffff8800257fff20
> (early) RBP: ffffffff81427e68 R08: 0000000000000005 R09: 0000000000000050
> (early) R10: 0000000000000005 R11: 0000000025800000 R12: ffffffff814bd000
> (early) R13: 0000000000000000 R14: 000000000000000e R15: 0000000000001000
> (early) FS: 0000000000000000(0000) GS:ffffffff8147f000(0000) knlGS:0000000000000000
> (early) CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033
> (early) CR2: 0000000000000000 CR3: 0000000001441000 CR4: 0000000000002660
> (early) DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> (early) DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> (early) Process swapper (pid: 0, threadinfo ffffffff81426000, task ffffffff81449020)
> (early) Stack:
> (early) ffffffff81427e88(early) 0000000000000000(early) ffffffff81427ea8(early) ffffffff814a343d(early)
> (early) 0000000000000100(early) 0000000026000000(early) ffffffff814bd000(early) ffffffffffffffff(early)
> (early) 0000000000000000(early) 0000000000000000(early) ffffffff81427eb8(early) ffffffff814a3577(early)
> (early) Call Trace:
> (early) [<ffffffff814a343d>] __memblock_x86_memory_in_range+0x42/0x171
> (early) [<ffffffff814a3577>] memblock_x86_memory_in_range+0xb/0xd
> (early) [<ffffffff81497348>] memblock_find_dma_reserve+0x15/0x3b
> (early) [<ffffffff81496b50>] setup_arch+0x721/0x7e5
> (early) [<ffffffff810065fd>] ? __raw_callee_save_xen_irq_disable+0x11/0x1e
> (early) [<ffffffff81493955>] start_kernel+0x8a/0x2db
> (early) [<ffffffff81493299>] x86_64_start_reservations+0x84/0x88
> (early) [<ffffffff814947d9>] xen_start_kernel+0x3e1/0x3e8
> (early) Code: (early) 66 (early) 00 (early) 00 (early) 48 (early) 85 (early) c0 (early) 75 (early) 0c (early) 48 (early) c7 (early) c7 (early) 86 (early) 80 (early) 3a (early) 81 (early) e8 (early) 55 (early) 41 (early) b9 (early) ff (early) 48 (early) bf (early) 00 (early) 00 (early) 00 (early) 00 (early) 00 (early) 88 (early) ff (early) ff (early) 48 (early) 89 (early) d9 (early) 48 (early) 8d (early) 14 (early) 38 (early) 31 (early) c0 (early) fc (early) 48 (early) 89 (early) d7 (early) <f3> (early) aa (early) 48 (early) 89 (early) d0 (early) 5f (early) 5b (early) c9 (early) c3 (early) 55 (early) 48 (early) 89 (early) e5 (early) 41 (early) 57 (early) 49 (early) 89 (early) f7 (early) 49 (early) c1 (early) ef (early)
> (early) RIP (early) [<ffffffff814a33f2>] find_range_array+0x4e/0x57
> (early) RSP <ffffffff81427e58>
> (early) CR2: 0000000000000000
> (early) ---[ end trace 4eaa2a86a8e2da22 ]---
> (early) Kernel panic - not syncing: Attempted to kill the idle task!
> (early) Pid: 0, comm: swapper Tainted: G D 2.6.39-rc5-x86_64.xenU.all.r0+ #5
> (early) Call Trace:
> (early) [<ffffffff810375ed>] panic+0xbd/0x1c7
> (early) [<ffffffff810383c7>] ? printk+0x67/0x69
> (early) [<ffffffff811fd7b0>] ? account+0xe1/0xf0
> (early) [<ffffffff8103a641>] do_exit+0xb4/0x676
> (early) [<ffffffff81037b53>] ? spin_unlock_irqrestore+0x9/0xb
> (early) [<ffffffff81038c24>] ? kmsg_dump+0x4a/0xd9
> (early) [<ffffffff8100d11d>] oops_end+0xc1/0xc9
> (early) [<ffffffff810252b1>] no_context+0x1f5/0x204
> (early) [<ffffffff81025448>] __bad_area_nosemaphore+0x188/0x1ab
> (early) [<ffffffff810065df>] ? __raw_callee_save_xen_restore_fl+0x11/0x1e
> (early) [<ffffffff810254e1>] bad_area_nosemaphore+0xe/0x10
> (early) [<ffffffff81025991>] do_page_fault+0x18c/0x337
> (early) [<ffffffff810065df>] ? __raw_callee_save_xen_restore_fl+0x11/0x1e
> (early) [<ffffffff810065df>] ? __raw_callee_save_xen_restore_fl+0x11/0x1e
> (early) [<ffffffff810065df>] ? __raw_callee_save_xen_restore_fl+0x11/0x1e
> (early) [<ffffffff812eebd5>] page_fault+0x25/0x30
> (early) [<ffffffff814a33f2>] ? find_range_array+0x4e/0x57
> (early) [<ffffffff814a33ca>] ? find_range_array+0x26/0x57
> (early) [<ffffffff814a343d>] __memblock_x86_memory_in_range+0x42/0x171
> (early) [<ffffffff814a3577>] memblock_x86_memory_in_range+0xb/0xd
> (early) [<ffffffff81497348>] memblock_find_dma_reserve+0x15/0x3b
> (early) [<ffffffff81496b50>] setup_arch+0x721/0x7e5
> (early) [<ffffffff810065fd>] ? __raw_callee_save_xen_irq_disable+0x11/0x1e
> (early) [<ffffffff81493955>] start_kernel+0x8a/0x2db
> (early) [<ffffffff81493299>] x86_64_start_reservations+0x84/0x88
> (early) [<ffffffff814947d9>] xen_start_kernel+0x3e1/0x3e8
>
> I think that (Stefano please confirm or not) this patch was prepared
> as workaround for similar issues. However, I do not like this patch
> because on systems with small amount of memory it leaves huge (to some
> extent) hole between max_low_pfn and 4G. Additionally, it affects
> memory hotplug a bit because it allocates memory starting from current
> max_mfn. It also breaks memory hotplug on i386 (maybe also others
> thinks, however, I could not confirm that). If it stay for some
> reason it should be amended in follwing way:
>
> #ifdef CONFIG_X86_32
> xen_extra_mem_start = mem_end;
> #else
> xen_extra_mem_start = max((1ULL << 32), mem_end);
> #endif
>
> Regarding comment for this patch it should be mentioned that without this
> patch e820_end_of_low_ram_pfn() is not broken. It is not called simply.
>
> Last but least. I found that memory sizes below and including exactly 1 GiB and
> exactly 2 GiB, 3 GiB (maybe higher, i.e. 4 GiB, 5 GiB, ...; I was not able to test
> them because I do not have sufficient memory) are magic. It means that if memory
> is set with those sizes everything is working good (without 4b239f458c229de044d6905c2b0f9fe16ed9e01e
> and 24bdb0b62cc82120924762ae6bc85afc8c3f2b26 applied). It means that domU
> should be tested with sizes which are not power of two nor multiple of that.

Hmm, I thought I did test 1500M.

> > +#ifdef CONFIG_X86_64
> > +static __initdata u64 __last_pgt_set_rw = 0;
> > +static __initdata u64 __pgt_buf_start = 0;
> > +static __initdata u64 __pgt_buf_end = 0;
> > +static __initdata u64 __pgt_buf_top = 0;
>
> Please look into include/linux/init.h for proper
> usage of __init macros. It should be changed to
>
> static u64 __last_pgt_set_rw __initdata = 0;
> ...
> ...
>
> Additionally,
>
> static const struct pv_mmu_ops xen_mmu_ops __initdata = {
>
> should be changed to:
>
> static const struct pv_mmu_ops xen_mmu_ops __initconst = {
>
> It is not in your patch, however, it conflicts
> with your definitions.

Ok. I am not that worried about the changes this patch brings. I hope
to have it removed in 2.6.40-ish time -frame.
>
> > +/*
> > + * As a consequence of the commit:
> > + *
> > + * commit 4b239f458c229de044d6905c2b0f9fe16ed9e01e
> > + * Author: Yinghai Lu <yinghai@kernel.org>
> > + * Date: Fri Dec 17 16:58:28 2010 -0800
> > + *
> > + * x86-64, mm: Put early page table high
> > + *
> > + * at some point init_memory_mapping is going to reach the pagetable pages
> > + * area and map those pages too (mapping them as normal memory that falls
> > + * in the range of addresses passed to init_memory_mapping as argument).
> > + * Some of those pages are already pagetable pages (they are in the range
> > + * pgt_buf_start-pgt_buf_end) therefore they are going to be mapped RO and
> > + * everything is fine.
> > + * Some of these pages are not pagetable pages yet (they fall in the range
> > + * pgt_buf_end-pgt_buf_top; for example the page at pgt_buf_end) so they
> > + * are going to be mapped RW. When these pages become pagetable pages and
> > + * are hooked into the pagetable, xen will find that the guest has already
> > + * a RW mapping of them somewhere and fail the operation.
> > + * The reason Xen requires pagetables to be RO is that the hypervisor needs
> > + * to verify that the pagetables are valid before using them. The validation
> > + * operations are called "pinning".
> > + *
> > + * In order to fix the issue we mark all the pages in the entire range
> > + * pgt_buf_start-pgt_buf_top as RO, however when the pagetable allocation
> > + * is completed only the range pgt_buf_start-pgt_buf_end is reserved by
> > + * init_memory_mapping. Hence the kernel is going to crash as soon as one
> > + * of the pages in the range pgt_buf_end-pgt_buf_top is reused (b/c those
> > + * ranges are RO).
> > + *
> > + * For this reason, 'mark_rw_past_pgt' is introduced which is called _after_
> > + * the init_memory_mapping has completed (in a perfect world we would
> > + * call this function from init_memory_mapping, but lets ignore that).
> > + *
> > + * Because we are called _after_ init_memory_mapping the pgt_buf_[start,
> > + * end,top] have all changed to new values (b/c init_memory_mapping
> > + * is called and setting up another new page-table). Hence, the first time
> > + * we enter this function, we save away the pgt_buf_start value and update
> > + * the pgt_buf_[end,top].
> > + *
> > + * When we detect that the "old" pgt_buf_start through pgt_buf_end
> > + * PFNs have been reserved (so memblock_x86_reserve_range has been called),
> > + * we immediately set out to RW the "old" pgt_buf_end through pgt_buf_top.
> > + *
> > + * And then we update those "old" pgt_buf_[end|top] with the new ones
> > + * so that we can redo this on the next pagetable.
> > + */
> > +static __init void mark_rw_past_pgt(void) {
>
> Please look into include/linux/init.h. I found much more similar
> mistakes in current Xen code. I will prepare relevant patch
> shortly.

Excellent. Looking forward to them.
..
> > + return;
>
> I think that this return is superfluous.

>nods>
>
> > +}
> > +#else
> > +static __init void mark_rw_past_pgt(void) { }
>
> Dito.

Ok. Not that much worried - I think we will get rid of this in 2.6.40 anyhow
(or I hope so).
>
> > +#endif
> > static void xen_pgd_free(struct mm_struct *mm, pgd_t *pgd)
> > {
> > #ifdef CONFIG_X86_64
> > @@ -1489,6 +1602,14 @@ static __init pte_t mask_rw_pte(pte_t *ptep, pte_t pte)
> > unsigned long pfn = pte_pfn(pte);
> >
> > /*
> > + * A bit of optimization. We do not need to call the workaround
> > + * when xen_set_pte_init is called with a PTE with 0 as PFN.
> > + * That is b/c the pagetable at that point are just being populated
> > + * with empty values and we can save some cycles by not calling
> > + * the 'memblock' code.*/
> > + if (pfn)
> > + mark_rw_past_pgt();
> > + /*
> > * If the new pfn is within the range of the newly allocated
> > * kernel pagetable, and it isn't being mapped into an
> > * early_ioremap fixmap slot as a freshly allocated page, make sure
> > @@ -1997,6 +2118,8 @@ __init void xen_ident_map_ISA(void)
> >
> > static __init void xen_post_allocator_init(void)
>
> Dito.

<nods> That looks like a candidate for another patch.
>
> Daniel


\
 
 \ /
  Last update: 2011-05-03 17:15    [from the cache]
©2003-2011 Jasper Spaans