public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: Nick Piggin <npiggin@suse.de>,
	Mark McLoughlin <markmc@redhat.com>,
	xen-devel <xen-devel@lists.xensource.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Vegard Nossum <vegard.nossum@gmail.com>,
	Stephen Tweedie <sct@redhat.com>,
	x86@kernel.org, LKML <linux-kernel@vger.kernel.org>,
	Yinghai Lu <yhlu.kernel@gmail.com>
Subject: Re: [Xen-devel] Re: [PATCH 00 of 36] x86/paravirt: groundwork for 64-bit Xen support
Date: Mon, 30 Jun 2008 16:04:48 -0700	[thread overview]
Message-ID: <48696690.90907@goop.org> (raw)
In-Reply-To: <20080630092209.GA29815@elte.hu>

Ingo Molnar wrote:
> -tip auto-testing found pagetable corruption (CPA self-test failure):
>
> [   32.956015] CPA self-test:
> [   32.958822]  4k 2048 large 508 gb 0 x 2556[ffff880000000000-ffff88003fe00000] miss 0
> [   32.964000] CPA ffff88001d54e000: bad pte 1d4000e3
> [   32.968000] CPA ffff88001d54e000: unexpected level 2
> [   32.972000] CPA ffff880022c5d000: bad pte 22c000e3
> [   32.976000] CPA ffff880022c5d000: unexpected level 2
> [   32.980000] CPA ffff8800200ce000: bad pte 200000e3
> [   32.984000] CPA ffff8800200ce000: unexpected level 2
> [   32.988000] CPA ffff8800210f0000: bad pte 210000e3
>
> config and full log can be found at:
>
>  http://redhat.com/~mingo/misc/config-Mon_Jun_30_11_11_51_CEST_2008.bad
>  http://redhat.com/~mingo/misc/log-Mon_Jun_30_11_11_51_CEST_2008.bad
>
> i've pushed that tree out into tip/tmp.xen-64bit.Mon_Jun_30_11_11. The 
> only new item in that tree over a well-tested base is x86/xen-64bit, so 
> i've taken it out again.
>   

Phew.  OK, I've worked this out.  Short version is that's it's a false 
alarm, and there was no real failure here.  Long version:

    * I changed the code to create the physical mapping pagetables to
      reuse any existing mapping rather than replace it.   Specifically,
      reusing an pud pointed to by the pgd caused this symptom to appear.
    * The specific PUD being reused is the one created statically in
      head_64.S, which creates an initial 1GB mapping.
    * That mapping doesn't have _PAGE_GLOBAL set on it, due to the
      inconsistency between __PAGE_* and PAGE_*.
    * The CPA test attempts to clear _PAGE_GLOBAL, and then checks to
      see that the resulting range is 1) shattered into 4k pages, and 2)
      has no _PAGE_GLOBAL.
    * However, since it didn't have _PAGE_GLOBAL on that range to start
      with, change_page_attr_clear() had nothing to do, and didn't
      bother shattering the range,
    * resulting in the reported messages

The simple fix is to set _PAGE_GLOBAL in level2_ident_pgt.

An additional fix to make CPA testing more robust by using some other 
pagetable bit (one of the unused available-to-software ones).  This 
would solve spurious CPA test warnings under Xen which uses _PAGE_GLOBAL 
for its own purposes (ie, not under guest control).

Also, we should revisit the use of _PAGE_GLOBAL in asm-x86/pgtable.h, 
and use it consistently, and drop MAKE_GLOBAL.  The first time I 
proposed it it caused breakages in the very early CPA code; with luck 
that's all fixed now.

Anyway, the simple fix below.  I'll put together RFC patches for the 
other suggestions.  I also split the originating patch into tiny, tiny 
bisectable pieces.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

---
 arch/x86/kernel/head_64.S |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

===================================================================
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -374,7 +374,7 @@
 	/* Since I easily can, map the first 1G.
 	 * Don't set NX because code runs from these pages.
 	 */
