All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] Xen bugfixes
@ 2009-09-09 23:51 ` Jeremy Fitzhardinge
  0 siblings, 0 replies; 11+ messages in thread
From: Jeremy Fitzhardinge @ 2009-09-09 23:51 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: the arch/x86 maintainers, Linux Kernel Mailing List, Xen-devel,
	Stable Kernel, Tejun Heo

Hi,

Here's 3 patches which fix two Xen PV spinlock bugs, and makes CONFIG_CC_STACKPROTECTOR work
on both 32- and 64-bit.

The spinlock bugs are both rare races:
 - lock can briefly hold the lock while interrupts are enabled, allowing an ISR to deadlock
 - unlock doesn't enforce CPU memory ordering between the actual unlock write and the check
   to see if there are any pending waiters, so it can end up deciding there's nobody to
   kick on unlock, leaving another CPU hanging.  It needs a full mb() to guarantee the correct
   ordering.

The stack-protector fix bites the bullet and does a full GDT setup early so that we can load
%gs for the stack-protector canary segment on i386.  This also removes the assumption that the
initial percpu %fs segment has a zero base.

x86-64 still just needs the GS_BASE MSR written, but that now happens using the same code as
i386 rather than being special cased.

Thanks,
	J

The following changes since commit e07cccf4046978df10f2e13fe2b99b2f9b3a65db:
  Linus Torvalds (1):
        Linux 2.6.31-rc9

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git bugfix

Jeremy Fitzhardinge (2):
      xen: make -fstack-protector work under Xen
      xen: only enable interrupts while actually blocking for spinlock

Yang Xiaowei (1):
      xen: use stronger barrier after unlocking lock

 arch/x86/mm/Makefile     |    4 ++
 arch/x86/xen/Makefile    |    2 +
 arch/x86/xen/enlighten.c |  131 +++++++++++++++++++++++++++++++++++++++-------
 arch/x86/xen/smp.c       |    1 +
 arch/x86/xen/spinlock.c  |   28 ++++++----
 drivers/xen/Makefile     |    3 +
 6 files changed, 140 insertions(+), 29 deletions(-)

diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile
index eefdeee..72bb3a2 100644
--- a/arch/x86/mm/Makefile
+++ b/arch/x86/mm/Makefile
@@ -1,6 +1,10 @@
 obj-y	:=  init.o init_$(BITS).o fault.o ioremap.o extable.o pageattr.o mmap.o \
 	    pat.o pgtable.o gup.o
 
+# Make sure __phys_addr has no stackprotector
+nostackp := $(call cc-option, -fno-stack-protector)
+CFLAGS_ioremap.o		:= $(nostackp)
+
 obj-$(CONFIG_SMP)		+= tlb.o
 
 obj-$(CONFIG_X86_32)		+= pgtable_32.o iomap_32.o
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index 7410640..3bb4fc2 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -8,6 +8,7 @@ endif
 # Make sure early boot has no stackprotector
 nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_enlighten.o		:= $(nostackp)
+CFLAGS_mmu.o			:= $(nostackp)
 
 obj-y		:= enlighten.o setup.o multicalls.o mmu.o irq.o \
 			time.o xen-asm.o xen-asm_$(BITS).o \
@@ -16,3 +17,4 @@ obj-y		:= enlighten.o setup.o multicalls.o mmu.o irq.o \
 obj-$(CONFIG_SMP)		+= smp.o
 obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= spinlock.o
 obj-$(CONFIG_XEN_DEBUG_FS)	+= debugfs.o
+
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index eb33aaa..7614313 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -51,6 +51,7 @@
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
 #include <asm/reboot.h>
+#include <asm/stackprotector.h>
 
 #include "xen-ops.h"
 #include "mmu.h"
@@ -330,18 +331,28 @@ static void xen_load_gdt(const struct desc_ptr *dtr)
 	unsigned long frames[pages];
 	int f;
 
-	/* A GDT can be up to 64k in size, which corresponds to 8192
-	   8-byte entries, or 16 4k pages.. */
+	/*
+	 * A GDT can be up to 64k in size, which corresponds to 8192
+	 * 8-byte entries, or 16 4k pages..
+	 */
 
 	BUG_ON(size > 65536);
 	BUG_ON(va & ~PAGE_MASK);
 
 	for (f = 0; va < dtr->address + size; va += PAGE_SIZE, f++) {
 		int level;
-		pte_t *ptep = lookup_address(va, &level);
+		pte_t *ptep;
 		unsigned long pfn, mfn;
 		void *virt;
 
+		/*
+		 * The GDT is per-cpu and is in the percpu data area.
+		 * That can be virtually mapped, so we need to do a
+		 * page-walk to get the underlying MFN for the
+		 * hypercall.  The page can also be in the kernel's
+		 * linear range, so we need to RO that mapping too.
+		 */
+		ptep = lookup_address(va, &level);
 		BUG_ON(ptep == NULL);
 
 		pfn = pte_pfn(*ptep);
@@ -358,6 +369,44 @@ static void xen_load_gdt(const struct desc_ptr *dtr)
 		BUG();
 }
 
+/*
+ * load_gdt for early boot, when the gdt is only mapped once
+ */
+static __init void xen_load_gdt_boot(const struct desc_ptr *dtr)
+{
+	unsigned long va = dtr->address;
+	unsigned int size = dtr->size + 1;
+	unsigned pages = (size + PAGE_SIZE - 1) / PAGE_SIZE;
+	unsigned long frames[pages];
+	int f;
+
+	/*
+	 * A GDT can be up to 64k in size, which corresponds to 8192
+	 * 8-byte entries, or 16 4k pages..
+	 */
+
+	BUG_ON(size > 65536);
+	BUG_ON(va & ~PAGE_MASK);
+
+	for (f = 0; va < dtr->address + size; va += PAGE_SIZE, f++) {
+		pte_t pte;
+		unsigned long pfn, mfn;
+
+		pfn = virt_to_pfn(va);
+		mfn = pfn_to_mfn(pfn);
+
+		pte = pfn_pte(pfn, PAGE_KERNEL_RO);
+
+		if (HYPERVISOR_update_va_mapping((unsigned long)va, pte, 0))
+			BUG();
+
+		frames[f] = mfn;
+	}
+
+	if (HYPERVISOR_set_gdt(frames, size / sizeof(struct desc_struct)))
+		BUG();
+}
+
 static void load_TLS_descriptor(struct thread_struct *t,
 				unsigned int cpu, unsigned int i)
 {
@@ -581,6 +630,29 @@ static void xen_write_gdt_entry(struct desc_struct *dt, int entry,
 	preempt_enable();
 }
 
+/*
+ * Version of write_gdt_entry for use at early boot-time needed to
+ * update an entry as simply as possible.
+ */
+static __init void xen_write_gdt_entry_boot(struct desc_struct *dt, int entry,
+					    const void *desc, int type)
+{
+	switch (type) {
+	case DESC_LDT:
+	case DESC_TSS:
+		/* ignore */
+		break;
+
+	default: {
+		xmaddr_t maddr = virt_to_machine(&dt[entry]);
+
+		if (HYPERVISOR_update_descriptor(maddr.maddr, *(u64 *)desc))
+			dt[entry] = *(struct desc_struct *)desc;
+	}
+
+	}
+}
+
 static void xen_load_sp0(struct tss_struct *tss,
 			 struct thread_struct *thread)
 {
@@ -965,6 +1037,23 @@ static const struct machine_ops __initdata xen_machine_ops = {
 	.emergency_restart = xen_emergency_restart,
 };
 
+/*
+ * Set up the GDT and segment registers for -fstack-protector.  Until
+ * we do this, we have to be careful not to call any stack-protected
+ * function, which is most of the kernel.
+ */
+static void __init xen_setup_stackprotector(void)
+{
+	pv_cpu_ops.write_gdt_entry = xen_write_gdt_entry_boot;
+	pv_cpu_ops.load_gdt = xen_load_gdt_boot;
+
+	setup_stack_canary_segment(0);
+	switch_to_new_gdt(0);
+
+	pv_cpu_ops.write_gdt_entry = xen_write_gdt_entry;
+	pv_cpu_ops.load_gdt = xen_load_gdt;
+}
+
 /* First C function to be called on Xen boot */
 asmlinkage void __init xen_start_kernel(void)
 {
@@ -983,13 +1072,28 @@ asmlinkage void __init xen_start_kernel(void)
 	pv_apic_ops = xen_apic_ops;
 	pv_mmu_ops = xen_mmu_ops;
 
-#ifdef CONFIG_X86_64
 	/*
-	 * Setup percpu state.  We only need to do this for 64-bit
-	 * because 32-bit already has %fs set properly.
+	 * Set up some pagetable state before starting to set any ptes.
 	 */
-	load_percpu_segment(0);
-#endif
+
+	/* Prevent unwanted bits from being set in PTEs. */
+	__supported_pte_mask &= ~_PAGE_GLOBAL;
+	if (!xen_initial_domain())
+		__supported_pte_mask &= ~(_PAGE_PWT | _PAGE_PCD);
+
+	__supported_pte_mask |= _PAGE_IOMAP;
+
+	xen_setup_features();
+
+	/* Get mfn list */
+	if (!xen_feature(XENFEAT_auto_translated_physmap))
+		xen_build_dynamic_phys_to_machine();
+
+	/*
+	 * Set up kernel GDT and segment registers, mainly so that
+	 * -fstack-protector code can be executed.
+	 */
+	xen_setup_stackprotector();
 
 	xen_init_irq_ops();
 	xen_init_cpuid_mask();
@@ -1001,8 +1105,6 @@ asmlinkage void __init xen_start_kernel(void)
 	set_xen_basic_apic_ops();
 #endif
 
-	xen_setup_features();
-
 	if (xen_feature(XENFEAT_mmu_pt_update_preserve_ad)) {
 		pv_mmu_ops.ptep_modify_prot_start = xen_ptep_modify_prot_start;
 		pv_mmu_ops.ptep_modify_prot_commit = xen_ptep_modify_prot_commit;
@@ -1019,17 +1121,8 @@ asmlinkage void __init xen_start_kernel(void)
 
 	xen_smp_init();
 
-	/* Get mfn list */
-	if (!xen_feature(XENFEAT_auto_translated_physmap))
-		xen_build_dynamic_phys_to_machine();
-
 	pgd = (pgd_t *)xen_start_info->pt_base;
 
-	/* Prevent unwanted bits from being set in PTEs. */
-	__supported_pte_mask &= ~_PAGE_GLOBAL;
-	if (!xen_initial_domain())
-		__supported_pte_mask &= ~(_PAGE_PWT | _PAGE_PCD);
-
 #ifdef CONFIG_X86_64
 	/* Work out if we support NX */
 	check_efer();
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 429834e..fe03eee 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -236,6 +236,7 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle)
 	ctxt->user_regs.ss = __KERNEL_DS;
 #ifdef CONFIG_X86_32
 	ctxt->user_regs.fs = __KERNEL_PERCPU;
+	ctxt->user_regs.gs = __KERNEL_STACK_CANARY;
 #else
 	ctxt->gs_base_kernel = per_cpu_offset(cpu);
 #endif
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 5601506..36a5141 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -187,7 +187,6 @@ static noinline int xen_spin_lock_slow(struct raw_spinlock *lock, bool irq_enabl
 	struct xen_spinlock *prev;
 	int irq = __get_cpu_var(lock_kicker_irq);
 	int ret;
-	unsigned long flags;
 	u64 start;
 
 	/* If kicker interrupts not initialized yet, just spin */
@@ -199,16 +198,12 @@ static noinline int xen_spin_lock_slow(struct raw_spinlock *lock, bool irq_enabl
 	/* announce we're spinning */
 	prev = spinning_lock(xl);
 
-	flags = __raw_local_save_flags();
-	if (irq_enable) {
-		ADD_STATS(taken_slow_irqenable, 1);
-		raw_local_irq_enable();
-	}
-
 	ADD_STATS(taken_slow, 1);
 	ADD_STATS(taken_slow_nested, prev != NULL);
 
 	do {
+		unsigned long flags;
+
 		/* clear pending */
 		xen_clear_irq_pending(irq);
 
@@ -228,6 +223,12 @@ static noinline int xen_spin_lock_slow(struct raw_spinlock *lock, bool irq_enabl
 			goto out;
 		}
 
+		flags = __raw_local_save_flags();
+		if (irq_enable) {
+			ADD_STATS(taken_slow_irqenable, 1);
+			raw_local_irq_enable();
+		}
+
 		/*
 		 * Block until irq becomes pending.  If we're
 		 * interrupted at this point (after the trylock but
@@ -238,13 +239,15 @@ static noinline int xen_spin_lock_slow(struct raw_spinlock *lock, bool irq_enabl
 		 * pending.
 		 */
 		xen_poll_irq(irq);
+
+		raw_local_irq_restore(flags);
+
 		ADD_STATS(taken_slow_spurious, !xen_test_irq_pending(irq));
 	} while (!xen_test_irq_pending(irq)); /* check for spurious wakeups */
 
 	kstat_incr_irqs_this_cpu(irq, irq_to_desc(irq));
 
 out:
-	raw_local_irq_restore(flags);
 	unspinning_lock(xl, prev);
 	spin_time_accum_blocked(start);
 
@@ -323,8 +326,13 @@ static void xen_spin_unlock(struct raw_spinlock *lock)
 	smp_wmb();		/* make sure no writes get moved after unlock */
 	xl->lock = 0;		/* release lock */
 
-	/* make sure unlock happens before kick */
-	barrier();
+	/*
+	 * Make sure unlock happens before checking for waiting
+	 * spinners.  We need a strong barrier to enforce the
+	 * write-read ordering to different memory locations, as the
+	 * CPU makes no implied guarantees about their ordering.
+	 */
+	mb();
 
 	if (unlikely(xl->spinners))
 		xen_spin_unlock_slow(xl);
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index ec2a39b..7c28434 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -1,6 +1,9 @@
 obj-y	+= grant-table.o features.o events.o manage.o
 obj-y	+= xenbus/
 
+nostackp := $(call cc-option, -fno-stack-protector)
+CFLAGS_features.o			:= $(nostackp)
+
 obj-$(CONFIG_HOTPLUG_CPU)	+= cpu_hotplug.o
 obj-$(CONFIG_XEN_XENCOMM)	+= xencomm.o
 obj-$(CONFIG_XEN_BALLOON)	+= balloon.o



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [GIT PULL] Xen bugfixes
@ 2009-09-09 23:51 ` Jeremy Fitzhardinge
  0 siblings, 0 replies; 11+ messages in thread
From: Jeremy Fitzhardinge @ 2009-09-09 23:51 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Xen-devel, Tejun Heo, the arch/x86 maintainers,
	Linux Kernel Mailing List, Stable Kernel

Hi,

Here's 3 patches which fix two Xen PV spinlock bugs, and makes CONFIG_CC_STACKPROTECTOR work
on both 32- and 64-bit.

The spinlock bugs are both rare races:
 - lock can briefly hold the lock while interrupts are enabled, allowing an ISR to deadlock
 - unlock doesn't enforce CPU memory ordering between the actual unlock write and the check
   to see if there are any pending waiters, so it can end up deciding there's nobody to
   kick on unlock, leaving another CPU hanging.  It needs a full mb() to guarantee the correct
   ordering.

The stack-protector fix bites the bullet and does a full GDT setup early so that we can load
%gs for the stack-protector canary segment on i386.  This also removes the assumption that the
initial percpu %fs segment has a zero base.

x86-64 still just needs the GS_BASE MSR written, but that now happens using the same code as
i386 rather than being special cased.

Thanks,
	J

The following changes since commit e07cccf4046978df10f2e13fe2b99b2f9b3a65db:
  Linus Torvalds (1):
        Linux 2.6.31-rc9

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git bugfix

Jeremy Fitzhardinge (2):
      xen: make -fstack-protector work under Xen
      xen: only enable interrupts while actually blocking for spinlock

