thr3ads.net - Xen devel - [Xen-devel] [GIT PULL] Xen core updates [May 2009]

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

Jeremy Fitzhardinge

2009-May-07 20:24 UTC

[Xen-devel] [GIT PULL] Xen core updates

Hi Ingo,

Here''s a set of core Xen updates, which is a superset of the bugfix
patches I posted earlier.

The additional patches are:

xen/i386: make sure initial VGA/ISA mappings are not overridden

  Make sure that ISA identity mappings are not overridden by _PAGE_IOMAP
  mappings.

x86: use flush_tlb_others to implement flush_tlb_all

  Paravirtualize flush_tlb_all by implementing it in terms of
  flush_tlb_others.  This avoids some unnecessary IPIs when flushing
  kernel mappings.

xen: cache cr0 value to avoid trap''n''emulate for read_cr0

  read_cr0 is used every context switch to set TS, but Xen has been
  implementing it with a trap''n''emulate of a native "mov
cr0".  This
  adds a Xen-specific implementation of the read_cr0 pvop to avoid
  the extra trap.


The following changes since commit e7c064889606aab3569669078c69b87b2c527e72:
  Jeremy Fitzhardinge (1):
        xen: add FIX_TEXT_POKE to fixmap

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git xen-tip/core

Jeremy Fitzhardinge (8):
      xen/x86-64: fix breakpoints and hardware watchpoints
      xen/x86-64: clean up warnings about IST-using traps
      xen: cache cr0 value to avoid trap''n''emulate for
read_cr0
      xen: deal with NMI''s use of IST too
      xen/i386: make sure initial VGA/ISA mappings are not overridden
      x86: use flush_tlb_others to implement flush_tlb_all
      xen/i386: reserve Xen pagetables
      xen: reserve Xen start_info rather than e820 reserving

 arch/x86/include/asm/traps.h |    2 +
 arch/x86/kernel/entry_64.S   |    4 +++
 arch/x86/mm/tlb.c            |   29 ++++++++++++++--------
 arch/x86/xen/enlighten.c     |   54 ++++++++++++++++++++++++++++++++++++++++-
 arch/x86/xen/mmu.c           |   23 ++++++++++++++++-
 arch/x86/xen/setup.c         |    6 ++--
 6 files changed, 100 insertions(+), 18 deletions(-)

Thanks,
	J

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

Jeremy Fitzhardinge

2009-May-07 20:24 UTC

head link

[Xen-devel] [PATCH 1/8] xen/x86-64: fix breakpoints and hardware watchpoints

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Native x86-64 uses the IST mechanism to run int3 and debug traps on
an alternative stack.  Xen does not do this, and so the frames were
being misinterpreted by the ptrace code.  This change special-cases
these two exceptions by using Xen variants which run on the normal
kernel stack properly.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/include/asm/traps.h |    2 ++
 arch/x86/kernel/entry_64.S   |    4 ++++
 arch/x86/xen/enlighten.c     |   17 ++++++++++++++++-
 3 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index 0d53425..d696185 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -13,6 +13,8 @@ asmlinkage void divide_error(void);
 asmlinkage void debug(void);
 asmlinkage void nmi(void);
 asmlinkage void int3(void);
+asmlinkage void xen_debug(void);
+asmlinkage void xen_int3(void);
 asmlinkage void overflow(void);
 asmlinkage void bounds(void);
 asmlinkage void invalid_op(void);
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index a331ec3..649db62 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -1378,6 +1378,10 @@ END(xen_failsafe_callback)
 
 paranoidzeroentry_ist debug do_debug DEBUG_STACK
 paranoidzeroentry_ist int3 do_int3 DEBUG_STACK
+#ifdef CONFIG_XEN
+zeroentry xen_debug do_debug
+zeroentry xen_int3 do_int3
+#endif
 paranoiderrorentry stack_segment do_stack_segment
 errorentry general_protection do_general_protection
 errorentry page_fault do_page_fault
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 12a3159..16afbc3 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -20,6 +20,7 @@
 #include <linux/delay.h>
 #include <linux/start_kernel.h>
 #include <linux/sched.h>
+#include <linux/kprobes.h>
 #include <linux/bootmem.h>
 #include <linux/module.h>
 #include <linux/mm.h>
@@ -44,6 +45,7 @@
 #include <asm/processor.h>
 #include <asm/proto.h>
 #include <asm/msr-index.h>
+#include <asm/traps.h>
 #include <asm/setup.h>
 #include <asm/desc.h>
 #include <asm/pgtable.h>