-	PMDS(0, __PAGE_KERNEL_LARGE_EXEC, PTRS_PER_PMD)
+	PMDS(0, __PAGE_KERNEL_LARGE_EXEC | _PAGE_GLOBAL, PTRS_PER_PMD)
 
 NEXT_PAGE(level2_kernel_pgt)
 	/*



  parent reply	other threads:[~2008-06-30 23:05 UTC|newest]

Thread overview: 96+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-25  4:18 [PATCH 00 of 36] x86/paravirt: groundwork for 64-bit Xen support Jeremy Fitzhardinge
2008-06-25  4:18 ` [PATCH 01 of 36] x86: asm-x86/pgtable.h: fix compiler warning Jeremy Fitzhardinge
2008-06-25  4:18 ` [PATCH 02 of 36] x86: add memory clobber to save/loadsegment Jeremy Fitzhardinge
2008-06-25  4:18 ` [PATCH 03 of 36] x86: add memory barriers to wrmsr Jeremy Fitzhardinge
2008-06-25  4:44   ` Arjan van de Ven
2008-06-25 21:08     ` Jeremy Fitzhardinge
2008-06-25 22:31       ` Arjan van de Ven
2008-06-25 23:05         ` Jeremy Fitzhardinge
2008-06-25 23:18         ` H. Peter Anvin
2008-06-25 23:37           ` Jeremy Fitzhardinge
2008-06-25 23:42             ` H. Peter Anvin
2008-06-25  4:19 ` [PATCH 04 of 36] x86: remove open-coded save/load segment operations Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 05 of 36] x86_64: use write_gdt_entry in vsyscall_set_cpu Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 06 of 36] x86_64: use p??_populate() to attach pages to pagetable Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 07 of 36] x86_64: unify early_ioremap Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 08 of 36] x86_64: Add gate_offset() and gate_segment() macros Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 09 of 36] x86_64: Use __pgd() on mk_kernel_pgd() Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 10 of 36] x86: unify pgd_index Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 11 of 36] x86: unify mmu_context.h Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 12 of 36] x86_64: replace end_pfn with num_physpages Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 13 of 36] x86_64: add prototype for x86_64_start_kernel() Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 14 of 36] x86_64: add sync_cmpxchg Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 15 of 36] x86: simplify vmalloc_sync_all Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 16 of 36] x86/paravirt: add a pgd_alloc/free hooks Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 17 of 36] x86: preallocate and prepopulate separately Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 18 of 36] x86/paravirt: add debugging for missing operations Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 19 of 36] paravirt_ops: define PARA_INDIRECT for indirect asm calls Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 20 of 36] paravirt/x86_64: move __PAGE_OFFSET to leave a space for hypervisor Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 21 of 36] x86-64: add FIX_PARAVIRT_BOOTMAP fixmap slot Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 22 of 36] x86_64: split x86_64_start_kernel Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 23 of 36] x86_64: adjust mapping of physical pagetables to work with Xen Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 24 of 36] x86_64: create small vmemmap mappings if PSE not available Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 25 of 36] x86_64: PSE no longer a hard requirement Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 26 of 36] x86_64: Split set_pte_vaddr() Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 27 of 36] x86_64: __switch_to(): Move arch_leave_lazy_cpu_mode() to the right place Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 28 of 36] Save %fs and %gs before load_TLS() and arch_leave_lazy_cpu_mode() Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 29 of 36] Use __KERNEL_DS as SS when returning to a kernel thread (VERIFY) Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 30 of 36] x86/paravirt_ops: split sysret and sysexit Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 31 of 36] x86_64 pvops: don't restore user rsp within sysret Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 32 of 36] Add sysret/sysexit pvops for returning to 32-bit compatibility userspace Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 33 of 36] x86_64: ia32entry: replace privileged instructions with pvops Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 34 of 36] x86_64: swapgs pvop with a user-stack can never be called Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 35 of 36] x86_64/paravirt: add adjust_exception_frame Jeremy Fitzhardinge
2008-06-25  4:19 ` [PATCH 36 of 36] x86_64/paravirt: Make load_gs_index() a paravirt operation Jeremy Fitzhardinge
2008-06-25  8:47   ` Ingo Molnar
2008-06-25 11:48     ` Jeremy Fitzhardinge
2008-06-25  8:42 ` [PATCH 00 of 36] x86/paravirt: groundwork for 64-bit Xen support Ingo Molnar
2008-06-25 11:46   ` Jeremy Fitzhardinge
2008-06-25 15:22   ` Ingo Molnar
2008-06-25 20:12     ` Jeremy Fitzhardinge
2008-06-26 10:57       ` Ingo Molnar
2008-06-26 10:58         ` Ingo Molnar
2008-06-26 14:34           ` [Xen-devel] " Jeremy Fitzhardinge
2008-06-27 15:56             ` Ingo Molnar
2008-06-27 16:02               ` Jeremy Fitzhardinge
2008-06-27 16:06                 ` Ingo Molnar
2008-06-27 16:25                   ` Jeremy Fitzhardinge
2008-06-27 16:03             ` Ingo Molnar
2008-06-27 19:04               ` Jeremy Fitzhardinge
2008-06-29  8:43                 ` Ingo Molnar
2008-06-30  3:02                   ` Jeremy Fitzhardinge
2008-06-30  4:35                     ` Yinghai Lu
2008-06-30  5:32                       ` Jeremy Fitzhardinge
2008-06-30  8:21                     ` Ingo Molnar
2008-06-30  9:22                       ` Ingo Molnar
2008-06-30 17:17                         ` Jeremy Fitzhardinge
2008-06-30 18:12                           ` Ingo Molnar
2008-06-30 18:36                             ` Jeremy Fitzhardinge
2008-06-30 18:44                               ` Ingo Molnar
2008-06-30 17:57                         ` Jeremy Fitzhardinge
2008-06-30 18:03                           ` Ingo Molnar
2008-06-30 23:04                         ` Jeremy Fitzhardinge [this message]
2008-07-01  8:52                           ` Ingo Molnar
2008-07-01  9:21                             ` Ingo Molnar
2008-07-01 16:10                               ` Jeremy Fitzhardinge
2008-07-01 16:14                               ` Jeremy Fitzhardinge
2008-07-01 20:31                                 ` Ingo Molnar
2008-07-03  9:10                                   ` Ingo Molnar
2008-07-03 15:47                                     ` Jeremy Fitzhardinge
2008-07-03 18:20                                     ` Yinghai Lu
2008-07-03 18:25                                       ` Jeremy Fitzhardinge
2008-07-03 18:30                                         ` Yinghai Lu
2008-07-03 18:41                                           ` Jeremy Fitzhardinge
2008-07-03 18:51                                             ` Yinghai Lu
2008-07-03 19:19                                               ` Yinghai Lu
2008-07-03 19:29                                                 ` Yinghai Lu
2008-07-09  7:42                                                   ` Ingo Molnar
2008-06-26 14:28         ` Jeremy Fitzhardinge
2008-06-26 18:25         ` Jeremy Fitzhardinge
2008-06-26 19:02         ` Jeremy Fitzhardinge
2008-06-25 12:40 ` Andi Kleen
2008-06-25 18:45   ` [Xen-devel] " Keir Fraser
2008-06-25 19:13     ` Andi Kleen
2008-06-25 19:22       ` Keir Fraser
2008-06-25 20:14         ` Andi Kleen
2008-06-25 20:03   ` Jeremy Fitzhardinge

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=48696690.90907@goop.org \
    --to=jeremy@goop.org \
    --cc=ehabkost@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markmc@redhat.com \
    --cc=mingo@elte.hu \
    --cc=npiggin@suse.de \
    --cc=sct@redhat.com \
    --cc=vegard.nossum@gmail.com \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xensource.com \
    --cc=yhlu.kernel@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox