[PATCH 8/8] x86_64: Support for new UV apic

[PATCH 8/8] x86_64: Support for new UV apic

[Jack Steiner]

UV supports really big systems. So big, in fact, that the APICID register
does not contain enough bits to contain an APICID that is unique across all
cpus.

The UV BIOS supports 3 APICID modes:

	- legacy mode. This mode uses the old APIC mode where
	  APICID is in bits [31:24] of the APICID register.

	- x2apic mode. This mode is whitebox-compatible. APICIDs
	  are unique across all cpus. Standard x2apic APIC operations
	  (Intel-defined) can be used for IPIs. The node identifier
	  fits within the Intel-defined portion of the APICID register.

	- x2apic-uv mode. In this mode, the APICIDs on each node have
	  unique IDs, but IDs on different node are not unique. For example,
	  if each mode has 32 cpus, the APICIDs on each node might be
	  0 - 31. Every node has the same set of IDs.
	  The UV hub is used to route IPIs/interrupts to the correct node.
	  Traditional APIC operations WILL NOT WORK.

In x2apic-uv mode, the ACPI tables all contain a full unique ID (note:
exact bit layout still changing but the following is close):
	
	nnnnnnnnnnlc0cch
		n = unique node number
		l = socket number on board
		c = core
		h = hyperthread
		
Only the "lc0cch" bits are written to the APICID register. The remaining bits are
supplied by having the get_apic_id() function "OR" the extra bits into the value
read from the APICID register. (Hmmm.. why not keep the ENTIRE APICID register
in per-cpu data....)

The x2apic-uv mode is recognized by the MADT table containing:
	  oem_id = "SGI"
	  oem_table_id = "UV-X"
	
Based on:
        git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86.git

Signed-off-by: Jack Steiner <steiner@sgi.com>

---
 arch/x86/kernel/Makefile         |    2 
 arch/x86/kernel/apic_64.c        |    2 
 arch/x86/kernel/genapic_64.c     |   18 ++
 arch/x86/kernel/genx2apic_uv_x.c |  245 +++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/setup64.c        |    4 
 arch/x86/kernel/smpboot.c        |    5 
 include/asm-x86/genapic_64.h     |    5 
 include/asm-x86/smp.h            |    5 
 8 files changed, 285 insertions(+), 1 deletion(-)

Index: linux/arch/x86/kernel/genapic_64.c
===================================================================
--- linux.orig/arch/x86/kernel/genapic_64.c	2008-03-28 13:06:07.000000000 -0500
+++ linux/arch/x86/kernel/genapic_64.c	2008-03-28 13:06:12.000000000 -0500
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/ctype.h>
 #include <linux/init.h>
+#include <linux/hardirq.h>
 
 #include <asm/smp.h>
 #include <asm/ipi.h>
@@ -32,6 +33,7 @@ void *x86_cpu_to_apicid_early_ptr;
 #endif
 DEFINE_PER_CPU(u16, x86_cpu_to_apicid) = BAD_APICID;
 EXPORT_PER_CPU_SYMBOL(x86_cpu_to_apicid);
+DEFINE_PER_CPU(int, x2apic_extra_bits);
 
 struct genapic __read_mostly *genapic = &apic_flat;
 
@@ -42,6 +44,9 @@ static enum uv_system_type uv_system_typ
  */
 void __init setup_apic_routing(void)
 {
+	if (uv_system_type == UV_NON_UNIQUE_APIC)
+		genapic = &apic_x2apic_uv_x;
+	else
 #ifdef CONFIG_ACPI
 	/*
 	 * Quirk: some x86_64 machines can only use physical APIC mode
@@ -82,6 +87,19 @@ int __init acpi_madt_oem_check(char *oem
 	return 0;
 }
 
+unsigned int read_apic_id(void)
+{
+	unsigned int id;
+
+	WARN_ON(preemptible());
+	id = apic_read(APIC_ID);
+	if (uv_system_type >= UV_X2APIC)
+		id  |= __get_cpu_var(x2apic_extra_bits);
+	else
+		id = (id >> 24) & 0xFFu;;
+	return id;
+}
+
 enum uv_system_type get_uv_system_type(void)
 {
 	return uv_system_type;
Index: linux/arch/x86/kernel/genx2apic_uv_x.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux/arch/x86/kernel/genx2apic_uv_x.c	2008-03-28 13:06:12.000000000 -0500
@@ -0,0 +1,245 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * SGI UV APIC functions (note: not an Intel compatible APIC)
+ *
+ * Copyright (C) 2007 Silicon Graphics, Inc. All rights reserved.
+ */
+
+#include <linux/threads.h>
+#include <linux/cpumask.h>
+#include <linux/string.h>
+#include <linux/kernel.h>
+#include <linux/ctype.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/bootmem.h>
+#include <linux/module.h>
+#include <asm/smp.h>
+#include <asm/ipi.h>
+#include <asm/genapic.h>
+#include <asm/uv/uv_mmrs.h>
+#include <asm/uv/uv_hub.h>
+
+DEFINE_PER_CPU(struct uv_hub_info_s, __uv_hub_info);
+EXPORT_PER_CPU_SYMBOL_GPL(__uv_hub_info);
+
+struct uv_blade_info *uv_blade_info;
+EXPORT_SYMBOL_GPL(uv_blade_info);
+
+short *uv_node_to_blade;
+EXPORT_SYMBOL_GPL(uv_node_to_blade);
+
+short *uv_cpu_to_blade;
+EXPORT_SYMBOL_GPL(uv_cpu_to_blade);
+
+short uv_possible_blades;
+EXPORT_SYMBOL_GPL(uv_possible_blades);
+
+/* Start with all IRQs pointing to boot CPU.  IRQ balancing will shift them. */
+
+static cpumask_t uv_target_cpus(void)
+{
+	return cpumask_of_cpu(0);
+}
+
+static cpumask_t uv_vector_allocation_domain(int cpu)
+{
+	cpumask_t domain = CPU_MASK_NONE;
+	cpu_set(cpu, domain);
+	return domain;
+}
+
+int uv_wakeup_secondary(int phys_apicid, unsigned int start_rip)
+{
+	unsigned long val;
+	int nasid;
+
+	nasid = uv_apicid_to_nasid(phys_apicid);
+	val = (1UL << UVH_IPI_INT_SEND_SHFT) |
+	    (phys_apicid << UVH_IPI_INT_APIC_ID_SHFT) |
+	    (((long)start_rip << UVH_IPI_INT_VECTOR_SHFT) >> 12) |
+	    (6 << UVH_IPI_INT_DELIVERY_MODE_SHFT);
+	uv_write_global_mmr64(nasid, UVH_IPI_INT, val);
+	return 0;
+}
+
+static void uv_send_IPI_one(int cpu, int vector)
+{
+	unsigned long val, apicid;
+	int nasid;
+
+	apicid = per_cpu(x86_cpu_to_apicid, cpu); /* ZZZ - cache node-local ? */
+	nasid = uv_apicid_to_nasid(apicid);
+	val =
+	    (1UL << UVH_IPI_INT_SEND_SHFT) | (apicid <<
+					      UVH_IPI_INT_APIC_ID_SHFT) |
+	    (vector << UVH_IPI_INT_VECTOR_SHFT);
+	uv_write_global_mmr64(nasid, UVH_IPI_INT, val);
+	printk(KERN_DEBUG
+	     "UV: IPI to cpu %d, apicid 0x%lx, vec %d, nasid%d, val 0x%lx\n",
+	     cpu, apicid, vector, nasid, val);
+}
+
+static void uv_send_IPI_mask(cpumask_t mask, int vector)
+{
+	unsigned int cpu;
+
+	for (cpu = 0; cpu < NR_CPUS; ++cpu)
+		if (cpu_isset(cpu, mask))
+			uv_send_IPI_one(cpu, vector);
+}
+
+static void uv_send_IPI_allbutself(int vector)
+{
+	cpumask_t mask = cpu_online_map;
+
+	cpu_clear(smp_processor_id(), mask);
+
+	if (!cpus_empty(mask))
+		uv_send_IPI_mask(mask, vector);
+}
+
+static void uv_send_IPI_all(int vector)
+{
+	uv_send_IPI_mask(cpu_online_map, vector);
+}
+
+static int uv_apic_id_registered(void)
+{
+	return 1;
+}
+
+static unsigned int uv_cpu_mask_to_apicid(cpumask_t cpumask)
+{
+	int cpu;
+
+	/*
+	 * We're using fixed IRQ delivery, can only return one phys APIC ID.
+	 * May as well be the first.
+	 */
+	cpu = first_cpu(cpumask);
+	if ((unsigned)cpu < NR_CPUS)
+		return per_cpu(x86_cpu_to_apicid, cpu);
+	else
+		return BAD_APICID;
+}
+
+static unsigned int phys_pkg_id(int index_msb)
+{
+	return GET_APIC_ID(read_apic_id()) >> index_msb;
+}
+
+#ifdef ZZZ		/* Needs x2apic patch */
+static void uv_send_IPI_self(int vector)
+{
+	apic_write(APIC_SELF_IPI, vector);
+}
+#endif
+
+struct genapic apic_x2apic_uv_x = {
+	.name = "UV large system",
+	.int_delivery_mode = dest_Fixed,
+	.int_dest_mode = (APIC_DEST_PHYSICAL != 0),
+	.target_cpus = uv_target_cpus,
+	.vector_allocation_domain = uv_vector_allocation_domain,/* Fixme ZZZ */
+	.apic_id_registered = uv_apic_id_registered,
+	.send_IPI_all = uv_send_IPI_all,
+	.send_IPI_allbutself = uv_send_IPI_allbutself,
+	.send_IPI_mask = uv_send_IPI_mask,
+	/* ZZZ.send_IPI_self = uv_send_IPI_self, */
+	.cpu_mask_to_apicid = uv_cpu_mask_to_apicid,
+	.phys_pkg_id = phys_pkg_id,	/* Fixme ZZZ */
+};
+
+static __cpuinit void set_x2apic_extra_bits(int nasid)
+{
+	__get_cpu_var(x2apic_extra_bits) = ((nasid >> 1) << 6);
+}
+
+/*
+ * Called on boot cpu.
+ */
+static __init void uv_system_init(void)
+{
+	union uvh_si_addr_map_config_u m_n_config;
+	int bytes, nid, cpu, lcpu, nasid, last_nasid, blade;
+	unsigned long mmr_base;
+
+	m_n_config.v = uv_read_local_mmr(UVH_SI_ADDR_MAP_CONFIG);
+	mmr_base =
+	    uv_read_local_mmr(UVH_RH_GAM_MMR_OVERLAY_CONFIG_MMR) &
+	    ~UV_MMR_ENABLE;
+	printk(KERN_DEBUG "UV: global MMR base 0x%lx\n", mmr_base);
+
+	last_nasid = -1;
+	for_each_possible_cpu(cpu) {
+		nid = cpu_to_node(cpu);
+		nasid = uv_apicid_to_nasid(per_cpu(x86_cpu_to_apicid, cpu));
+		if (nasid != last_nasid)
+			uv_possible_blades++;
+		last_nasid = nasid;
+	}
+	printk(KERN_DEBUG "UV: Found %d blades\n", uv_num_possible_blades());
+
+	bytes = sizeof(struct uv_blade_info) * uv_num_possible_blades();
+	uv_blade_info = alloc_bootmem_pages(bytes);
+
+	bytes = sizeof(uv_node_to_blade[0]) * num_possible_nodes();
+	uv_node_to_blade = alloc_bootmem_pages(bytes);
+	memset(uv_node_to_blade, 255, bytes);
+
+	bytes = sizeof(uv_cpu_to_blade[0]) * num_possible_cpus();
+	uv_cpu_to_blade = alloc_bootmem_pages(bytes);
+	memset(uv_cpu_to_blade, 255, bytes);
+
+	last_nasid = -1;
+	blade = -1;
+	lcpu = -1;
+	for_each_possible_cpu(cpu) {
+		nid = cpu_to_node(cpu);
+		nasid = uv_apicid_to_nasid(per_cpu(x86_cpu_to_apicid, cpu));
+		if (nasid != last_nasid) {
+			blade++;
+			lcpu = -1;
+			uv_blade_info[blade].nr_posible_cpus = 0;
+			uv_blade_info[blade].nr_online_cpus = 0;
+		}
+		last_nasid = nasid;
+		lcpu++;
+
+		uv_cpu_hub_info(cpu)->m_val = m_n_config.s.m_skt;
+		uv_cpu_hub_info(cpu)->n_val = m_n_config.s.n_skt;
+		uv_cpu_hub_info(cpu)->numa_blade_id = blade;
+		uv_cpu_hub_info(cpu)->blade_processor_id = lcpu;
+		uv_cpu_hub_info(cpu)->local_nasid = nasid;
+		uv_cpu_hub_info(cpu)->gnode_upper =
+		    nasid & ~((1 << uv_hub_info->n_val) - 1);
+		uv_cpu_hub_info(cpu)->global_mmr_base = mmr_base;
+		uv_cpu_hub_info(cpu)->coherency_domain_number = 0;/* ZZZ */
+		uv_blade_info[blade].nasid = nasid;
+		uv_blade_info[blade].nr_posible_cpus++;
+		uv_node_to_blade[nid] = blade;
+		uv_cpu_to_blade[cpu] = blade;
+
+		printk(KERN_DEBUG "UV cpu %d, apicid 0x%x, nasid %d, nid %d\n",
+		       cpu, per_cpu(x86_cpu_to_apicid, cpu), nasid, nid);
+		printk(KERN_DEBUG "UV   lcpu %d, blade %d\n", lcpu, blade);
+	}
+}
+
+/*
+ * Called on each cpu to initialize the per_cpu UV data area.
+ */
+void __cpuinit uv_cpu_init(void)
+{
+	if (!uv_node_to_blade)
+		uv_system_init();
+
+	uv_blade_info[uv_numa_blade_id()].nr_online_cpus++;
+
+	if (get_uv_system_type() == UV_NON_UNIQUE_APIC)
+		set_x2apic_extra_bits(uv_hub_info->local_nasid);
+}
Index: linux/arch/x86/kernel/setup64.c
===================================================================
--- linux.orig/arch/x86/kernel/setup64.c	2008-03-28 13:00:22.000000000 -0500
+++ linux/arch/x86/kernel/setup64.c	2008-03-28 13:06:12.000000000 -0500
@@ -24,6 +24,7 @@
 #include <asm/proto.h>
 #include <asm/sections.h>
 #include <asm/setup.h>
+#include <asm/genapic.h>
 
 #ifndef CONFIG_DEBUG_BOOT_PARAMS
 struct boot_params __initdata boot_params;
@@ -280,4 +281,7 @@ void __cpuinit cpu_init (void)
 	fpu_init(); 
 
 	raw_local_save_flags(kernel_eflags);
+
+	if (is_uv_system())
+		uv_cpu_init();
 }