Yang Xiaowei (1):
      xen: use stronger barrier after unlocking lock

 arch/x86/mm/Makefile     |    4 ++
 arch/x86/xen/Makefile    |    2 +
 arch/x86/xen/enlighten.c |  131 +++++++++++++++++++++++++++++++++++++++-------
 arch/x86/xen/smp.c       |    1 +
 arch/x86/xen/spinlock.c  |   28 ++++++----
 drivers/xen/Makefile     |    3 +
 6 files changed, 140 insertions(+), 29 deletions(-)

diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile
index eefdeee..72bb3a2 100644
--- a/arch/x86/mm/Makefile
+++ b/arch/x86/mm/Makefile
@@ -1,6 +1,10 @@
 obj-y	:=  init.o init_$(BITS).o fault.o ioremap.o extable.o pageattr.o mmap.o \
 	    pat.o pgtable.o gup.o
 
+# Make sure __phys_addr has no stackprotector
+nostackp := $(call cc-option, -fno-stack-protector)
+CFLAGS_ioremap.o		:= $(nostackp)
+
 obj-$(CONFIG_SMP)		+= tlb.o
 
 obj-$(CONFIG_X86_32)		+= pgtable_32.o iomap_32.o
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index 7410640..3bb4fc2 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -8,6 +8,7 @@ endif
 # Make sure early boot has no stackprotector
 nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_enlighten.o		:= $(nostackp)
+CFLAGS_mmu.o			:= $(nostackp)
 
 obj-y		:= enlighten.o setup.o multicalls.o mmu.o irq.o \
 			time.o xen-asm.o xen-asm_$(BITS).o \
@@ -16,3 +17,4 @@ obj-y		:= enlighten.o setup.o multicalls.o mmu.o irq.o \
 obj-$(CONFIG_SMP)		+= smp.o
 obj-$(CONFIG_PARAVIRT_SPINLOCKS)+= spinlock.o
 obj-$(CONFIG_XEN_DEBUG_FS)	+= debugfs.o
+
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index eb33aaa..7614313 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -51,6 +51,7 @@
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
 #include <asm/reboot.h>
+#include <asm/stackprotector.h>
 
 #include "xen-ops.h"
 #include "mmu.h"
@@ -330,18 +331,28 @@ static void xen_load_gdt(const struct desc_ptr *dtr)
 	unsigned long frames[pages];
 	int f;
 
-	/* A GDT can be up to 64k in size, which corresponds to 8192
-	   8-byte entries, or 16 4k pages.. */
+	/*
+	 * A GDT can be up to 64k in size, which corresponds to 8192
+	 * 8-byte entries, or 16 4k pages..
+	 */
 
 	BUG_ON(size > 65536);
 	BUG_ON(va & ~PAGE_MASK);
 
 	for (f = 0; va < dtr->address + size; va += PAGE_SIZE, f++) {
 		int level;
-		pte_t *ptep = lookup_address(va, &level);
+		pte_t *ptep;
 		unsigned long pfn, mfn;
 		void *virt;
 
+		/*
+		 * The GDT is per-cpu and is in the percpu data area.
+		 * That can be virtually mapped, so we need to do a
+		 * page-walk to get the underlying MFN for the
+		 * hypercall.  The page can also be in the kernel's
+		 * linear range, so we need to RO that mapping too.
+		 */
+		ptep = lookup_address(va, &level);
 		BUG_ON(ptep == NULL);
 
 		pfn = pte_pfn(*ptep);
@@ -358,6 +369,44 @@ static void xen_load_gdt(const struct desc_ptr *dtr)
 		BUG();
 }
 