@@ -428,11 +430,24 @@ static void xen_write_ldt_entry(struct desc_struct *dt,
int entrynum,
 static int cvt_gate_to_trap(int vector, const gate_desc *val,
 			    struct trap_info *info)
 {
+	unsigned long addr;
+
 	if (val->type != GATE_TRAP && val->type != GATE_INTERRUPT)
 		return 0;
 
 	info->vector = vector;
-	info->address = gate_offset(*val);
+
+	addr = gate_offset(*val);
+#ifdef CONFIG_X86_64
+	if (addr == (unsigned long)debug)
+		addr = (unsigned long)xen_debug;
+	else if (addr == (unsigned long)int3)
+		addr = (unsigned long)xen_int3;
+	else
+		WARN_ON(val->ist != 0);
+#endif	/* CONFIG_X86_64 */
+	info->address = addr;
+
 	info->cs = gate_segment(*val);
 	info->flags = val->dpl;
 	/* interrupt gates clear IF */
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-07 20:24 UTC

head link

[Xen-devel] [PATCH 2/8] xen/x86-64: clean up warnings about IST-using traps

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Ignore known IST-using traps.  Aside from the debugger traps, they''re
low-level faults which Xen will handle for us, so the kernel needn''t
worry about them.  Keep warning in case unknown trap starts using IST.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/enlighten.c |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 16afbc3..cbb31eb 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -439,12 +439,30 @@ static int cvt_gate_to_trap(int vector, const gate_desc
*val,
 
 	addr = gate_offset(*val);
 #ifdef CONFIG_X86_64
+	/*
+	 * Look for known traps using IST, and substitute them
+	 * appropriately.  The debugger ones are the only ones we care
+	 * about.  Xen will handle faults like double_fault and
+	 * machine_check, so we should never see them.  Warn if
+	 * there''s an unexpected IST-using fault handler.
+	 */
 	if (addr == (unsigned long)debug)
 		addr = (unsigned long)xen_debug;
 	else if (addr == (unsigned long)int3)
 		addr = (unsigned long)xen_int3;
-	else
-		WARN_ON(val->ist != 0);
+	else if (addr == (unsigned long)double_fault ||
+		 addr == (unsigned long)stack_segment) {
+		/* Don''t need to handle these */
+		return 0;
+#ifdef CONFIG_X86_MCE
+	} else if (addr == (unsigned long)machine_check) {
+		return 0;
+#endif
+	} else {
+		/* Some other trap using IST? */
+		if (WARN_ON(val->ist != 0))
+			return 0;
+	}
 #endif	/* CONFIG_X86_64 */
 	info->address = addr;
 
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-07 20:24 UTC

head link

[Xen-devel] [PATCH 3/8] xen: cache cr0 value to avoid trap''n''emulate for read_cr0

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

stts() is implemented in terms of read_cr0/write_cr0 to update the
state of the TS bit.  This happens during context switch, and so
is fairly performance critical.  Rather than falling back to
a trap-and-emulate native read_cr0, implement our own by caching
the last-written value from write_cr0 (the TS bit is the only one
we really care about).

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/enlighten.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index cbb31eb..9c6de48 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -656,10 +656,26 @@ static void xen_clts(void)
 	xen_mc_issue(PARAVIRT_LAZY_CPU);
 }
 
+static DEFINE_PER_CPU(unsigned long, xen_cr0_value);
+
+static unsigned long xen_read_cr0(void)
+{
+	unsigned long cr0 = percpu_read(xen_cr0_value);
+
+	if (unlikely(cr0 == 0)) {
+		cr0 = native_read_cr0();
+		percpu_write(xen_cr0_value, cr0);
+	}
+
+	return cr0;
+}
+
 static void xen_write_cr0(unsigned long cr0)
 {
 	struct multicall_space mcs;
 
+	percpu_write(xen_cr0_value, cr0);
+
 	/* Only pay attention to cr0.TS; everything else is
 	   ignored. */
 	mcs = xen_mc_entry(0);
@@ -845,7 +861,7 @@ static const struct pv_cpu_ops xen_cpu_ops __initdata = {
 
 	.clts = xen_clts,
 
-	.read_cr0 = native_read_cr0,
+	.read_cr0 = xen_read_cr0,
 	.write_cr0 = xen_write_cr0,
 
 	.read_cr4 = native_read_cr4,
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-07 20:24 UTC

head link

[Xen-devel] [PATCH 4/8] xen: deal with NMI''s use of IST too

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

NMI uses the IST, and Xen will also deal with it.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/enlighten.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 9c6de48..852db47 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -451,7 +451,8 @@ static int cvt_gate_to_trap(int vector, const gate_desc
*val,
 	else if (addr == (unsigned long)int3)
 		addr = (unsigned long)xen_int3;
 	else if (addr == (unsigned long)double_fault ||
-		 addr == (unsigned long)stack_segment) {
+		 addr == (unsigned long)stack_segment ||
+		 addr == (unsigned long)nmi) {
 		/* Don''t need to handle these */
 		return 0;
 #ifdef CONFIG_X86_MCE
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-07 20:24 UTC

head link

[Xen-devel] [PATCH 5/8] xen/i386: make sure initial VGA/ISA mappings are not overridden

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

arch/x86/mm/init_32.c overrides the ISA/VGA mappings with direct mappings
which do not have _PAGE_IOMAP set, thereby making the ISA space inaccessible.

This patch adds to the existing hack to make sure the pre-constructed
ISA mappings are not incorrectly overwritten.

This makes 32-bit dom0 VGA work properly.

Thanks to Gerd Hoffman for pointing this out.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/mmu.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index a96f5b9..5af62d8 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1443,10 +1443,17 @@ static void *xen_kmap_atomic_pte(struct page *page, enum
km_type type)
 #ifdef CONFIG_X86_32
 static __init pte_t mask_rw_pte(pte_t *ptep, pte_t pte)
 {
-	/* If there''s an existing pte, then don''t allow _PAGE_RW to
be set */
-	if (pte_val_ma(*ptep) & _PAGE_PRESENT)
+	pte_t oldpte = *ptep;
+
+	if (pte_flags(oldpte) & _PAGE_PRESENT) {
+		/* Don''t allow existing IO mappings to be overridden */
+		if (pte_flags(oldpte) & _PAGE_IOMAP)
+			pte = oldpte;
+
+		/* Don''t allow _PAGE_RW to be set on existing pte */
 		pte = __pte_ma(((pte_val_ma(*ptep) & _PAGE_RW) | ~_PAGE_RW) &
 			       pte_val_ma(pte));
+	}
 
 	return pte;
 }
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-07 20:25 UTC

head link

[Xen-devel] [PATCH 6/8] x86: use flush_tlb_others to implement flush_tlb_all

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Use the flush_tlb_others() call to implement flush_tlb_all().  This is
useful because flush_tlb_others() already goes via paravirt_ops, and so
will be properly paravirtualized.  This needs a small extension of the
extension to the existing native_flush_tlb_others: the global flush is
indicated by setting the "mm" parameter to NULL, so that kernel
mappings
are also flushed.

(Nothing similar is required for xen_flush_tlb_others, as we don''t use
global mappings in a guest-visible way under Xen.)

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/mm/tlb.c  |   29 ++++++++++++++++++-----------
 arch/x86/xen/mmu.c |    7 +++++++
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 821e970..e69bdad 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -147,13 +147,25 @@ void smp_invalidate_interrupt(struct pt_regs *regs)
 		 * BUG();
 		 */
 
-	if (f->flush_mm == percpu_read(cpu_tlbstate.active_mm)) {
-		if (percpu_read(cpu_tlbstate.state) == TLBSTATE_OK) {
+	if (f->flush_mm == NULL ||
+	    f->flush_mm == percpu_read(cpu_tlbstate.active_mm)) {
+		int tlbstate = percpu_read(cpu_tlbstate.state);
+
+		/* 
+		 * flush_mm == NULL means flush everything, including
+		 * global tlbs, which will only happen when flushing
+		 * kernel mappings. 
+		 */
+		if (f->flush_mm == NULL)
+			__flush_tlb_all();
+		else if (tlbstate == TLBSTATE_OK) {
 			if (f->flush_va == TLB_FLUSH_ALL)
 				local_flush_tlb();
 			else
 				__flush_tlb_one(f->flush_va);
-		} else
+		}
+
+		if (tlbstate == TLBSTATE_LAZY)
 			leave_mm(cpu);
 	}
 out:
@@ -275,16 +287,11 @@ void flush_tlb_page(struct vm_area_struct *vma, unsigned
long va)
 	preempt_enable();
 }
 
-static void do_flush_tlb_all(void *info)
+void flush_tlb_all(void)
 {
-	unsigned long cpu = smp_processor_id();
+	flush_tlb_others(cpu_online_mask, NULL, TLB_FLUSH_ALL);
 
 	__flush_tlb_all();
 	if (percpu_read(cpu_tlbstate.state) == TLBSTATE_LAZY)
-		leave_mm(cpu);
-}
-
-void flush_tlb_all(void)
-{
-	on_each_cpu(do_flush_tlb_all, NULL, 1);
+		leave_mm(smp_processor_id());
 }
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 5af62d8..0e13477 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1284,6 +1284,13 @@ static void xen_flush_tlb_single(unsigned long addr)
 	preempt_enable();
 }
 
+/* 
+ * Flush tlb on other cpus.  Xen can do this via a single hypercall
+ * rather than explicit IPIs, which has the nice property of avoiding
+ * any cpus which don''t actually have dirty tlbs.  Unfortunately it
+ * doesn''t give us an opportunity to kick out cpus which are in lazy
+ * tlb state, so we may end up reflushing some cpus unnecessarily.
+ */
 static void xen_flush_tlb_others(const struct cpumask *cpus,
 				 struct mm_struct *mm, unsigned long va)
 {
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-07 20:25 UTC

head link

[Xen-devel] [PATCH 7/8] xen/i386: reserve Xen pagetables

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

The Xen pagetables are no longer implicitly reserved as part of the other
i386_start_kernel reservations, so make sure we explicitly reserve them.
This prevents them from being released into the general kernel free page
pool and reused.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/mmu.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 0e13477..801d042 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1799,6 +1799,11 @@ __init pgd_t *xen_setup_kernel_pagetable(pgd_t *pgd,
 
 	pin_pagetable_pfn(MMUEXT_PIN_L3_TABLE, PFN_DOWN(__pa(swapper_pg_dir)));
 
+	reserve_early(__pa(xen_start_info->pt_base),
+		      __pa(xen_start_info->pt_base +
+			   xen_start_info->nr_pt_frames * PAGE_SIZE),
+		      "XEN PAGETABLES");
+
 	return swapper_pg_dir;
 }
 #endif	/* CONFIG_X86_64 */
-- 
1.6.0.6


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

Jeremy Fitzhardinge

2009-May-07 20:25 UTC

head link

[Xen-devel] [PATCH 8/8] xen: reserve Xen start_info rather than e820 reserving

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Use reserve_early rather than e820 reservations for Xen start info and
mfn->pfn
table, so that the memory use is a bit more self-documenting.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
---
 arch/x86/xen/setup.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 15c6c68..ad0047f 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -61,9 +61,9 @@ char * __init xen_memory_setup(void)
 	 *  - xen_start_info
 	 * See comment above "struct start_info" in
<xen/interface/xen.h>
 	 */
-	e820_add_region(__pa(xen_start_info->mfn_list),
-			xen_start_info->pt_base - xen_start_info->mfn_list,
-			E820_RESERVED);
+	reserve_early(__pa(xen_start_info->mfn_list),
+		      __pa(xen_start_info->pt_base),
+			"XEN START INFO");
 
 	sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
 
-- 
1.6.0.6


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

Ingo Molnar

2009-May-08 10:57 UTC

head link

[Xen-devel] Re: [PATCH 5/8] xen/i386: make sure initial VGA/ISA mappings are not overridden

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> 
> arch/x86/mm/init_32.c overrides the ISA/VGA mappings with direct mappings
> which do not have _PAGE_IOMAP set, thereby making the ISA space
inaccessible.
> 
> This patch adds to the existing hack to make sure the 
> pre-constructed ISA mappings are not incorrectly overwritten.
> 
> This makes 32-bit dom0 VGA work properly.
I''m wondering, should we add this fix to .30 as well, is there any 
relevancy beyond dom0?
> Thanks to Gerd Hoffman for pointing this out.
Please add Reported-by lines in such cases. That way not only does 
the reporting get reported prominently, but the tip-bot will Cc: 
Gerd too on the commit notification too. (which will sometimes spur 
further comments - while a commit hidden somewhere might not)
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Please also add impact-footers to all commits you queue up, if you 
want me to pull your tree directly. I can only guess at the impact 
of this one, is it:

[ Impact: fix dom0-Xen-guest boot crash ]

or:

[ Impact: fix dom0-Xen-guest non-working VGA console ]

?

(Please look at latest tip/master for examples about various impact 
lines, their precise format, and what we try to describe in them and 
how.)

Thanks,

	Ingo

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

Jeremy Fitzhardinge

2009-May-08 15:08 UTC

head link

[Xen-devel] Re: [PATCH 5/8] xen/i386: make sure initial VGA/ISA mappings are not overridden

Ingo Molnar wrote:> * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>
>   
>> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>>
>> arch/x86/mm/init_32.c overrides the ISA/VGA mappings with direct
mappings
>> which do not have _PAGE_IOMAP set, thereby making the ISA space
inaccessible.
>>
>> This patch adds to the existing hack to make sure the 
>> pre-constructed ISA mappings are not incorrectly overwritten.
>>
>> This makes 32-bit dom0 VGA work properly.
>>     
>
> I''m wondering, should we add this fix to .30 as well, is there any
> relevancy beyond dom0?
>   
No, it only matters if the ISA region is mapping real hardware.  For 
domU we set up a set of dummy maps for ISA to mop up any stray 
references, but they have no connection to underlying hardware.

This area is all a bit ugly, and I hope to ultimately address it by 
making 32 and 64-bit use the same code for setting up the kernel 
mappings (as the 64-bit code doesn''t try to double-map the ISA area).
>> Thanks to Gerd Hoffman for pointing this out.
>>     
>
> Please add Reported-by lines in such cases. That way not only does 
> the reporting get reported prominently, but the tip-bot will Cc: 
> Gerd too on the commit notification too. (which will sometimes spur 
> further comments - while a commit hidden somewhere might not)
>   
OK.  I guess Diagnosed-by: would be the most appropriate tag in this case.
>> Signed-off-by: Jeremy Fitzhardinge
<jeremy.fitzhardinge@citrix.com>
>>     
>
> Please also add impact-footers to all commits you queue up, if you 
> want me to pull your tree directly. I can only guess at the impact 
> of this one, is it:
>
> [ Impact: fix dom0-Xen-guest boot crash ]
>
> or:
>
> [ Impact: fix dom0-Xen-guest non-working VGA console ]
>
> ?
>
> (Please look at latest tip/master for examples about various impact 
> lines, their precise format, and what we try to describe in them and 
> how.)
>   
OK, they''re footers now?

Do you want me to respin these patches and repost?

    J

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

Ian Campbell

2009-May-08 15:24 UTC

head link

[Xen-devel] Re: [PATCH 6/8] x86: use flush_tlb_others to implement flush_tlb_all

On Thu, 2009-05-07 at 13:25 -0700, Jeremy Fitzhardinge
wrote:> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> 
> Use the flush_tlb_others() call to implement flush_tlb_all().
This causes:
        BUG: spinlock bad magic on CPU#0, swapper/0
         lock: c05b7588, .magic: 00000000, .owner: <none>/-1, .owner_cpu:
0
        Pid: 0, comm: swapper Not tainted
2.6.30-rc3-x86_32p-xen0-tip-01797-g3db7847 #1054
        Call Trace:
         [<c023e656>] spin_bug+0x96/0xf0
         [<c023e90f>] _raw_spin_lock+0x6f/0x150
         [<c0158baf>] ? lock_acquire+0x7f/0x90
         [<c03e89a9>] _spin_lock+0x39/0x40
         [<c0127bdf>] ? flush_tlb_others_ipi+0x3f/0xc0
         [<c0127bdf>] flush_tlb_others_ipi+0x3f/0xc0
         [<c0127c68>] native_flush_tlb_others+0x8/0x10
         [<c0127b4f>] flush_tlb_all+0x1f/0x70
         [<c0121b1c>] zap_low_mappings+0x7c/0x90
         [<c055caa5>] mem_init+0x2f5/0x330
         [<c054a000>] ? __init_begin+0x0/0x20
         [<c03e9bc2>] ? _etext+0x0/0x2
         [<c03e9bc2>] ? _etext+0x0/0x2
         [<c054a928>] start_kernel+0x1e8/0x300
         [<c054a2c0>] ? unknown_bootoption+0x0/0x210
         [<c054a076>] i386_start_kernel+0x56/0xa0
Because zap_low_mappings happens before the init_smp_flush
core_initcall. Also I don''t think init_smp_flush has needed to be
__cpuinit since 09b3ec73, or even before then since it doesn''t seem to
be called in any hotplug paths.

--- 
Subject: initialise tlbflush locks before any callers

Now that flush_tlb_all() is implemented via flush_tlb_others() the later
can be called before the core_initcalls are run. Make it an
early_initcall so it happens earlier.

Also there is no need for init_smp_flush to be __cpuinit.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

[ Impact: initialise spinlock before use ]

diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index e69bdad..c6af8aa 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -229,7 +229,7 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
 	flush_tlb_others_ipi(cpumask, mm, va);
 }
 
-static int __cpuinit init_smp_flush(void)
+static int __init init_smp_flush(void)
 {
 	int i;
 
@@ -238,7 +238,7 @@ static int __cpuinit init_smp_flush(void)
 
 	return 0;
 }
-core_initcall(init_smp_flush);
+early_initcall(init_smp_flush);
 
 void flush_tlb_current_task(void)
 {



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

Jeremy Fitzhardinge

2009-May-08 15:49 UTC

head link

[Xen-devel] Re: [PATCH 6/8] x86: use flush_tlb_others to implement flush_tlb_all

Ian Campbell wrote:> On Thu, 2009-05-07 at 13:25 -0700, Jeremy Fitzhardinge wrote:
>   
>> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
>>
>> Use the flush_tlb_others() call to implement flush_tlb_all().
>>     
>
> This causes:
>         BUG: spinlock bad magic on CPU#0, swapper/0
>          lock: c05b7588, .magic: 00000000, .owner: <none>/-1,
.owner_cpu: 0
>         Pid: 0, comm: swapper Not tainted
2.6.30-rc3-x86_32p-xen0-tip-01797-g3db7847 #1054
>         Call Trace:
>          [<c023e656>] spin_bug+0x96/0xf0
>          [<c023e90f>] _raw_spin_lock+0x6f/0x150
>   
Ah, right.  I had another change (somewhere around here) to explicitly 
initialize the tlb flush code from the normal early init code, rather 
than make it implicit via initcall.  Otherwise we have an unknown point 
at which we''re allowed to start using global flushes, which is a bit
flakey.

    J

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

Ian Campbell

2009-May-08 15:52 UTC

head link

[Xen-devel] Re: [PATCH 6/8] x86: use flush_tlb_others to implement flush_tlb_all

On Fri, 2009-05-08 at 11:49 -0400, Jeremy Fitzhardinge
wrote:> Ian Campbell wrote:
> > On Thu, 2009-05-07 at 13:25 -0700, Jeremy Fitzhardinge wrote:
> >   
> >> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> >>
> >> Use the flush_tlb_others() call to implement flush_tlb_all().
> >>     
> >
> > This causes:
> >         BUG: spinlock bad magic on CPU#0, swapper/0
> >          lock: c05b7588, .magic: 00000000, .owner: <none>/-1,
.owner_cpu: 0
> >         Pid: 0, comm: swapper Not tainted
2.6.30-rc3-x86_32p-xen0-tip-01797-g3db7847 #1054
> >         Call Trace:
> >          [<c023e656>] spin_bug+0x96/0xf0
> >          [<c023e90f>] _raw_spin_lock+0x6f/0x150
> >   
> 
> Ah, right.  I had another change (somewhere around here) to explicitly 
> initialize the tlb flush code from the normal early init code, rather 
> than make it implicit via initcall.  Otherwise we have an unknown point 
> at which we''re allowed to start using global flushes, which is a
bit flakey.
That was my v1, which I still happen to have lying around:

---

Subject: initialise tlbflush locks before any callers

Now that flush_tlb_all() is implemented via flush_tlb_others() the later
can be called before the core_initcalls are run. Explicitly call
init_smp_flush early on.

Also there is no need for init_smp_flush to be __cpuinit.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

[ Impact: initialise spinlock before use ]

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 16a5c84..2a4848d 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -129,6 +129,8 @@ static inline void reset_lazy_tlbstate(void)
 
 #define local_flush_tlb() __flush_tlb()
 
+extern int init_smp_flush(void);
+
 extern void flush_tlb_all(void);
 extern void flush_tlb_current_task(void);
 extern void flush_tlb_mm(struct mm_struct *);
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index e69bdad..0f030fb 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -229,7 +229,7 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
 	flush_tlb_others_ipi(cpumask, mm, va);
 }
 
-static int __cpuinit init_smp_flush(void)
+int __init init_smp_flush(void)
 {
 	int i;
 
@@ -238,7 +238,6 @@ static int __cpuinit init_smp_flush(void)
 
 	return 0;
 }
-core_initcall(init_smp_flush);
 
 void flush_tlb_current_task(void)
 {
diff --git a/init/main.c b/init/main.c
index 33ce929..f798501 100644
--- a/init/main.c
+++ b/init/main.c
@@ -651,6 +651,7 @@ asmlinkage void __init start_kernel(void)
 	vfs_caches_init_early();
 	cpuset_init_early();
 	page_cgroup_init();
+	init_smp_flush();
 	mem_init();
 	enable_debug_pagealloc();
 	cpu_hotplug_init();
> 
>     J

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

tip-bot for Ian Campbell

2009-May-11 13:09 UTC

head link

[Xen-devel] [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

Commit-ID:  636afdbe85d50d9b88f5283b5e6d0f8518431aac
Gitweb:     http://git.kernel.org/tip/636afdbe85d50d9b88f5283b5e6d0f8518431aac
Author:     Ian Campbell <Ian.Campbell@eu.citrix.com>
AuthorDate: Fri, 8 May 2009 16:52:07 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 11 May 2009 15:08:08 +0200

x86: use flush_tlb_others to implement flush_tlb_all, fix

"use the flush_tlb_others() call to implement flush_tlb_all()"
causes:
         BUG: spinlock bad magic on CPU#0, swapper/0
          lock: c05b7588, .magic: 00000000, .owner: <none>/-1, .owner_cpu:
0
         Pid: 0, comm: swapper Not tainted
2.6.30-rc3-x86_32p-xen0-tip-01797-g3db7847 #1054
         Call Trace:
          [<c023e656>] spin_bug+0x96/0xf0
          [<c023e90f>] _raw_spin_lock+0x6f/0x150

It can be called before the core_initcalls are run. Explicitly call
init_smp_flush early on.

Also there is no need for init_smp_flush to be __cpuinit.

[ Impact: fix boot crash/warning ]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Xen-devel <xen-devel@lists.xensource.com>
Acked-by: Jeremy Fitzhardinge <Jeremy.Fitzhardinge@citrix.com>
LKML-Reference: <1241797927.15972.51.camel@zakaz.uk.xensource.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/include/asm/tlbflush.h |    2 ++
 arch/x86/mm/tlb.c               |    3 +--
 init/main.c                     |    1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 16a5c84..2a4848d 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -129,6 +129,8 @@ static inline void reset_lazy_tlbstate(void)
 
 #define local_flush_tlb() __flush_tlb()
 
+extern int init_smp_flush(void);
+
 extern void flush_tlb_all(void);
 extern void flush_tlb_current_task(void);
 extern void flush_tlb_mm(struct mm_struct *);
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index e69bdad..0f030fb 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -229,7 +229,7 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
 	flush_tlb_others_ipi(cpumask, mm, va);
 }
 
-static int __cpuinit init_smp_flush(void)
+int __init init_smp_flush(void)
 {
 	int i;
 
@@ -238,7 +238,6 @@ static int __cpuinit init_smp_flush(void)
 
 	return 0;
 }
-core_initcall(init_smp_flush);
 
 void flush_tlb_current_task(void)
 {
diff --git a/init/main.c b/init/main.c
index 3bbf93b..20e86f8 100644
--- a/init/main.c
+++ b/init/main.c
@@ -650,6 +650,7 @@ asmlinkage void __init start_kernel(void)
 	vfs_caches_init_early();
 	cpuset_init_early();
 	page_cgroup_init();
+	init_smp_flush();
 	mem_init();
 	enable_debug_pagealloc();
 	cpu_hotplug_init();

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

Ingo Molnar

2009-May-11 13:12 UTC

head link

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

* tip-bot for Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> Commit-ID:  636afdbe85d50d9b88f5283b5e6d0f8518431aac
> Gitweb:    
http://git.kernel.org/tip/636afdbe85d50d9b88f5283b5e6d0f8518431aac
> Author:     Ian Campbell <Ian.Campbell@eu.citrix.com>
> AuthorDate: Fri, 8 May 2009 16:52:07 +0100
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Mon, 11 May 2009 15:08:08 +0200
> 
> x86: use flush_tlb_others to implement flush_tlb_all, fix
> 
> "use the flush_tlb_others() call to implement flush_tlb_all()"
> causes:
>          BUG: spinlock bad magic on CPU#0, swapper/0
>           lock: c05b7588, .magic: 00000000, .owner: <none>/-1,
.owner_cpu: 0
>          Pid: 0, comm: swapper Not tainted
2.6.30-rc3-x86_32p-xen0-tip-01797-g3db7847 #1054
>          Call Trace:
>           [<c023e656>] spin_bug+0x96/0xf0
>           [<c023e90f>] _raw_spin_lock+0x6f/0x150
> 
> It can be called before the core_initcalls are run. Explicitly call
> init_smp_flush early on.
> 
> Also there is no need for init_smp_flush to be __cpuinit.
> 
> [ Impact: fix boot crash/warning ]
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Xen-devel <xen-devel@lists.xensource.com>
> Acked-by: Jeremy Fitzhardinge <Jeremy.Fitzhardinge@citrix.com>
> LKML-Reference: <1241797927.15972.51.camel@zakaz.uk.xensource.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> 
> ---
>  arch/x86/include/asm/tlbflush.h |    2 ++
>  arch/x86/mm/tlb.c               |    3 +--
>  init/main.c                     |    1 +
>  3 files changed, 4 insertions(+), 2 deletions(-)
which then does:

init/main.c:728: error: implicit declaration of function ‘init_smp_flush’

i''ve excluded tip:x86/xen from tip:master for now.

	Ingo

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

Ian Campbell

2009-May-11 15:00 UTC

head link

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

On Mon, 2009-05-11 at 09:12 -0400, Ingo Molnar wrote:
> which then does:
> 
> init/main.c:728: error: implicit declaration of function ‘init_smp_flush’
Er, yeah, that change is pretty bogus for !x86 (I guess you were
compiling for some other $ARCH?)

You could either take the original patch which made it an early initcall
instead (http://marc.info/?l=linux-kernel&m=124179652227660&w=2) or we
could move it much earlier into setup arch, as per:

Subject: Call init_smp_flush() from arch code

init_smp_flush() is x86 specific so call it from arch code rather than
generic code.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 1b1c851..aade391 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -985,6 +985,8 @@ void __init setup_arch(char **cmdline_p)
 
 	e820_setup_gap();
 
+	init_smp_flush();
+
 #ifdef CONFIG_VT
 #if defined(CONFIG_VGA_CONSOLE)
 	if (!efi_enabled || (efi_mem_type(0xa0000) != EFI_CONVENTIONAL_MEMORY))
diff --git a/init/main.c b/init/main.c
index f798501..33ce929 100644
--- a/init/main.c
+++ b/init/main.c
@@ -651,7 +651,6 @@ asmlinkage void __init start_kernel(void)
 	vfs_caches_init_early();
 	cpuset_init_early();
 	page_cgroup_init();
-	init_smp_flush();
 	mem_init();
 	enable_debug_pagealloc();
 	cpu_hotplug_init();



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

tip-bot for Ian Campbell

2009-May-11 15:06 UTC

head link

[Xen-devel] [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

Commit-ID:  c3f10fbccdb558dbc0652487502bbd7150231945
Gitweb:     http://git.kernel.org/tip/c3f10fbccdb558dbc0652487502bbd7150231945
Author:     Ian Campbell <Ian.Campbell@eu.citrix.com>
AuthorDate: Fri, 8 May 2009 16:52:07 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 11 May 2009 17:03:02 +0200

x86: use flush_tlb_others to implement flush_tlb_all, fix

"use the flush_tlb_others() call to implement flush_tlb_all()"
causes:
         BUG: spinlock bad magic on CPU#0, swapper/0
          lock: c05b7588, .magic: 00000000, .owner: <none>/-1, .owner_cpu:
0
         Pid: 0, comm: swapper Not tainted
2.6.30-rc3-x86_32p-xen0-tip-01797-g3db7847 #1054
         Call Trace:
          [<c023e656>] spin_bug+0x96/0xf0
          [<c023e90f>] _raw_spin_lock+0x6f/0x150

It can be called before the core_initcalls are run. Explicitly call
init_smp_flush early on.

Also there is no need for init_smp_flush to be __cpuinit.

[ Impact: fix boot crash/warning ]

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Xen-devel <xen-devel@lists.xensource.com>
Acked-by: Jeremy Fitzhardinge <Jeremy.Fitzhardinge@citrix.com>
LKML-Reference: <1241797927.15972.51.camel@zakaz.uk.xensource.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/include/asm/tlbflush.h |    2 ++
 arch/x86/kernel/setup.c         |    2 ++
 arch/x86/mm/tlb.c               |    3 +--
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 16a5c84..2a4848d 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -129,6 +129,8 @@ static inline void reset_lazy_tlbstate(void)
 
 #define local_flush_tlb() __flush_tlb()
 
+extern int init_smp_flush(void);
+
 extern void flush_tlb_all(void);
 extern void flush_tlb_current_task(void);
 extern void flush_tlb_mm(struct mm_struct *);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index b415843..bde4baf 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -984,6 +984,8 @@ void __init setup_arch(char **cmdline_p)
 
 	e820_setup_gap();
 
+	init_smp_flush();
+
 #ifdef CONFIG_VT
 #if defined(CONFIG_VGA_CONSOLE)
 	if (!efi_enabled || (efi_mem_type(0xa0000) != EFI_CONVENTIONAL_MEMORY))
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index e69bdad..0f030fb 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -229,7 +229,7 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
 	flush_tlb_others_ipi(cpumask, mm, va);
 }
 
-static int __cpuinit init_smp_flush(void)
+int __init init_smp_flush(void)
 {
 	int i;
 
@@ -238,7 +238,6 @@ static int __cpuinit init_smp_flush(void)
 
 	return 0;
 }
-core_initcall(init_smp_flush);
 
 void flush_tlb_current_task(void)
 {

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

Ingo Molnar

2009-May-11 15:11 UTC

head link

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

* Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -985,6 +985,8 @@ void __init setup_arch(char **cmdline_p)
>  
>  	e820_setup_gap();
>  
> +	init_smp_flush();
> +
please test patches better:

arch/x86/kernel/setup.c:1006: error: implicit declaration of function
‘init_smp_flush’

	Ingo

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

Ian Campbell

2009-May-11 15:39 UTC

head link

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

On Mon, 2009-05-11 at 11:11 -0400, Ingo Molnar wrote:> * Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> 
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -985,6 +985,8 @@ void __init setup_arch(char **cmdline_p)
> >  
> >  	e820_setup_gap();
> >  
> > +	init_smp_flush();
> > +
> 
> please test patches better:
> 
> arch/x86/kernel/setup.c:1006: error: implicit declaration of function
‘init_smp_flush’
Doesn''t happen in my environment:
+ make O=../linux-2.6-build-dom0-x86_32p-xen0 ARCH=x86 arch/x86/kernel/setup.o
  Using /local/scratch/ianc/devel/kernels/linux-2.6 as source for kernel
  GEN    
/local/scratch/ianc/devel/kernels/linux-2.6-build-dom0-x86_32p-xen0/Makefile
  CHK     include/linux/version.h
  CHK     include/linux/utsrelease.h
  SYMLINK include/asm -> include/asm-x86
  CALL    /local/scratch/ianc/devel/kernels/linux-2.6/scripts/checksyscalls.sh
  CC      arch/x86/kernel/setup.o

This was on f6271becbae641903a9f703b7b75cccb202df82a which is latest
tip/master. I guess either you have unpushed changes or it is config
specific.

in any case I would guess that the fix is:

--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -87,6 +87,7 @@
 #include <asm/cacheflush.h>
 #include <asm/processor.h>
 #include <asm/bugs.h>
+#include <asm/tlbflush.h>
 
 #include <asm/system.h>
 #include <asm/vsyscall.h>




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

Ingo Molnar

2009-May-11 15:40 UTC

head link

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

sigh:

BUG: using smp_processor_id() in preemptible [00000000] code: S99local/1903
caller is flush_tlb_others_ipi+0x1b/0xc0
Pid: 1903, comm: S99local Not tainted 
2.6.30-rc5-tip-01485-g1304846-dirty #40905
Call Trace:
 [<40a9e184>] ? printk+0x1d/0x1f
 [<4066d2e6>] debug_smp_processor_id+0xc6/0xd0
 [<40422dab>] flush_tlb_others_ipi+0x1b/0xc0
 [<40422e5d>] native_flush_tlb_others+0xd/0x10
 [<40422ea9>] flush_tlb_all+0x19/0x60
 [<4041e972>] zap_low_mappings+0x52/0x60
 [<40a9b080>] native_cpu_up+0x156/0x220
 [<40a9ca42>] _cpu_up+0xb0/0x151
 [<40a9cb66>] cpu_up+0x83/0x8d
 [<40a68b90>] store_online+0x60/0xa0
 [<40a68b30>] ? store_online+0x0/0xa0
 [<4072ed8a>] sysdev_store+0x2a/0x40
 [<40522511>] sysfs_write_file+0xa1/0xf0
 [<404d9d44>] vfs_write+0xa4/0x130
 [<4065fcec>] ? trace_hardirqs_on_thunk+0xc/0x10
 [<40522470>] ? sysfs_write_file+0x0/0xf0
 [<404d9ea2>] sys_write+0x42/0x70
 [<40403bc8>] sysenter_do_call+0x12/0x3c
CPU0 attaching NULL sched-domain.

	Ingo

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

Ingo Molnar

2009-May-11 15:42 UTC

head link

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

* Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> in any case I would guess that the fix is:
> 
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -87,6 +87,7 @@
>  #include <asm/cacheflush.h>
>  #include <asm/processor.h>
>  #include <asm/bugs.h>
> +#include <asm/tlbflush.h>
>  
>  #include <asm/system.h>
>  #include <asm/vsyscall.h>
Guessing is not enough - please review the prototype position. Is it 
available on all configs? It is not ...

But even the proper fix runs into that smp_processor_id() bug i just 
posted.

	Ingo

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

Ian Campbell

2009-May-11 15:59 UTC

head link

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

On Mon, 2009-05-11 at 11:42 -0400, Ingo Molnar wrote:> * Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:
> 
> > in any case I would guess that the fix is:
> > 
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -87,6 +87,7 @@
> >  #include <asm/cacheflush.h>
> >  #include <asm/processor.h>
> >  #include <asm/bugs.h>
> > +#include <asm/tlbflush.h>
> >  
> >  #include <asm/system.h>
> >  #include <asm/vsyscall.h>
> 
> Guessing is not enough - please review the prototype position. Is it 
> available on all configs? It is not ...
Damn, you are (of course) right. There is nothing for it to do on !SMP
so patch below. I also observe that since it''s no longer an initcall
there is no need for a return value so included that change too.
> But even the proper fix runs into that smp_processor_id() bug i just 
> posted.
That''s back to an issue in the original patch from Jeremy I think,
I''ll
leave that one to him and/or tomorrow -- I''ve botched enough patches
for
one day!

Ian.

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index a3f42bf..e1e3e6a 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -89,6 +89,8 @@ static inline void __flush_tlb_one(unsigned long addr)
 
 #ifndef CONFIG_SMP
 
+static inline void init_smp_flush(void) {}
+
 #define flush_tlb() __flush_tlb()
 #define flush_tlb_all() __flush_tlb_all()
 #define local_flush_tlb() __flush_tlb()
@@ -129,7 +131,7 @@ static inline void reset_lazy_tlbstate(void)
 
 #define local_flush_tlb() __flush_tlb()
 
-extern int init_smp_flush(void);
+extern void init_smp_flush(void);
 
 extern void flush_tlb_all(void);
 extern void flush_tlb_current_task(void);
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 03f52f5..db28919 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -217,14 +217,12 @@ void native_flush_tlb_others(const struct cpumask
*cpumask,
 	flush_tlb_others_ipi(cpumask, mm, va);
 }
 
-int __init init_smp_flush(void)
+void __init init_smp_flush(void)
 {
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(flush_state); i++)
 		spin_lock_init(&flush_state[i].tlbstate_lock);
-
-	return 0;
 }
 
 void flush_tlb_current_task(void)



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

Xen devel - May 2009 - [GIT PULL] Xen core updates

[Xen-devel] [GIT PULL] Xen core updates

[Xen-devel] [PATCH 1/8] xen/x86-64: fix breakpoints and hardware watchpoints

[Xen-devel] [PATCH 2/8] xen/x86-64: clean up warnings about IST-using traps

[Xen-devel] [PATCH 3/8] xen: cache cr0 value to avoid trap''n''emulate for read_cr0

[Xen-devel] [PATCH 4/8] xen: deal with NMI''s use of IST too

[Xen-devel] [PATCH 5/8] xen/i386: make sure initial VGA/ISA mappings are not overridden

[Xen-devel] [PATCH 6/8] x86: use flush_tlb_others to implement flush_tlb_all

[Xen-devel] [PATCH 7/8] xen/i386: reserve Xen pagetables

[Xen-devel] [PATCH 8/8] xen: reserve Xen start_info rather than e820 reserving

[Xen-devel] Re: [PATCH 5/8] xen/i386: make sure initial VGA/ISA mappings are not overridden

[Xen-devel] Re: [PATCH 5/8] xen/i386: make sure initial VGA/ISA mappings are not overridden

[Xen-devel] Re: [PATCH 6/8] x86: use flush_tlb_others to implement flush_tlb_all

[Xen-devel] Re: [PATCH 6/8] x86: use flush_tlb_others to implement flush_tlb_all

[Xen-devel] Re: [PATCH 6/8] x86: use flush_tlb_others to implement flush_tlb_all

[Xen-devel] [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

[Xen-devel] [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix

[Xen-devel] Re: [tip:x86/xen] x86: use flush_tlb_others to implement flush_tlb_all, fix