Index: linux/arch/x86/kernel/Makefile
===================================================================
--- linux.orig/arch/x86/kernel/Makefile	2008-03-28 13:00:22.000000000 -0500
+++ linux/arch/x86/kernel/Makefile	2008-03-28 13:06:12.000000000 -0500
@@ -97,7 +97,7 @@ obj-$(CONFIG_KMEMCHECK)		+= kmemcheck_32
 ###
 # 64 bit specific files
 ifeq ($(CONFIG_X86_64),y)
-        obj-y				+= genapic_64.o genapic_flat_64.o
+        obj-y				+= genapic_64.o genapic_flat_64.o genx2apic_uv_x.o
         obj-$(CONFIG_X86_PM_TIMER)	+= pmtimer_64.o
         obj-$(CONFIG_AUDIT)		+= audit_64.o
 
Index: linux/include/asm-x86/genapic_64.h
===================================================================
--- linux.orig/include/asm-x86/genapic_64.h	2008-03-28 13:06:07.000000000 -0500
+++ linux/include/asm-x86/genapic_64.h	2008-03-28 13:06:12.000000000 -0500
@@ -39,4 +39,9 @@ enum uv_system_type {UV_NONE, UV_LEGACY_
 extern enum uv_system_type get_uv_system_type(void);
 extern int is_uv_system(void);
 
+extern struct genapic apic_x2apic_uv_x;
+DECLARE_PER_CPU(int, x2apic_extra_bits);
+extern void uv_cpu_init(void);
+extern int uv_wakeup_secondary(int phys_apicid, unsigned int start_rip);
+
 #endif
Index: linux/arch/x86/kernel/apic_64.c
===================================================================
--- linux.orig/arch/x86/kernel/apic_64.c	2008-03-28 13:00:22.000000000 -0500
+++ linux/arch/x86/kernel/apic_64.c	2008-03-28 13:06:12.000000000 -0500
@@ -738,6 +738,7 @@ void __cpuinit setup_local_APIC(void)
 	unsigned int value;
 	int i, j;
 
+	preempt_disable();
 	value = apic_read(APIC_LVR);
 
 	BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);
@@ -831,6 +832,7 @@ void __cpuinit setup_local_APIC(void)
 	else
 		value = APIC_DM_NMI | APIC_LVT_MASKED;
 	apic_write(APIC_LVT1, value);
+	preempt_enable();
 }
 
 void __cpuinit lapic_setup_esr(void)
Index: linux/arch/x86/kernel/smpboot.c
===================================================================
--- linux.orig/arch/x86/kernel/smpboot.c	2008-03-28 13:00:22.000000000 -0500
+++ linux/arch/x86/kernel/smpboot.c	2008-03-28 13:06:12.000000000 -0500
@@ -1101,6 +1101,7 @@ static __init void disable_smp(void)
  */
 static int __init smp_sanity_check(unsigned max_cpus)
 {
+	preempt_disable();
 	if (!physid_isset(hard_smp_processor_id(), phys_cpu_present_map)) {
 		printk(KERN_WARNING "weird, boot CPU (#%d) not listed"
 				    "by the BIOS.\n", hard_smp_processor_id());
@@ -1112,6 +1113,7 @@ static int __init smp_sanity_check(unsig
 	 * get out of here now!
 	 */
 	if (!smp_found_config && !acpi_lapic) {
+		preempt_enable();
 		printk(KERN_NOTICE "SMP motherboard not detected.\n");
 		disable_smp();
 		if (APIC_init_uniprocessor())
@@ -1130,6 +1132,7 @@ static int __init smp_sanity_check(unsig
 			boot_cpu_physical_apicid);
 		physid_set(hard_smp_processor_id(), phys_cpu_present_map);
 	}
+	preempt_enable();
 
 	/*
 	 * If we couldn't find a local APIC, then get out of here now!
@@ -1205,11 +1208,13 @@ void __init native_smp_prepare_cpus(unsi
 		return;
 	}
 
+	preempt_disable();
 	if (GET_APIC_ID(read_apic_id()) != boot_cpu_physical_apicid) {
 		panic("Boot APIC ID in local APIC unexpected (%d vs %d)",
 		     GET_APIC_ID(read_apic_id()), boot_cpu_physical_apicid);
 		/* Or can we switch back to PIC here? */
 	}
+	preempt_enable();
 
 #ifdef CONFIG_X86_32
 	connect_bsp_APIC();
Index: linux/include/asm-x86/smp.h
===================================================================
--- linux.orig/include/asm-x86/smp.h	2008-03-28 13:00:22.000000000 -0500
+++ linux/include/asm-x86/smp.h	2008-03-28 13:06:58.000000000 -0500
@@ -179,10 +179,15 @@ static inline int logical_smp_processor_
 	return GET_APIC_LOGICAL_ID(*(u32 *)(APIC_BASE + APIC_LDR));
 }
 
+#ifdef CONFIG_X86_32_SMP
 static inline unsigned int read_apic_id(void)
 {
 	return *(u32 *)(APIC_BASE + APIC_ID);
 }
+#else
+extern unsigned int read_apic_id(void);
+#endif
+
 
 # ifdef APIC_DEFINITION
 extern int hard_smp_processor_id(void);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Ingo Molnar]