+/*
+ * load_gdt for early boot, when the gdt is only mapped once
+ */
+static __init void xen_load_gdt_boot(const struct desc_ptr *dtr)
+{
+	unsigned long va = dtr->address;
+	unsigned int size = dtr->size + 1;
+	unsigned pages = (size + PAGE_SIZE - 1) / PAGE_SIZE;
+	unsigned long frames[pages];
+	int f;
+
+	/*
+	 * A GDT can be up to 64k in size, which corresponds to 8192
+	 * 8-byte entries, or 16 4k pages..
+	 */
+
+	BUG_ON(size > 65536);
+	BUG_ON(va & ~PAGE_MASK);
+
+	for (f = 0; va < dtr->address + size; va += PAGE_SIZE, f++) {
+		pte_t pte;
+		unsigned long pfn, mfn;
+
+		pfn = virt_to_pfn(va);
+		mfn = pfn_to_mfn(pfn);
+
+		pte = pfn_pte(pfn, PAGE_KERNEL_RO);
+
+		if (HYPERVISOR_update_va_mapping((unsigned long)va, pte, 0))
+			BUG();
+
+		frames[f] = mfn;
+	}
+
+	if (HYPERVISOR_set_gdt(frames, size / sizeof(struct desc_struct)))
+		BUG();
+}
+
 static void load_TLS_descriptor(struct thread_struct *t,
 				unsigned int cpu, unsigned int i)
 {
@@ -581,6 +630,29 @@ static void xen_write_gdt_entry(struct desc_struct *dt, int entry,
 	preempt_enable();
 }
 
+/*
+ * Version of write_gdt_entry for use at early boot-time needed to
+ * update an entry as simply as possible.
+ */
+static __init void xen_write_gdt_entry_boot(struct desc_struct *dt, int entry,
+					    const void *desc, int type)
+{
+	switch (type) {
+	case DESC_LDT:
+	case DESC_TSS:
+		/* ignore */
+		break;
+
+	default: {
+		xmaddr_t maddr = virt_to_machine(&dt[entry]);
+
+		if (HYPERVISOR_update_descriptor(maddr.maddr, *(u64 *)desc))
+			dt[entry] = *(struct desc_struct *)desc;
+	}
+
+	}
+}
+
 static void xen_load_sp0(struct tss_struct *tss,
 			 struct thread_struct *thread)
 {
@@ -965,6 +1037,23 @@ static const struct machine_ops __initdata xen_machine_ops = {
 	.emergency_restart = xen_emergency_restart,
 };
 
+/*
+ * Set up the GDT and segment registers for -fstack-protector.  Until
+ * we do this, we have to be careful not to call any stack-protected
+ * function, which is most of the kernel.
+ */
+static void __init xen_setup_stackprotector(void)
+{
+	pv_cpu_ops.write_gdt_entry = xen_write_gdt_entry_boot;
+	pv_cpu_ops.load_gdt = xen_load_gdt_boot;
+
+	setup_stack_canary_segment(0);
+	switch_to_new_gdt(0);
+
+	pv_cpu_ops.write_gdt_entry = xen_write_gdt_entry;
+	pv_cpu_ops.load_gdt = xen_load_gdt;
+}
+
 /* First C function to be called on Xen boot */
 asmlinkage void __init xen_start_kernel(void)
 {
@@ -983,13 +1072,28 @@ asmlinkage void __init xen_start_kernel(void)
 	pv_apic_ops = xen_apic_ops;
 	pv_mmu_ops = xen_mmu_ops;
 
-#ifdef CONFIG_X86_64
 	/*
-	 * Setup percpu state.  We only need to do this for 64-bit
-	 * because 32-bit already has %fs set properly.
+	 * Set up some pagetable state before starting to set any ptes.
 	 */
-	load_percpu_segment(0);
-#endif
+
+	/* Prevent unwanted bits from being set in PTEs. */
+	__supported_pte_mask &= ~_PAGE_GLOBAL;
+	if (!xen_initial_domain())
+		__supported_pte_mask &= ~(_PAGE_PWT | _PAGE_PCD);
+
+	__supported_pte_mask |= _PAGE_IOMAP;
+
+	xen_setup_features();
+
+	/* Get mfn list */
+	if (!xen_feature(XENFEAT_auto_translated_physmap))
+		xen_build_dynamic_phys_to_machine();
+
+	/*
+	 * Set up kernel GDT and segment registers, mainly so that
+	 * -fstack-protector code can be executed.
+	 */
+	xen_setup_stackprotector();
 
 	xen_init_irq_ops();
 	xen_init_cpuid_mask();
@@ -1001,8 +1105,6 @@ asmlinkage void __init xen_start_kernel(void)
 	set_xen_basic_apic_ops();
 #endif
 
-	xen_setup_features();
-
 	if (xen_feature(XENFEAT_mmu_pt_update_preserve_ad)) {
 		pv_mmu_ops.ptep_modify_prot_start = xen_ptep_modify_prot_start;
 		pv_mmu_ops.ptep_modify_prot_commit = xen_ptep_modify_prot_commit;
@@ -1019,17 +1121,8 @@ asmlinkage void __init xen_start_kernel(void)
 
 	xen_smp_init();
 
-	/* Get mfn list */
-	if (!xen_feature(XENFEAT_auto_translated_physmap))
-		xen_build_dynamic_phys_to_machine();
-
 	pgd = (pgd_t *)xen_start_info->pt_base;
 
-	/* Prevent unwanted bits from being set in PTEs. */
-	__supported_pte_mask &= ~_PAGE_GLOBAL;
-	if (!xen_initial_domain())
-		__supported_pte_mask &= ~(_PAGE_PWT | _PAGE_PCD);
-
 #ifdef CONFIG_X86_64
 	/* Work out if we support NX */
 	check_efer();
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 429834e..fe03eee 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -236,6 +236,7 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle)
 	ctxt->user_regs.ss = __KERNEL_DS;
 #ifdef CONFIG_X86_32
 	ctxt->user_regs.fs = __KERNEL_PERCPU;
+	ctxt->user_regs.gs = __KERNEL_STACK_CANARY;
 #else
 	ctxt->gs_base_kernel = per_cpu_offset(cpu);
 #endif
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index 5601506..36a5141 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -187,7 +187,6 @@ static noinline int xen_spin_lock_slow(struct raw_spinlock *lock, bool irq_enabl
 	struct xen_spinlock *prev;
 	int irq = __get_cpu_var(lock_kicker_irq);
 	int ret;
-	unsigned long flags;
 	u64 start;
 
 	/* If kicker interrupts not initialized yet, just spin */
@@ -199,16 +198,12 @@ static noinline int xen_spin_lock_slow(struct raw_spinlock *lock, bool irq_enabl
 	/* announce we're spinning */
 	prev = spinning_lock(xl);
 
-	flags = __raw_local_save_flags();
-	if (irq_enable) {
-		ADD_STATS(taken_slow_irqenable, 1);
-		raw_local_irq_enable();
-	}
-
 	ADD_STATS(taken_slow, 1);
 	ADD_STATS(taken_slow_nested, prev != NULL);
 
 	do {
+		unsigned long flags;
+
 		/* clear pending */
 		xen_clear_irq_pending(irq);
 
@@ -228,6 +223,12 @@ static noinline int xen_spin_lock_slow(struct raw_spinlock *lock, bool irq_enabl
 			goto out;
 		}
 
+		flags = __raw_local_save_flags();
+		if (irq_enable) {
+			ADD_STATS(taken_slow_irqenable, 1);
+			raw_local_irq_enable();
+		}
+
 		/*
 		 * Block until irq becomes pending.  If we're
 		 * interrupted at this point (after the trylock but
@@ -238,13 +239,15 @@ static noinline int xen_spin_lock_slow(struct raw_spinlock *lock, bool irq_enabl
 		 * pending.
 		 */
 		xen_poll_irq(irq);
+
+		raw_local_irq_restore(flags);
+
 		ADD_STATS(taken_slow_spurious, !xen_test_irq_pending(irq));
 	} while (!xen_test_irq_pending(irq)); /* check for spurious wakeups */
 
 	kstat_incr_irqs_this_cpu(irq, irq_to_desc(irq));
 
 out:
-	raw_local_irq_restore(flags);
 	unspinning_lock(xl, prev);
 	spin_time_accum_blocked(start);
 
@@ -323,8 +326,13 @@ static void xen_spin_unlock(struct raw_spinlock *lock)
 	smp_wmb();		/* make sure no writes get moved after unlock */
 	xl->lock = 0;		/* release lock */
 
-	/* make sure unlock happens before kick */
-	barrier();
+	/*
+	 * Make sure unlock happens before checking for waiting
+	 * spinners.  We need a strong barrier to enforce the
+	 * write-read ordering to different memory locations, as the
+	 * CPU makes no implied guarantees about their ordering.
+	 */
+	mb();
 
 	if (unlikely(xl->spinners))
 		xen_spin_unlock_slow(xl);
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index ec2a39b..7c28434 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -1,6 +1,9 @@
 obj-y	+= grant-table.o features.o events.o manage.o
 obj-y	+= xenbus/
 
+nostackp := $(call cc-option, -fno-stack-protector)
+CFLAGS_features.o			:= $(nostackp)
+
 obj-$(CONFIG_HOTPLUG_CPU)	+= cpu_hotplug.o
 obj-$(CONFIG_XEN_XENCOMM)	+= xencomm.o
 obj-$(CONFIG_XEN_BALLOON)	+= balloon.o

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Xen bugfixes
  2009-09-09 23:51 ` Jeremy Fitzhardinge
@ 2009-09-10  5:16   ` Ingo Molnar
  -1 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2009-09-10  5:16 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Ingo Molnar, the arch/x86 maintainers, Linux Kernel Mailing List,
	Xen-devel, Stable Kernel, Tejun Heo


* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Hi,
> 
> Here's 3 patches which fix two Xen PV spinlock bugs, and makes 
> CONFIG_CC_STACKPROTECTOR work on both 32- and 64-bit.
> 
> The spinlock bugs are both rare races:
>  - lock can briefly hold the lock while interrupts are enabled, allowing an ISR to deadlock
>  - unlock doesn't enforce CPU memory ordering between the actual unlock write and the check
>    to see if there are any pending waiters, so it can end up deciding there's nobody to
>    kick on unlock, leaving another CPU hanging.  It needs a full mb() to guarantee the correct
>    ordering.
> 
> The stack-protector fix bites the bullet and does a full GDT setup 
> early so that we can load %gs for the stack-protector canary 
> segment on i386.  This also removes the assumption that the 
> initial percpu %fs segment has a zero base.
> 
> x86-64 still just needs the GS_BASE MSR written, but that now 
> happens using the same code as i386 rather than being special 
> cased.
> 
> Thanks,
> 	J
> 
> The following changes since commit e07cccf4046978df10f2e13fe2b99b2f9b3a65db:
>   Linus Torvalds (1):
>         Linux 2.6.31-rc9
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git bugfix
> 
> Jeremy Fitzhardinge (2):
>       xen: make -fstack-protector work under Xen
>       xen: only enable interrupts while actually blocking for spinlock
> 
> Yang Xiaowei (1):
>       xen: use stronger barrier after unlocking lock
> 
>  arch/x86/mm/Makefile     |    4 ++
>  arch/x86/xen/Makefile    |    2 +
>  arch/x86/xen/enlighten.c |  131 +++++++++++++++++++++++++++++++++++++++-------
>  arch/x86/xen/smp.c       |    1 +
>  arch/x86/xen/spinlock.c  |   28 ++++++----
>  drivers/xen/Makefile     |    3 +
>  6 files changed, 140 insertions(+), 29 deletions(-)

Pulled, thanks a lot Jeremy!

A few comments:

> +# Make sure __phys_addr has no stackprotector
> +nostackp := $(call cc-option, -fno-stack-protector)
> +CFLAGS_ioremap.o		:= $(nostackp)
> +

Sure we could move __phys_addr into its own file and thus avoid 
turning off stackprotector for the rest of ioremap.c?

> --- a/arch/x86/xen/Makefile
> +++ b/arch/x86/xen/Makefile
> @@ -8,6 +8,7 @@ endif
>  # Make sure early boot has no stackprotector
>  nostackp := $(call cc-option, -fno-stack-protector)
>  CFLAGS_enlighten.o		:= $(nostackp)
> +CFLAGS_mmu.o			:= $(nostackp)

A similar argument could be made here - what proportion of mmu.c is 
affected?

Also, once the commits have hit upstream feel free bounce them to 
stable@kernel.org - they dont have Cc: <stable@kernel.org> tags for 
automatic back-merging requests. The fixes narrowly missed v2.6.31.

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Xen bugfixes
@ 2009-09-10  5:16   ` Ingo Molnar
  0 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2009-09-10  5:16 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Xen-devel, the arch/x86 maintainers, Linux Kernel Mailing List,
	Ingo Molnar, Tejun Heo, Stable Kernel


* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Hi,
> 
> Here's 3 patches which fix two Xen PV spinlock bugs, and makes 
> CONFIG_CC_STACKPROTECTOR work on both 32- and 64-bit.
> 
> The spinlock bugs are both rare races:
>  - lock can briefly hold the lock while interrupts are enabled, allowing an ISR to deadlock
>  - unlock doesn't enforce CPU memory ordering between the actual unlock write and the check
>    to see if there are any pending waiters, so it can end up deciding there's nobody to
>    kick on unlock, leaving another CPU hanging.  It needs a full mb() to guarantee the correct
>    ordering.
> 
> The stack-protector fix bites the bullet and does a full GDT setup 
> early so that we can load %gs for the stack-protector canary 
> segment on i386.  This also removes the assumption that the 
> initial percpu %fs segment has a zero base.
> 
> x86-64 still just needs the GS_BASE MSR written, but that now 
> happens using the same code as i386 rather than being special 
> cased.
> 
> Thanks,
> 	J
> 
> The following changes since commit e07cccf4046978df10f2e13fe2b99b2f9b3a65db:
>   Linus Torvalds (1):
>         Linux 2.6.31-rc9
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git bugfix
> 
> Jeremy Fitzhardinge (2):
>       xen: make -fstack-protector work under Xen
>       xen: only enable interrupts while actually blocking for spinlock
> 
> Yang Xiaowei (1):
>       xen: use stronger barrier after unlocking lock
> 
>  arch/x86/mm/Makefile     |    4 ++
>  arch/x86/xen/Makefile    |    2 +
>  arch/x86/xen/enlighten.c |  131 +++++++++++++++++++++++++++++++++++++++-------
>  arch/x86/xen/smp.c       |    1 +
>  arch/x86/xen/spinlock.c  |   28 ++++++----
>  drivers/xen/Makefile     |    3 +
>  6 files changed, 140 insertions(+), 29 deletions(-)

Pulled, thanks a lot Jeremy!

A few comments:

> +# Make sure __phys_addr has no stackprotector
> +nostackp := $(call cc-option, -fno-stack-protector)
> +CFLAGS_ioremap.o		:= $(nostackp)
> +

Sure we could move __phys_addr into its own file and thus avoid 
turning off stackprotector for the rest of ioremap.c?

> --- a/arch/x86/xen/Makefile
> +++ b/arch/x86/xen/Makefile
> @@ -8,6 +8,7 @@ endif
>  # Make sure early boot has no stackprotector
>  nostackp := $(call cc-option, -fno-stack-protector)
>  CFLAGS_enlighten.o		:= $(nostackp)
> +CFLAGS_mmu.o			:= $(nostackp)

A similar argument could be made here - what proportion of mmu.c is 
affected?

Also, once the commits have hit upstream feel free bounce them to 
stable@kernel.org - they dont have Cc: <stable@kernel.org> tags for 
automatic back-merging requests. The fixes narrowly missed v2.6.31.

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Xen bugfixes
  2009-09-10  5:16   ` Ingo Molnar
@ 2009-09-10 17:16     ` Jeremy Fitzhardinge
  -1 siblings, 0 replies; 11+ messages in thread
From: Jeremy Fitzhardinge @ 2009-09-10 17:16 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Ingo Molnar, the arch/x86 maintainers, Linux Kernel Mailing List,
	Xen-devel, Stable Kernel, Tejun Heo

On 09/09/09 22:16, Ingo Molnar wrote:
>> +# Make sure __phys_addr has no stackprotector
>> +nostackp := $(call cc-option, -fno-stack-protector)
>> +CFLAGS_ioremap.o		:= $(nostackp)
>> +
>>     
> Sure we could move __phys_addr into its own file and thus avoid 
> turning off stackprotector for the rest of ioremap.c?
>   

I'm not very keen on having zillions of tiny files just to cope with the
lack of per-function stackprotector disable.  I don't see any code in
ioremap.c that would really benefit from stack-protector anyway; there
are no local arrays.

At least __phys_addr and friends aren't terribly closely related to
ioremap so it would at least make some sense.  Patch below.

>> --- a/arch/x86/xen/Makefile
>> +++ b/arch/x86/xen/Makefile
>> @@ -8,6 +8,7 @@ endif
>>  # Make sure early boot has no stackprotector
>>  nostackp := $(call cc-option, -fno-stack-protector)
>>  CFLAGS_enlighten.o		:= $(nostackp)
>> +CFLAGS_mmu.o			:= $(nostackp)
>>     
> A similar argument could be made here - what proportion of mmu.c is 
> affected?
>   

More.  It would be a fairly arbitrary chunk of code to split out into a
separate file.

> Also, once the commits have hit upstream feel free bounce them to 
> stable@kernel.org - they dont have Cc: <stable@kernel.org> tags for 
> automatic back-merging requests. The fixes narrowly missed v2.6.31.
>   

Will do.

Thanks,
    J

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Subject: [PATCH] x86: split __phys_addr out into separate file

Split __phys_addr out into its own file so we can disable
-fstack-protector in a fine-grained fashion.  Also it doesn't
have terribly much to do with the rest of ioremap.c.

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

diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile
index 72bb3a2..9b5a9f5 100644
--- a/arch/x86/mm/Makefile
+++ b/arch/x86/mm/Makefile
@@ -1,9 +1,9 @@
 obj-y	:=  init.o init_$(BITS).o fault.o ioremap.o extable.o pageattr.o mmap.o \
-	    pat.o pgtable.o gup.o
+	    pat.o pgtable.o physaddr.o gup.o
 
 # Make sure __phys_addr has no stackprotector
 nostackp := $(call cc-option, -fno-stack-protector)
-CFLAGS_ioremap.o		:= $(nostackp)
+CFLAGS_physaddr.o		:= $(nostackp)
 
 obj-$(CONFIG_SMP)		+= tlb.o
 
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 8a45093..04e1ad6 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -22,77 +22,7 @@
 #include <asm/pgalloc.h>
 #include <asm/pat.h>
 
-static inline int phys_addr_valid(resource_size_t addr)
-{
-#ifdef CONFIG_PHYS_ADDR_T_64BIT
-	return !(addr >> boot_cpu_data.x86_phys_bits);
-#else
-	return 1;
-#endif
-}
-
-#ifdef CONFIG_X86_64
-
-unsigned long __phys_addr(unsigned long x)
-{
-	if (x >= __START_KERNEL_map) {
-		x -= __START_KERNEL_map;
-		VIRTUAL_BUG_ON(x >= KERNEL_IMAGE_SIZE);
-		x += phys_base;
-	} else {
-		VIRTUAL_BUG_ON(x < PAGE_OFFSET);
-		x -= PAGE_OFFSET;
-		VIRTUAL_BUG_ON(!phys_addr_valid(x));
-	}
-	return x;
-}
-EXPORT_SYMBOL(__phys_addr);
-
-bool __virt_addr_valid(unsigned long x)
-{
-	if (x >= __START_KERNEL_map) {
-		x -= __START_KERNEL_map;
-		if (x >= KERNEL_IMAGE_SIZE)
-			return false;
-		x += phys_base;
-	} else {
-		if (x < PAGE_OFFSET)
-			return false;
-		x -= PAGE_OFFSET;
-		if (!phys_addr_valid(x))
-			return false;
-	}
-
-	return pfn_valid(x >> PAGE_SHIFT);
-}
-EXPORT_SYMBOL(__virt_addr_valid);
-
-#else
-
-#ifdef CONFIG_DEBUG_VIRTUAL
-unsigned long __phys_addr(unsigned long x)
-{
-	/* VMALLOC_* aren't constants  */
-	VIRTUAL_BUG_ON(x < PAGE_OFFSET);
-	VIRTUAL_BUG_ON(__vmalloc_start_set && is_vmalloc_addr((void *) x));
-	return x - PAGE_OFFSET;
-}
-EXPORT_SYMBOL(__phys_addr);
-#endif
-
-bool __virt_addr_valid(unsigned long x)
-{
-	if (x < PAGE_OFFSET)
-		return false;
-	if (__vmalloc_start_set && is_vmalloc_addr((void *) x))
-		return false;
-	if (x >= FIXADDR_START)
-		return false;
-	return pfn_valid((x - PAGE_OFFSET) >> PAGE_SHIFT);
-}
-EXPORT_SYMBOL(__virt_addr_valid);
-
-#endif
+#include "physaddr.h"
 
 int page_is_ram(unsigned long pagenr)
 {
diff --git a/arch/x86/mm/physaddr.c b/arch/x86/mm/physaddr.c
new file mode 100644
index 0000000..d2e2735
--- /dev/null
+++ b/arch/x86/mm/physaddr.c
@@ -0,0 +1,70 @@
+#include <linux/mmdebug.h>
+#include <linux/module.h>
+#include <linux/mm.h>
+
+#include <asm/page.h>
+
+#include "physaddr.h"
+
+#ifdef CONFIG_X86_64
+
+unsigned long __phys_addr(unsigned long x)
+{
+	if (x >= __START_KERNEL_map) {
+		x -= __START_KERNEL_map;
+		VIRTUAL_BUG_ON(x >= KERNEL_IMAGE_SIZE);
+		x += phys_base;
+	} else {
+		VIRTUAL_BUG_ON(x < PAGE_OFFSET);
+		x -= PAGE_OFFSET;
+		VIRTUAL_BUG_ON(!phys_addr_valid(x));
+	}
+	return x;
+}
+EXPORT_SYMBOL(__phys_addr);
+
+bool __virt_addr_valid(unsigned long x)
+{
+	if (x >= __START_KERNEL_map) {
+		x -= __START_KERNEL_map;
+		if (x >= KERNEL_IMAGE_SIZE)
+			return false;
+		x += phys_base;
+	} else {
+		if (x < PAGE_OFFSET)
+			return false;
+		x -= PAGE_OFFSET;
+		if (!phys_addr_valid(x))
+			return false;
+	}
+
+	return pfn_valid(x >> PAGE_SHIFT);
+}
+EXPORT_SYMBOL(__virt_addr_valid);
+
+#else
+
+#ifdef CONFIG_DEBUG_VIRTUAL
+unsigned long __phys_addr(unsigned long x)
+{
+	/* VMALLOC_* aren't constants  */
+	VIRTUAL_BUG_ON(x < PAGE_OFFSET);
+	VIRTUAL_BUG_ON(__vmalloc_start_set && is_vmalloc_addr((void *) x));
+	return x - PAGE_OFFSET;
+}
+EXPORT_SYMBOL(__phys_addr);
+#endif
+
+bool __virt_addr_valid(unsigned long x)
+{
+	if (x < PAGE_OFFSET)
+		return false;
+	if (__vmalloc_start_set && is_vmalloc_addr((void *) x))
+		return false;
+	if (x >= FIXADDR_START)
+		return false;
+	return pfn_valid((x - PAGE_OFFSET) >> PAGE_SHIFT);
+}
+EXPORT_SYMBOL(__virt_addr_valid);
+
+#endif	/* CONFIG_X86_64 */
diff --git a/arch/x86/mm/physaddr.h b/arch/x86/mm/physaddr.h
new file mode 100644
index 0000000..a3cd5a0
--- /dev/null
+++ b/arch/x86/mm/physaddr.h
@@ -0,0 +1,10 @@
+#include <asm/processor.h>
+
+static inline int phys_addr_valid(resource_size_t addr)
+{
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
+	return !(addr >> boot_cpu_data.x86_phys_bits);
+#else
+	return 1;
+#endif
+}



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Xen bugfixes
@ 2009-09-10 17:16     ` Jeremy Fitzhardinge
  0 siblings, 0 replies; 11+ messages in thread
From: Jeremy Fitzhardinge @ 2009-09-10 17:16 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Xen-devel, the arch/x86 maintainers, Linux Kernel Mailing List,
	Ingo Molnar, Tejun Heo, Stable Kernel

On 09/09/09 22:16, Ingo Molnar wrote:
>> +# Make sure __phys_addr has no stackprotector
>> +nostackp := $(call cc-option, -fno-stack-protector)
>> +CFLAGS_ioremap.o		:= $(nostackp)
>> +
>>     
> Sure we could move __phys_addr into its own file and thus avoid 
> turning off stackprotector for the rest of ioremap.c?
>   

I'm not very keen on having zillions of tiny files just to cope with the
lack of per-function stackprotector disable.  I don't see any code in
ioremap.c that would really benefit from stack-protector anyway; there
are no local arrays.

At least __phys_addr and friends aren't terribly closely related to
ioremap so it would at least make some sense.  Patch below.

>> --- a/arch/x86/xen/Makefile
>> +++ b/arch/x86/xen/Makefile
>> @@ -8,6 +8,7 @@ endif
>>  # Make sure early boot has no stackprotector
>>  nostackp := $(call cc-option, -fno-stack-protector)
>>  CFLAGS_enlighten.o		:= $(nostackp)
>> +CFLAGS_mmu.o			:= $(nostackp)
>>     
> A similar argument could be made here - what proportion of mmu.c is 
> affected?
>   

More.  It would be a fairly arbitrary chunk of code to split out into a
separate file.

> Also, once the commits have hit upstream feel free bounce them to 
> stable@kernel.org - they dont have Cc: <stable@kernel.org> tags for 
> automatic back-merging requests. The fixes narrowly missed v2.6.31.
>   

Will do.

Thanks,
    J

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Subject: [PATCH] x86: split __phys_addr out into separate file

Split __phys_addr out into its own file so we can disable
-fstack-protector in a fine-grained fashion.  Also it doesn't
have terribly much to do with the rest of ioremap.c.

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

diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile
index 72bb3a2..9b5a9f5 100644
--- a/arch/x86/mm/Makefile
+++ b/arch/x86/mm/Makefile
@@ -1,9 +1,9 @@
 obj-y	:=  init.o init_$(BITS).o fault.o ioremap.o extable.o pageattr.o mmap.o \
-	    pat.o pgtable.o gup.o
+	    pat.o pgtable.o physaddr.o gup.o
 
 # Make sure __phys_addr has no stackprotector
 nostackp := $(call cc-option, -fno-stack-protector)
-CFLAGS_ioremap.o		:= $(nostackp)
+CFLAGS_physaddr.o		:= $(nostackp)
 
 obj-$(CONFIG_SMP)		+= tlb.o
 
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 8a45093..04e1ad6 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -22,77 +22,7 @@
 #include <asm/pgalloc.h>
 #include <asm/pat.h>
 
-static inline int phys_addr_valid(resource_size_t addr)
-{
-#ifdef CONFIG_PHYS_ADDR_T_64BIT
-	return !(addr >> boot_cpu_data.x86_phys_bits);
-#else
-	return 1;
-#endif
-}
-
-#ifdef CONFIG_X86_64
-
-unsigned long __phys_addr(unsigned long x)
-{
-	if (x >= __START_KERNEL_map) {
-		x -= __START_KERNEL_map;
-		VIRTUAL_BUG_ON(x >= KERNEL_IMAGE_SIZE);
-		x += phys_base;
-	} else {
-		VIRTUAL_BUG_ON(x < PAGE_OFFSET);
-		x -= PAGE_OFFSET;
-		VIRTUAL_BUG_ON(!phys_addr_valid(x));
-	}
-	return x;
-}
-EXPORT_SYMBOL(__phys_addr);
-
-bool __virt_addr_valid(unsigned long x)
-{
-	if (x >= __START_KERNEL_map) {
-		x -= __START_KERNEL_map;
-		if (x >= KERNEL_IMAGE_SIZE)
-			return false;
-		x += phys_base;
-	} else {
-		if (x < PAGE_OFFSET)
-			return false;
-		x -= PAGE_OFFSET;
-		if (!phys_addr_valid(x))
-			return false;
-	}
-
-	return pfn_valid(x >> PAGE_SHIFT);
-}
-EXPORT_SYMBOL(__virt_addr_valid);
-
-#else
-
-#ifdef CONFIG_DEBUG_VIRTUAL
-unsigned long __phys_addr(unsigned long x)
-{
-	/* VMALLOC_* aren't constants  */
-	VIRTUAL_BUG_ON(x < PAGE_OFFSET);
-	VIRTUAL_BUG_ON(__vmalloc_start_set && is_vmalloc_addr((void *) x));
-	return x - PAGE_OFFSET;
-}
-EXPORT_SYMBOL(__phys_addr);
-#endif
-
-bool __virt_addr_valid(unsigned long x)
-{
-	if (x < PAGE_OFFSET)
-		return false;
-	if (__vmalloc_start_set && is_vmalloc_addr((void *) x))
-		return false;
-	if (x >= FIXADDR_START)
-		return false;
-	return pfn_valid((x - PAGE_OFFSET) >> PAGE_SHIFT);
-}
-EXPORT_SYMBOL(__virt_addr_valid);
-
-#endif
+#include "physaddr.h"
 
 int page_is_ram(unsigned long pagenr)
 {
diff --git a/arch/x86/mm/physaddr.c b/arch/x86/mm/physaddr.c
new file mode 100644
index 0000000..d2e2735
--- /dev/null
+++ b/arch/x86/mm/physaddr.c
@@ -0,0 +1,70 @@
+#include <linux/mmdebug.h>
+#include <linux/module.h>
+#include <linux/mm.h>
+
+#include <asm/page.h>
+
+#include "physaddr.h"
+
+#ifdef CONFIG_X86_64
+
+unsigned long __phys_addr(unsigned long x)
+{
+	if (x >= __START_KERNEL_map) {
+		x -= __START_KERNEL_map;
+		VIRTUAL_BUG_ON(x >= KERNEL_IMAGE_SIZE);
+		x += phys_base;
+	} else {
+		VIRTUAL_BUG_ON(x < PAGE_OFFSET);
+		x -= PAGE_OFFSET;
+		VIRTUAL_BUG_ON(!phys_addr_valid(x));
+	}
+	return x;
+}
+EXPORT_SYMBOL(__phys_addr);
+
+bool __virt_addr_valid(unsigned long x)
+{
+	if (x >= __START_KERNEL_map) {
+		x -= __START_KERNEL_map;
+		if (x >= KERNEL_IMAGE_SIZE)
+			return false;
+		x += phys_base;
+	} else {
+		if (x < PAGE_OFFSET)
+			return false;
+		x -= PAGE_OFFSET;
+		if (!phys_addr_valid(x))
+			return false;
+	}
+
+	return pfn_valid(x >> PAGE_SHIFT);
+}
+EXPORT_SYMBOL(__virt_addr_valid);
+
+#else
+
+#ifdef CONFIG_DEBUG_VIRTUAL
+unsigned long __phys_addr(unsigned long x)
+{
+	/* VMALLOC_* aren't constants  */
+	VIRTUAL_BUG_ON(x < PAGE_OFFSET);
+	VIRTUAL_BUG_ON(__vmalloc_start_set && is_vmalloc_addr((void *) x));
+	return x - PAGE_OFFSET;
+}
+EXPORT_SYMBOL(__phys_addr);
+#endif
+
+bool __virt_addr_valid(unsigned long x)
+{
+	if (x < PAGE_OFFSET)
+		return false;
+	if (__vmalloc_start_set && is_vmalloc_addr((void *) x))
+		return false;
+	if (x >= FIXADDR_START)
+		return false;
+	return pfn_valid((x - PAGE_OFFSET) >> PAGE_SHIFT);
+}
+EXPORT_SYMBOL(__virt_addr_valid);
+
+#endif	/* CONFIG_X86_64 */
diff --git a/arch/x86/mm/physaddr.h b/arch/x86/mm/physaddr.h
new file mode 100644
index 0000000..a3cd5a0
--- /dev/null
+++ b/arch/x86/mm/physaddr.h
@@ -0,0 +1,10 @@
+#include <asm/processor.h>
+
+static inline int phys_addr_valid(resource_size_t addr)
+{
+#ifdef CONFIG_PHYS_ADDR_T_64BIT
+	return !(addr >> boot_cpu_data.x86_phys_bits);
+#else
+	return 1;
+#endif
+}

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Xen bugfixes
  2009-09-10 17:16     ` Jeremy Fitzhardinge
@ 2009-09-10 17:26       ` Ingo Molnar
  -1 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2009-09-10 17:26 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Ingo Molnar, the arch/x86 maintainers, Linux Kernel Mailing List,
	Xen-devel, Stable Kernel, Tejun Heo


* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> On 09/09/09 22:16, Ingo Molnar wrote:
> >> +# Make sure __phys_addr has no stackprotector
> >> +nostackp := $(call cc-option, -fno-stack-protector)
> >> +CFLAGS_ioremap.o		:= $(nostackp)
> >> +
> >>     
> > Sure we could move __phys_addr into its own file and thus avoid 
> > turning off stackprotector for the rest of ioremap.c?
> >   
> 
> I'm not very keen on having zillions of tiny files just to cope 
> with the lack of per-function stackprotector disable.  I don't see 
> any code in ioremap.c that would really benefit from 
> stack-protector anyway; there are no local arrays.
> 
> At least __phys_addr and friends aren't terribly closely related 
> to ioremap so it would at least make some sense.

Agreed, i wouldnt do it just for the stackprotector benefit (it's 
really stupid that GCC does not allow per function exceptions) - but 
here __phys_addr() looked out of place a bit.

> [...]  Patch below.

Looks like a nice cleanup. Mind sticking it into your next pull 
request?

> >> +CFLAGS_mmu.o			:= $(nostackp)
> >>     
> > A similar argument could be made here - what proportion of mmu.c is 
> > affected?
> 
> More.  It would be a fairly arbitrary chunk of code to split out 
> into a separate file.

Ok - i'd not do it then.

> > Also, once the commits have hit upstream feel free bounce them 
> > to stable@kernel.org - they dont have Cc: <stable@kernel.org> 
> > tags for automatic back-merging requests. The fixes narrowly 
> > missed v2.6.31.
> >   
> 
> Will do.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Xen bugfixes
@ 2009-09-10 17:26       ` Ingo Molnar
  0 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2009-09-10 17:26 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Xen-devel, the arch/x86 maintainers, Linux Kernel Mailing List,
	Ingo Molnar, Tejun Heo, Stable Kernel


* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> On 09/09/09 22:16, Ingo Molnar wrote:
> >> +# Make sure __phys_addr has no stackprotector
> >> +nostackp := $(call cc-option, -fno-stack-protector)
> >> +CFLAGS_ioremap.o		:= $(nostackp)
> >> +
> >>     
> > Sure we could move __phys_addr into its own file and thus avoid 
> > turning off stackprotector for the rest of ioremap.c?
> >   
> 
> I'm not very keen on having zillions of tiny files just to cope 
> with the lack of per-function stackprotector disable.  I don't see 
> any code in ioremap.c that would really benefit from 
> stack-protector anyway; there are no local arrays.
> 
> At least __phys_addr and friends aren't terribly closely related 
> to ioremap so it would at least make some sense.

Agreed, i wouldnt do it just for the stackprotector benefit (it's 
really stupid that GCC does not allow per function exceptions) - but 
here __phys_addr() looked out of place a bit.

> [...]  Patch below.

Looks like a nice cleanup. Mind sticking it into your next pull 
request?

> >> +CFLAGS_mmu.o			:= $(nostackp)
> >>     
> > A similar argument could be made here - what proportion of mmu.c is 
> > affected?
> 
> More.  It would be a fairly arbitrary chunk of code to split out 
> into a separate file.

Ok - i'd not do it then.

> > Also, once the commits have hit upstream feel free bounce them 
> > to stable@kernel.org - they dont have Cc: <stable@kernel.org> 
> > tags for automatic back-merging requests. The fixes narrowly 
> > missed v2.6.31.
> >   
> 
> Will do.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Xen bugfixes
  2009-09-10 17:26       ` Ingo Molnar
@ 2009-09-10 19:02         ` Jeremy Fitzhardinge
  -1 siblings, 0 replies; 11+ messages in thread
From: Jeremy Fitzhardinge @ 2009-09-10 19:02 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Ingo Molnar, the arch/x86 maintainers, Linux Kernel Mailing List,
	Xen-devel, Stable Kernel, Tejun Heo

On 09/10/09 10:26, Ingo Molnar wrote:
> Looks like a nice cleanup. Mind sticking it into your next pull 
> request?
>   

The following changes since commit 2496afbf1e50c70f80992656bcb730c8583ddac3:
  Yang Xiaowei (1):
        xen: use stronger barrier after unlocking lock

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git bugfix
Jeremy Fitzhardinge (1):
      x86: split __phys_addr out into separate file

 arch/x86/mm/Makefile   |    4 +-
 arch/x86/mm/ioremap.c  |   72 +-----------------------------------------------
 arch/x86/mm/physaddr.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/mm/physaddr.h |   10 ++++++
 4 files changed, 83 insertions(+), 73 deletions(-)
 create mode 100644 arch/x86/mm/physaddr.c
 create mode 100644 arch/x86/mm/physaddr.h

	J


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] Xen bugfixes
@ 2009-09-10 19:02         ` Jeremy Fitzhardinge
  0 siblings, 0 replies; 11+ messages in thread
From: Jeremy Fitzhardinge @ 2009-09-10 19:02 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Xen-devel, the arch/x86 maintainers, Linux Kernel Mailing List,
	Ingo Molnar, Tejun Heo, Stable Kernel

On 09/10/09 10:26, Ingo Molnar wrote:
> Looks like a nice cleanup. Mind sticking it into your next pull 
> request?
>   

The following changes since commit 2496afbf1e50c70f80992656bcb730c8583ddac3:
  Yang Xiaowei (1):
        xen: use stronger barrier after unlocking lock

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git bugfix
Jeremy Fitzhardinge (1):
      x86: split __phys_addr out into separate file

 arch/x86/mm/Makefile   |    4 +-
 arch/x86/mm/ioremap.c  |   72 +-----------------------------------------------
 arch/x86/mm/physaddr.c |   70 ++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/mm/physaddr.h |   10 ++++++
 4 files changed, 83 insertions(+), 73 deletions(-)
 create mode 100644 arch/x86/mm/physaddr.c
 create mode 100644 arch/x86/mm/physaddr.h

	J

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [GIT PULL] Xen bugfixes
@ 2009-12-03 23:46 Jeremy Fitzhardinge
  0 siblings, 0 replies; 11+ messages in thread
From: Jeremy Fitzhardinge @ 2009-12-03 23:46 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Xen-devel, Linux Kernel Mailing List, Stable Kernel

Hi Linus,

Please pull this set of bugfixes, mostly to do with Xen domain 
save/restore.  They're all stable material as well.

(Note, if you pull quickly from git.kernel.org, you may pick up some 
more changes that I rolled back pending maintainer approval; pulling 
from master might be safer.)

Thanks,
     J

The following changes since commit 82d6469916c6fcfa345636a49004c9d1753905d1:
   Jeremy Fitzhardinge (1):
         xen: mask extended topology info in cpuid

are available in the git repository at:

   git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git bugfix

Ian Campbell (8):
       xen: re-register runstate area earlier on resume.
       xen: correctly restore pfn_to_mfn_list_list after resume
       xen: register timer interrupt with IRQF_TIMER
       xen: register runstate on secondary CPUs
       xen: call clock resume notifier on all CPUs
       xen: don't leak IRQs over suspend/resume.
       xen: improve error handling in do_suspend.
       xen: explicitly create/destroy stop_machine workqueues outside suspend/resume region.

Jeremy Fitzhardinge (5):
       xen/xenbus: make DEVICE_ATTR()s static
       xen: restore runstate_info even if !have_vcpu_info_placement
       xen: register runstate info for boot CPU early
       xen: don't call dpm_resume_noirq() with interrupts disabled.
       xen: use iret for return from 64b kernel to 32b usermode

Paolo Bonzini (3):
       xen: fix is_disconnected_device/exists_disconnected_device
       xen: improvement to wait_for_devices()
       xen: wait up to 5 minutes for device connetion

  arch/x86/xen/enlighten.c          |   27 ++++++++++----------
  arch/x86/xen/mmu.c                |    2 +-
  arch/x86/xen/smp.c                |    1 +
  arch/x86/xen/suspend.c            |   17 ++++++++++++-
  arch/x86/xen/time.c               |    7 ++---
  arch/x86/xen/xen-asm_64.S         |    4 +-
  arch/x86/xen/xen-ops.h            |    2 +
  drivers/xen/events.c              |    3 ++
  drivers/xen/manage.c              |   37 ++++++++++++++++++----------
  drivers/xen/xenbus/xenbus_probe.c |   48 ++++++++++++++++++++++++------------
  10 files changed, 98 insertions(+), 50 deletions(-)



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2009-12-03 23:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-09 23:51 [GIT PULL] Xen bugfixes Jeremy Fitzhardinge
2009-09-09 23:51 ` Jeremy Fitzhardinge
2009-09-10  5:16 ` Ingo Molnar
2009-09-10  5:16   ` Ingo Molnar
2009-09-10 17:16   ` Jeremy Fitzhardinge
2009-09-10 17:16     ` Jeremy Fitzhardinge
2009-09-10 17:26     ` Ingo Molnar
2009-09-10 17:26       ` Ingo Molnar
2009-09-10 19:02       ` Jeremy Fitzhardinge
2009-09-10 19:02         ` Jeremy Fitzhardinge
2009-12-03 23:46 Jeremy Fitzhardinge

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.