* Jack Steiner <steiner@sgi.com> wrote:

> Index: linux/arch/x86/kernel/apic_64.c
> ===================================================================
> --- linux.orig/arch/x86/kernel/apic_64.c	2008-03-28 13:00:22.000000000 -0500
> +++ linux/arch/x86/kernel/apic_64.c	2008-03-28 13:06:12.000000000 -0500
> @@ -738,6 +738,7 @@ void __cpuinit setup_local_APIC(void)
>  	unsigned int value;
>  	int i, j;
>  
> +	preempt_disable();
>  	value = apic_read(APIC_LVR);
>  
>  	BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);
> @@ -831,6 +832,7 @@ void __cpuinit setup_local_APIC(void)
>  	else
>  		value = APIC_DM_NMI | APIC_LVT_MASKED;
>  	apic_write(APIC_LVT1, value);
> +	preempt_enable();
>  }

hm, this looks a bit weird - why are all the preempt-disable/enable 
calls needed?

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Jack Steiner]

On Fri, Mar 28, 2008 at 09:15:32PM +0100, Ingo Molnar wrote:
> 
> * Jack Steiner <steiner@sgi.com> wrote:
> 
> > Index: linux/arch/x86/kernel/apic_64.c
> > ===================================================================
> > --- linux.orig/arch/x86/kernel/apic_64.c	2008-03-28 13:00:22.000000000 -0500
> > +++ linux/arch/x86/kernel/apic_64.c	2008-03-28 13:06:12.000000000 -0500
> > @@ -738,6 +738,7 @@ void __cpuinit setup_local_APIC(void)
> >  	unsigned int value;
> >  	int i, j;
> >  
> > +	preempt_disable();
> >  	value = apic_read(APIC_LVR);
> >  
> >  	BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);
> > @@ -831,6 +832,7 @@ void __cpuinit setup_local_APIC(void)
> >  	else
> >  		value = APIC_DM_NMI | APIC_LVT_MASKED;
> >  	apic_write(APIC_LVT1, value);
> > +	preempt_enable();
> >  }
> 
> hm, this looks a bit weird - why are all the preempt-disable/enable 
> calls needed?

The first patch had a preempt disable/enable in the function
that reads apicid (see read_apic_id() in arch/x86/kernel/genapic_64.c).
This seemed necessary since large system generate an apicid by reading
the live id & concatenating it with extra bits.

One of the review comments suggested that I change the preempt to a WARN()
since reading apic_id really should be done with preemtion disabled. The
added code eliminates the warnings.


--- jack

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Ingo Molnar]

* Jack Steiner <steiner@sgi.com> wrote:

> On Fri, Mar 28, 2008 at 09:15:32PM +0100, Ingo Molnar wrote:
> > 
> > * Jack Steiner <steiner@sgi.com> wrote:
> > 
> > > Index: linux/arch/x86/kernel/apic_64.c
> > > ===================================================================
> > > --- linux.orig/arch/x86/kernel/apic_64.c	2008-03-28 13:00:22.000000000 -0500
> > > +++ linux/arch/x86/kernel/apic_64.c	2008-03-28 13:06:12.000000000 -0500
> > > @@ -738,6 +738,7 @@ void __cpuinit setup_local_APIC(void)
> > >  	unsigned int value;
> > >  	int i, j;
> > >  
> > > +	preempt_disable();
> > >  	value = apic_read(APIC_LVR);
> > >  
> > >  	BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);
> > > @@ -831,6 +832,7 @@ void __cpuinit setup_local_APIC(void)
> > >  	else
> > >  		value = APIC_DM_NMI | APIC_LVT_MASKED;
> > >  	apic_write(APIC_LVT1, value);
> > > +	preempt_enable();
> > >  }
> > 
> > hm, this looks a bit weird - why are all the preempt-disable/enable 
> > calls needed?
> 
> The first patch had a preempt disable/enable in the function that 
> reads apicid (see read_apic_id() in arch/x86/kernel/genapic_64.c). 
> This seemed necessary since large system generate an apicid by reading 
> the live id & concatenating it with extra bits.
> 
> One of the review comments suggested that I change the preempt to a 
> WARN() since reading apic_id really should be done with preemtion 
> disabled. The added code eliminates the warnings.

could you post the stacktraces of the warnings as they occured? All 
those codepaths should be running with preemption disabled in some 
natural way.

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Jack Steiner]

On Fri, Mar 28, 2008 at 09:45:33PM +0100, Ingo Molnar wrote:
> 
> * Jack Steiner <steiner@sgi.com> wrote:
> 
> > On Fri, Mar 28, 2008 at 09:15:32PM +0100, Ingo Molnar wrote:
> > > 
> > > * Jack Steiner <steiner@sgi.com> wrote:
> > > 
> > > > Index: linux/arch/x86/kernel/apic_64.c
> > > > ===================================================================
> > > > --- linux.orig/arch/x86/kernel/apic_64.c	2008-03-28 13:00:22.000000000 -0500
> > > > +++ linux/arch/x86/kernel/apic_64.c	2008-03-28 13:06:12.000000000 -0500
> > > > @@ -738,6 +738,7 @@ void __cpuinit setup_local_APIC(void)
> > > >  	unsigned int value;
> > > >  	int i, j;
> > > >  
> > > > +	preempt_disable();
> > > >  	value = apic_read(APIC_LVR);
> > > >  
> > > >  	BUILD_BUG_ON((SPURIOUS_APIC_VECTOR & 0x0f) != 0x0f);
> > > > @@ -831,6 +832,7 @@ void __cpuinit setup_local_APIC(void)
> > > >  	else
> > > >  		value = APIC_DM_NMI | APIC_LVT_MASKED;
> > > >  	apic_write(APIC_LVT1, value);
> > > > +	preempt_enable();
> > > >  }
> > > 
> > > hm, this looks a bit weird - why are all the preempt-disable/enable 
> > > calls needed?
> > 
> > The first patch had a preempt disable/enable in the function that 
> > reads apicid (see read_apic_id() in arch/x86/kernel/genapic_64.c). 
> > This seemed necessary since large system generate an apicid by reading 
> > the live id & concatenating it with extra bits.
> > 
> > One of the review comments suggested that I change the preempt to a 
> > WARN() since reading apic_id really should be done with preemtion 
> > disabled. The added code eliminates the warnings.
> 
> could you post the stacktraces of the warnings as they occured? All 
> those codepaths should be running with preemption disabled in some 
> natural way.
> 
> 	Ingo

Here is one of the pathes. I have not been able to reproduce the 
other (smp_sanity_check()).  It's possible there was cleanup that
eliminated the message, or I might have forgotten the receipe. I
know it was early in boot:



WARNING: at arch/x86/kernel/genapic_64.c:97 read_apic_id+0x30/0x6a()
Modules linked in:
Pid: 1, comm: swapper Not tainted 2.6.25-rc7-x86-latest.git-00829-gfb5f592-dirty #15

Call Trace:
 [<ffffffff80231e69>] warn_on_slowpath+0x51/0x63
 [<ffffffff80578d37>] _spin_lock+0xe/0x24
 [<ffffffff8022b289>] task_rq_lock+0x3d/0x6f
 [<ffffffff80579133>] _spin_unlock_irqrestore+0x12/0x2c
 [<ffffffff8022d7fd>] set_cpus_allowed+0xd9/0xe6
 [<ffffffff8057319c>] set_cpu_sibling_map+0x2f4/0x30c
 [<ffffffff8021f434>] read_apic_id+0x30/0x6a
 [<ffffffff807cf898>] native_smp_prepare_cpus+0xc4/0x2d4
 [<ffffffff807c580f>] kernel_init+0x4e/0x2cb
 [<ffffffff8020bee8>] child_rip+0xa/0x12


--- jack

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Yinghai Lu]

On Fri, Mar 28, 2008 at 12:12 PM, Jack Steiner <steiner@sgi.com> wrote:
>
>  UV supports really big systems. So big, in fact, that the APICID register
>  does not contain enough bits to contain an APICID that is unique across all
>  cpus.
>
>  The UV BIOS supports 3 APICID modes:
>
>         - legacy mode. This mode uses the old APIC mode where
>           APICID is in bits [31:24] of the APICID register.
>
>         - x2apic mode. This mode is whitebox-compatible. APICIDs
>           are unique across all cpus. Standard x2apic APIC operations
>           (Intel-defined) can be used for IPIs. The node identifier
>           fits within the Intel-defined portion of the APICID register.
>
>         - x2apic-uv mode. In this mode, the APICIDs on each node have
>           unique IDs, but IDs on different node are not unique. For example,
>           if each mode has 32 cpus, the APICIDs on each node might be
>           0 - 31. Every node has the same set of IDs.
>           The UV hub is used to route IPIs/interrupts to the correct node.
>           Traditional APIC operations WILL NOT WORK.
>
>  In x2apic-uv mode, the ACPI tables all contain a full unique ID (note:
>  exact bit layout still changing but the following is close):
>
>         nnnnnnnnnnlc0cch
>                 n = unique node number
>                 l = socket number on board
>                 c = core
>                 h = hyperthread
>
>  Only the "lc0cch" bits are written to the APICID register. The remaining bits are
>  supplied by having the get_apic_id() function "OR" the extra bits into the value
>  read from the APICID register. (Hmmm.. why not keep the ENTIRE APICID register
>  in per-cpu data....)
>
>  The x2apic-uv mode is recognized by the MADT table containing:
>           oem_id = "SGI"
>           oem_table_id = "UV-X"
>
>  Based on:
>         git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86.git
>
>  Signed-off-by: Jack Steiner <steiner@sgi.com>
>
>  ---
>   arch/x86/kernel/Makefile         |    2
>   arch/x86/kernel/apic_64.c        |    2
>   arch/x86/kernel/genapic_64.c     |   18 ++
>   arch/x86/kernel/genx2apic_uv_x.c |  245 +++++++++++++++++++++++++++++++++++++++
>   arch/x86/kernel/setup64.c        |    4
>   arch/x86/kernel/smpboot.c        |    5
>   include/asm-x86/genapic_64.h     |    5
>   include/asm-x86/smp.h            |    5
>   8 files changed, 285 insertions(+), 1 deletion(-)
>
>  Index: linux/arch/x86/kernel/genapic_64.c
>  ===================================================================
>  --- linux.orig/arch/x86/kernel/genapic_64.c     2008-03-28 13:06:07.000000000 -0500
>  +++ linux/arch/x86/kernel/genapic_64.c  2008-03-28 13:06:12.000000000 -0500
>  @@ -15,6 +15,7 @@
>   #include <linux/kernel.h>
>   #include <linux/ctype.h>
>   #include <linux/init.h>
>  +#include <linux/hardirq.h>
>
>   #include <asm/smp.h>
>   #include <asm/ipi.h>
>  @@ -32,6 +33,7 @@ void *x86_cpu_to_apicid_early_ptr;
>   #endif
>   DEFINE_PER_CPU(u16, x86_cpu_to_apicid) = BAD_APICID;
>   EXPORT_PER_CPU_SYMBOL(x86_cpu_to_apicid);
>  +DEFINE_PER_CPU(int, x2apic_extra_bits);
>
>   struct genapic __read_mostly *genapic = &apic_flat;
>
>  @@ -42,6 +44,9 @@ static enum uv_system_type uv_system_typ
>   */
>   void __init setup_apic_routing(void)
>   {
>  +       if (uv_system_type == UV_NON_UNIQUE_APIC)
>  +               genapic = &apic_x2apic_uv_x;
>  +       else
>   #ifdef CONFIG_ACPI
>         /*
>          * Quirk: some x86_64 machines can only use physical APIC mode
>  @@ -82,6 +87,19 @@ int __init acpi_madt_oem_check(char *oem
>         return 0;
>   }
>
>  +unsigned int read_apic_id(void)
>  +{
>  +       unsigned int id;
>  +
>  +       WARN_ON(preemptible());
>  +       id = apic_read(APIC_ID);
>  +       if (uv_system_type >= UV_X2APIC)
>  +               id  |= __get_cpu_var(x2apic_extra_bits);
>  +       else
>  +               id = (id >> 24) & 0xFFu;;
>  +       return id;
>  +}

so this is "the new one of Friday"?

it still wrong. you can not shit id here. ot broke all x86_64 smp.

Did you test it on non UV_X2APIC box?

YH

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Yinghai Lu]

On Sun, Mar 30, 2008 at 4:22 PM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
>
> On Fri, Mar 28, 2008 at 12:12 PM, Jack Steiner <steiner@sgi.com> wrote:
>  >
>  >  UV supports really big systems. So big, in fact, that the APICID register
>  >  does not contain enough bits to contain an APICID that is unique across all
>  >  cpus.
>  >
>  >  The UV BIOS supports 3 APICID modes:
>  >
>  >         - legacy mode. This mode uses the old APIC mode where
>  >           APICID is in bits [31:24] of the APICID register.
>  >
>  >         - x2apic mode. This mode is whitebox-compatible. APICIDs
>  >           are unique across all cpus. Standard x2apic APIC operations
>  >           (Intel-defined) can be used for IPIs. The node identifier
>  >           fits within the Intel-defined portion of the APICID register.
>  >
>  >         - x2apic-uv mode. In this mode, the APICIDs on each node have
>  >           unique IDs, but IDs on different node are not unique. For example,
>  >           if each mode has 32 cpus, the APICIDs on each node might be
>  >           0 - 31. Every node has the same set of IDs.
>  >           The UV hub is used to route IPIs/interrupts to the correct node.
>  >           Traditional APIC operations WILL NOT WORK.
>  >
>  >  In x2apic-uv mode, the ACPI tables all contain a full unique ID (note:
>  >  exact bit layout still changing but the following is close):
>  >
>  >         nnnnnnnnnnlc0cch
>  >                 n = unique node number
>  >                 l = socket number on board
>  >                 c = core
>  >                 h = hyperthread
>  >
>  >  Only the "lc0cch" bits are written to the APICID register. The remaining bits are
>  >  supplied by having the get_apic_id() function "OR" the extra bits into the value
>  >  read from the APICID register. (Hmmm.. why not keep the ENTIRE APICID register
>  >  in per-cpu data....)
>  >
>  >  The x2apic-uv mode is recognized by the MADT table containing:
>  >           oem_id = "SGI"
>  >           oem_table_id = "UV-X"
>  >
>  >  Based on:
>  >         git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86.git
>  >
>  >  Signed-off-by: Jack Steiner <steiner@sgi.com>
>  >
>  >  ---
>  >   arch/x86/kernel/Makefile         |    2
>  >   arch/x86/kernel/apic_64.c        |    2
>  >   arch/x86/kernel/genapic_64.c     |   18 ++
>  >   arch/x86/kernel/genx2apic_uv_x.c |  245 +++++++++++++++++++++++++++++++++++++++
>  >   arch/x86/kernel/setup64.c        |    4
>  >   arch/x86/kernel/smpboot.c        |    5
>  >   include/asm-x86/genapic_64.h     |    5
>  >   include/asm-x86/smp.h            |    5
>  >   8 files changed, 285 insertions(+), 1 deletion(-)
>  >
>  >  Index: linux/arch/x86/kernel/genapic_64.c
>  >  ===================================================================
>  >  --- linux.orig/arch/x86/kernel/genapic_64.c     2008-03-28 13:06:07.000000000 -0500
>  >  +++ linux/arch/x86/kernel/genapic_64.c  2008-03-28 13:06:12.000000000 -0500
>  >  @@ -15,6 +15,7 @@
>  >   #include <linux/kernel.h>
>  >   #include <linux/ctype.h>
>  >   #include <linux/init.h>
>  >  +#include <linux/hardirq.h>
>  >
>  >   #include <asm/smp.h>
>  >   #include <asm/ipi.h>
>  >  @@ -32,6 +33,7 @@ void *x86_cpu_to_apicid_early_ptr;
>  >   #endif
>  >   DEFINE_PER_CPU(u16, x86_cpu_to_apicid) = BAD_APICID;
>  >   EXPORT_PER_CPU_SYMBOL(x86_cpu_to_apicid);
>  >  +DEFINE_PER_CPU(int, x2apic_extra_bits);
>  >
>  >   struct genapic __read_mostly *genapic = &apic_flat;
>  >
>  >  @@ -42,6 +44,9 @@ static enum uv_system_type uv_system_typ
>  >   */
>  >   void __init setup_apic_routing(void)
>  >   {
>  >  +       if (uv_system_type == UV_NON_UNIQUE_APIC)
>  >  +               genapic = &apic_x2apic_uv_x;
>  >  +       else
>  >   #ifdef CONFIG_ACPI
>  >         /*
>  >          * Quirk: some x86_64 machines can only use physical APIC mode
>  >  @@ -82,6 +87,19 @@ int __init acpi_madt_oem_check(char *oem
>  >         return 0;
>  >   }
>  >
>  >  +unsigned int read_apic_id(void)
>  >  +{
>  >  +       unsigned int id;
>  >  +
>  >  +       WARN_ON(preemptible());
>  >  +       id = apic_read(APIC_ID);
>  >  +       if (uv_system_type >= UV_X2APIC)
>  >  +               id  |= __get_cpu_var(x2apic_extra_bits);
>  >  +       else
>  >  +               id = (id >> 24) & 0xFFu;;
>  >  +       return id;
>  >  +}
>
>  so this is "the new one of Friday"?
>
>  it still wrong. you can not shit id here. ot broke all x86_64 smp.
>
>  Did you test it on non UV_X2APIC box?

anyway the read_apic_id is totally wrong, even for your UV_X2APIC box.
because id=apic_read(APIC_ID) will have apic_id at bits [31,24], and
id |= __get_cpu_var(x2apic_extra_bits) is assuming that is on bits [5,0]

so you even didn't test in your UV_X2APIC box!

YH

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: V2 Support for new UV apic

[Jack Steiner]

Fix double-shift of apicid in previous patch.

Signed-off-by: Jack Steiner <steiner@sgi.com>


---
The code is clearly wrong.  I booted on an 8p AMD box and
had no problems. Apparently the kernel (at least basic booting) is
not too sensitive to incorrect apicids being returned. Most
critical-to-boot code must use apicids from the ACPI tables.


 arch/x86/kernel/genapic_64.c |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: linux/arch/x86/kernel/genapic_64.c
===================================================================
--- linux.orig/arch/x86/kernel/genapic_64.c	2008-03-30 20:37:18.000000000 -0500
+++ linux/arch/x86/kernel/genapic_64.c	2008-03-30 20:48:30.000000000 -0500
@@ -98,8 +98,6 @@ unsigned int read_apic_id(void)
 	id = apic_read(APIC_ID);
 	if (uv_system_type >= UV_X2APIC)
 		id  |= __get_cpu_var(x2apic_extra_bits);
-	else
-		id = (id >> 24) & 0xFFu;;
 	return id;
 }
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Jack Steiner]

> so this is "the new one of Friday"?

Yes, and it has the same bug although it is located
in a slightly different place.

A few minutes ago, I posted a patch to delete the extra lines.


> Did you test it on non UV_X2APIC box?

The code is clearly wrong.  I booted on an 8p AMD box and
had no problems. Apparently the kernel (at least basic booting) is
not too sensitive to incorrect apicids being returned. Most
critical-to-boot code must use apicids from the ACPI tables.
However, the bug does affect numa node assignment. And probably
other places, too.


--- jack

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Jack Steiner]

> >  Did you test it on non UV_X2APIC box?
> 
> anyway the read_apic_id is totally wrong, even for your UV_X2APIC box.
> because id=apic_read(APIC_ID) will have apic_id at bits [31,24], and
> id |= __get_cpu_var(x2apic_extra_bits) is assuming that is on bits [5,0]
> 
> so you even didn't test in your UV_X2APIC box!
> 

It works fine on UV_X2APIX boxes because the double shift does
not occur. However, support for UV_X2APIC is dependent on
x2apic code that is not yet in the tree. Once the APIC
is switched into x2apic mode, the apicid is located in the LOW
bits of the apicid register, not the HIGH bits.

I have a local x2apic patch that I apply on top of the previous
patches. The local patch is an early copy of a patch that will come
from Intel. However, the other 2 apic modes work fine with the
patches that were submitted.

--- jack

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Yinghai Lu]

On Sun, Mar 30, 2008 at 7:06 PM, Jack Steiner <steiner@sgi.com> wrote:
> > so this is "the new one of Friday"?
>
>  Yes, and it has the same bug although it is located
>  in a slightly different place.
>
>  A few minutes ago, I posted a patch to delete the extra lines.
>
>
>
>  > Did you test it on non UV_X2APIC box?
>
>  The code is clearly wrong.  I booted on an 8p AMD box and
>  had no problems. Apparently the kernel (at least basic booting) is
>  not too sensitive to incorrect apicids being returned. Most
>  critical-to-boot code must use apicids from the ACPI tables.
>  However, the bug does affect numa node assignment. And probably
>  other places, too.

please consider one global get_apic_id() and bad_apicid to replace
GET_APIC_ID and BAD_APICID at this point.

YH

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Yinghai Lu]

On Sun, Mar 30, 2008 at 7:12 PM, Jack Steiner <steiner@sgi.com> wrote:
> > >  Did you test it on non UV_X2APIC box?
>  >
>  > anyway the read_apic_id is totally wrong, even for your UV_X2APIC box.
>  > because id=apic_read(APIC_ID) will have apic_id at bits [31,24], and
>  > id |= __get_cpu_var(x2apic_extra_bits) is assuming that is on bits [5,0]
>  >
>  > so you even didn't test in your UV_X2APIC box!
>  >
>
>  It works fine on UV_X2APIX boxes because the double shift does
>  not occur. However, support for UV_X2APIC is dependent on
>  x2apic code that is not yet in the tree. Once the APIC
>  is switched into x2apic mode, the apicid is located in the LOW
>  bits of the apicid register, not the HIGH bits.

oh, so that will need have new version GET_APIC_ID too.

YH

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Jack Steiner]

On Sun, Mar 30, 2008 at 07:13:42PM -0700, Yinghai Lu wrote:
> On Sun, Mar 30, 2008 at 7:06 PM, Jack Steiner <steiner@sgi.com> wrote:
> > > so this is "the new one of Friday"?
> >
> >  Yes, and it has the same bug although it is located
> >  in a slightly different place.
> >
> >  A few minutes ago, I posted a patch to delete the extra lines.
> >
> >
> >
> >  > Did you test it on non UV_X2APIC box?
> >
> >  The code is clearly wrong.  I booted on an 8p AMD box and
> >  had no problems. Apparently the kernel (at least basic booting) is
> >  not too sensitive to incorrect apicids being returned. Most
> >  critical-to-boot code must use apicids from the ACPI tables.
> >  However, the bug does affect numa node assignment. And probably
> >  other places, too.
> 
> please consider one global get_apic_id() and bad_apicid to replace
> GET_APIC_ID and BAD_APICID at this point.

I think that makes sense.

The x2apic patch that should be posted in the near future also makes
significant changes in this area.  Once that patch is posted, I'll
make the simplifications.

Ok???

--- jack

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Jack Steiner]

On Sun, Mar 30, 2008 at 07:23:07PM -0700, Yinghai Lu wrote:
> On Sun, Mar 30, 2008 at 7:12 PM, Jack Steiner <steiner@sgi.com> wrote:
> > > >  Did you test it on non UV_X2APIC box?
> >  >
> >  > anyway the read_apic_id is totally wrong, even for your UV_X2APIC box.
> >  > because id=apic_read(APIC_ID) will have apic_id at bits [31,24], and
> >  > id |= __get_cpu_var(x2apic_extra_bits) is assuming that is on bits [5,0]
> >  >
> >  > so you even didn't test in your UV_X2APIC box!
> >  >
> >
> >  It works fine on UV_X2APIX boxes because the double shift does
> >  not occur. However, support for UV_X2APIC is dependent on
> >  x2apic code that is not yet in the tree. Once the APIC
> >  is switched into x2apic mode, the apicid is located in the LOW
> >  bits of the apicid register, not the HIGH bits.
> 
> oh, so that will need have new version GET_APIC_ID too.

Yes, although I think all the changes will be unified into
one non-inline function that is a combination of
GET_APIC_ID() & read_apic_id().

--- jack

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Yinghai Lu]

On Sun, Mar 30, 2008 at 7:23 PM, Jack Steiner <steiner@sgi.com> wrote:
>
> On Sun, Mar 30, 2008 at 07:13:42PM -0700, Yinghai Lu wrote:
>  > On Sun, Mar 30, 2008 at 7:06 PM, Jack Steiner <steiner@sgi.com> wrote:
>  > > > so this is "the new one of Friday"?
>  > >
>  > >  Yes, and it has the same bug although it is located
>  > >  in a slightly different place.
>  > >
>  > >  A few minutes ago, I posted a patch to delete the extra lines.
>  > >
>  > >
>  > >
>  > >  > Did you test it on non UV_X2APIC box?
>  > >
>  > >  The code is clearly wrong.  I booted on an 8p AMD box and
>  > >  had no problems. Apparently the kernel (at least basic booting) is
>  > >  not too sensitive to incorrect apicids being returned. Most
>  > >  critical-to-boot code must use apicids from the ACPI tables.
>  > >  However, the bug does affect numa node assignment. And probably
>  > >  other places, too.
>  >
>  > please consider one global get_apic_id() and bad_apicid to replace
>  > GET_APIC_ID and BAD_APICID at this point.
>
>  I think that makes sense.
>
>  The x2apic patch that should be posted in the near future also makes
>  significant changes in this area.  Once that patch is posted, I'll
>  make the simplifications.
>
>  Ok???

good.

with the x2apic patch, GET_APIC_ID is read_apic_id, and it will not shift again?

YH

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Yinghai Lu]

On Sun, Mar 30, 2008 at 7:26 PM, Jack Steiner <steiner@sgi.com> wrote:
>
> On Sun, Mar 30, 2008 at 07:23:07PM -0700, Yinghai Lu wrote:
>  > On Sun, Mar 30, 2008 at 7:12 PM, Jack Steiner <steiner@sgi.com> wrote:
>  > > > >  Did you test it on non UV_X2APIC box?
>  > >  >
>  > >  > anyway the read_apic_id is totally wrong, even for your UV_X2APIC box.
>  > >  > because id=apic_read(APIC_ID) will have apic_id at bits [31,24], and
>  > >  > id |= __get_cpu_var(x2apic_extra_bits) is assuming that is on bits [5,0]
>  > >  >
>  > >  > so you even didn't test in your UV_X2APIC box!
>  > >  >
>  > >
>  > >  It works fine on UV_X2APIX boxes because the double shift does
>  > >  not occur. However, support for UV_X2APIC is dependent on
>  > >  x2apic code that is not yet in the tree. Once the APIC
>  > >  is switched into x2apic mode, the apicid is located in the LOW
>  > >  bits of the apicid register, not the HIGH bits.
>  >
>  > oh, so that will need have new version GET_APIC_ID too.
>
>  Yes, although I think all the changes will be unified into
>  one non-inline function that is a combination of
>  GET_APIC_ID() & read_apic_id().

it seems x2apic patch should be applied before uv patch...

YH

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Jack Steiner]

On Sun, Mar 30, 2008 at 07:27:25PM -0700, Yinghai Lu wrote:
> On Sun, Mar 30, 2008 at 7:23 PM, Jack Steiner <steiner@sgi.com> wrote:
> >
> > On Sun, Mar 30, 2008 at 07:13:42PM -0700, Yinghai Lu wrote:
> >  > On Sun, Mar 30, 2008 at 7:06 PM, Jack Steiner <steiner@sgi.com> wrote:
> >  > > > so this is "the new one of Friday"?
> >  > >
> >  > >  Yes, and it has the same bug although it is located
> >  > >  in a slightly different place.
> >  > >
> >  > >  A few minutes ago, I posted a patch to delete the extra lines.
> >  > >
> >  > >
> >  > >
> >  > >  > Did you test it on non UV_X2APIC box?
> >  > >
> >  > >  The code is clearly wrong.  I booted on an 8p AMD box and
> >  > >  had no problems. Apparently the kernel (at least basic booting) is
> >  > >  not too sensitive to incorrect apicids being returned. Most
> >  > >  critical-to-boot code must use apicids from the ACPI tables.
> >  > >  However, the bug does affect numa node assignment. And probably
> >  > >  other places, too.
> >  >
> >  > please consider one global get_apic_id() and bad_apicid to replace
> >  > GET_APIC_ID and BAD_APICID at this point.
> >
> >  I think that makes sense.
> >
> >  The x2apic patch that should be posted in the near future also makes
> >  significant changes in this area.  Once that patch is posted, I'll
> >  make the simplifications.
> >
> >  Ok???
> 
> good.
> 
> with the x2apic patch, GET_APIC_ID is read_apic_id, and it will not shift again?
> 
> YH

Right....

--- jack

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: Support for new UV apic

[Jack Steiner]

On Sun, Mar 30, 2008 at 07:29:40PM -0700, Yinghai Lu wrote:
> On Sun, Mar 30, 2008 at 7:26 PM, Jack Steiner <steiner@sgi.com> wrote:
> >
> > On Sun, Mar 30, 2008 at 07:23:07PM -0700, Yinghai Lu wrote:
> >  > On Sun, Mar 30, 2008 at 7:12 PM, Jack Steiner <steiner@sgi.com> wrote:
> >  > > > >  Did you test it on non UV_X2APIC box?
> >  > >  >
> >  > >  > anyway the read_apic_id is totally wrong, even for your UV_X2APIC box.
> >  > >  > because id=apic_read(APIC_ID) will have apic_id at bits [31,24], and
> >  > >  > id |= __get_cpu_var(x2apic_extra_bits) is assuming that is on bits [5,0]
> >  > >  >
> >  > >  > so you even didn't test in your UV_X2APIC box!
> >  > >  >
> >  > >
> >  > >  It works fine on UV_X2APIX boxes because the double shift does
> >  > >  not occur. However, support for UV_X2APIC is dependent on
> >  > >  x2apic code that is not yet in the tree. Once the APIC
> >  > >  is switched into x2apic mode, the apicid is located in the LOW
> >  > >  bits of the apicid register, not the HIGH bits.
> >  >
> >  > oh, so that will need have new version GET_APIC_ID too.
> >
> >  Yes, although I think all the changes will be unified into
> >  one non-inline function that is a combination of
> >  GET_APIC_ID() & read_apic_id().
> 
> it seems x2apic patch should be applied before uv patch...
> 
> YH

That was the original plan but the x2apic code, although functional,
is not yet ready to be integrated. (This was mentioned as an
outstanding issue in the patch on Friday).

Once the x2apic patch is applied, there will be one additional
trivial UV patch (currently 1-line) that will be required to
make UV_X2APIX fully functional. Since the other 2 UV apic modes
are working right now, I decided not to wait for the x2apic patch
before submitting the patches for the basic UV infrastructure.

We have a lot of code that is queued up waiting for the
basic UV infrastructure to be integrated.


--- jack


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] x86_64: V2 Support for new UV apic

[Ingo Molnar]

* Jack Steiner <steiner@sgi.com> wrote:

> Fix double-shift of apicid in previous patch.

> The code is clearly wrong.  I booted on an 8p AMD box and had no 
> problems. Apparently the kernel (at least basic booting) is not too 
> sensitive to incorrect apicids being returned. Most critical-to-boot 
> code must use apicids from the ACPI tables.

yeah - patch added. Thanks Yinghai for persisting on this.

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>