Skip to contentSkip to conversation

16-way x440 breakage

PATCH In review Matthew Dobson 2 participants 2 messages Aug 8, 2003 – Aug 12, 2003

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 16-way x440 breakage

There's some ugliness (which I'm unfortunately responsible for) in some of the x440 specific code. It's causing boot problems for us with certain configs. This patch remedies those problems.

* There was an #ifdef'd call in setup_arch. It's been moved into subarch code where it belongs.

* There were some ugly for loops that now are more readable.

* The NUMA-specific summit code is now only compiled in & executed if CONFIG_NUMA.

* The code checks that MAX_NUMNODES has a sane value before proceeding.

Compiled and tested on 16 x440s, both NUMA & SMP.

Cheers!

-Matt

arch/i386/kernel/setup.c +0 -3
diff -Nurp --exclude-from=/home/mcd/.dontdiff linux-2.6.0-test2-vanilla/arch/i386/kernel/setup.c linux-2.6.0-test2-16wayfix/arch/i386/kernel/setup.c
@@ -989,9 +989,6 @@ void __init setup_arch(char **cmdline_p)
 	if (smp_found_config)
 		get_smp_config();
 #endif
-#ifdef CONFIG_X86_SUMMIT
-	setup_summit();
-#endif
 
 	register_memory(max_low_pfn);
 
arch/i386/kernel/summit.c +19 -11
diff -Nurp --exclude-from=/home/mcd/.dontdiff linux-2.6.0-test2-vanilla/arch/i386/kernel/summit.c linux-2.6.0-test2-16wayfix/arch/i386/kernel/summit.c
@@ -31,6 +31,7 @@
 #include <asm/io.h>
 #include <mach_mpparse.h>
 
+#ifdef CONFIG_NUMA
 static void __init setup_pci_node_map_for_wpeg(int wpeg_num, struct rio_table_hdr *rth, 
 		struct scal_detail **scal_nodes, struct rio_detail **rio_nodes){
 	int twst_num = 0, node = 0, first_bus = 0;
@@ -93,15 +94,21 @@ static void __init setup_pci_node_map_fo
 		mp_bus_id_to_node[bus] = node;
 }
 
-static void __init build_detail_arrays(struct rio_table_hdr *rth,
+static int __init build_detail_arrays(struct rio_table_hdr *rth,
 		struct scal_detail **sd, struct rio_detail **rd){
 	unsigned long ptr;
 	int i, scal_detail_size, rio_detail_size;
 
+	if ((rth->num_scal_dev > MAX_NUMNODES) || 
+	    (rth->num_rio_dev > MAX_NUMNODES * 2)){
+		printk("%s ERROR!  MAX_NUMNODES incorrectly defined as %d!!!\n", __FUNCTION__, MAX_NUMNODES);
+		return 1;
+	}
+
 	switch (rth->version){
 	default:
 		printk("%s: Bad Rio Grande Table Version: %d\n", __FUNCTION__, rth->version);
-		/* Fall through to default to version 2 spec */
+		return 1;
 	case 2:
 		scal_detail_size = 11;
 		rio_detail_size = 13;
@@ -113,12 +120,13 @@ static void __init build_detail_arrays(s
 	}
 
 	ptr = (unsigned long)rth + 3;
-	for(i = 0; i < rth->num_scal_dev; i++)
-		sd[i] = (struct scal_detail *)(ptr + (scal_detail_size * i));
+	for(i = 0; i < rth->num_scal_dev; i++, ptr += scal_detail_size)
+		sd[i] = (struct scal_detail *)ptr;
+
+	for(i = 0; i < rth->num_rio_dev; i++, ptr += rio_detail_size)
+		rd[i] = (struct rio_detail *)ptr;
 
-	ptr += scal_detail_size * rth->num_scal_dev;
-	for(i = 0; i < rth->num_rio_dev; i++)
-		rd[i] = (struct rio_detail *)(ptr + (rio_detail_size * i));
+	return 0;
 }
 
 void __init setup_summit(void)
@@ -130,8 +138,6 @@ void __init setup_summit(void)
 	unsigned short		offset;
 	int			i;
 
-	memset(mp_bus_id_to_node, -1, sizeof(mp_bus_id_to_node));
-
 	/* The pointer to the EBDA is stored in the word @ phys 0x40E(40:0E) */
 	ptr = *(unsigned short *)phys_to_virt(0x40Eul);
 	ptr = (unsigned long)phys_to_virt(ptr << 4);
@@ -152,11 +158,13 @@ void __init setup_summit(void)
 		return;
 	}
 
-	/* Deal with the ugly version 2/3 pointer arithmetic */
-	build_detail_arrays(rio_table_hdr, scal_devs, rio_devs);
+	/* Parse table.  Non-zero return means bad table. */
+	if (build_detail_arrays(rio_table_hdr, scal_devs, rio_devs))
+		return;
 
 	for(i = 0; i < rio_table_hdr->num_rio_dev; i++)
 		if (is_WPEG(rio_devs[i]->type))
 			/* It's a Winnipeg, it's got PCI Busses */
 			setup_pci_node_map_for_wpeg(i, rio_table_hdr, scal_devs, rio_devs);
 }
+#endif /* CONFIG_NUMA */
include/asm-i386/mach-summit/mach_mpparse.h +8 -0
diff -Nurp --exclude-from=/home/mcd/.dontdiff linux-2.6.0-test2-vanilla/include/asm-i386/mach-summit/mach_mpparse.h linux-2.6.0-test2-16wayfix/include/asm-i386/mach-summit/mach_mpparse.h
@@ -5,6 +5,12 @@
 
 extern int use_cyclone;
 
+#ifdef CONFIG_NUMA
+extern void setup_summit(void);
+#else /* !CONFIG_NUMA */
+#define setup_summit()	{}
+#endif /* CONFIG_NUMA */
+
 static inline void mpc_oem_bus_info(struct mpc_config_bus *m, char *name, 
 				struct mpc_config_translation *translation)
 {
@@ -24,6 +30,7 @@ static inline int mps_oem_check(struct m
 			 || !strncmp(productid, "EXA", 3)
 			 || !strncmp(productid, "RUTHLESS SMP", 12))){
 		use_cyclone = 1; /*enable cyclone-timer*/
+		setup_summit();
 		return 1;
 	}
 	return 0;
@@ -36,6 +43,7 @@ static inline int acpi_madt_oem_check(ch
 	    (!strncmp(oem_table_id, "SERVIGIL", 8)
 	     || !strncmp(oem_table_id, "EXA", 3))){
 		use_cyclone = 1; /*enable cyclone-timer*/
+		setup_summit();
 		return 1;
 	}
 	return 0;
#1 lore raw

arch/i386/kernel/summit.c static void __init setup_pci_node_map_fo · line 106

Quoted 5 lines · Matthew Dobson
arch/i386/kernel/summit.c +5 -0
+	if ((rth->num_scal_dev > MAX_NUMNODES) || 
+	    (rth->num_rio_dev > MAX_NUMNODES * 2)){
+		printk("%s ERROR!  MAX_NUMNODES incorrectly defined as %d!!!\n", __FUNCTION__, MAX_NUMNODES);
+		return 1;
+	}

Why don't you actually warn for the real condition? MAX_NUMNODES isn't incorrect, it's just too low. Could you mention that it needs to be raised, and maybe the value it should be raised to?

arch/i386/kernel/summit.c static void __init build_detail_arrays(s · line 127

Quoted 8 lines
arch/i386/kernel/summit.c +5 -2
 	ptr = (unsigned long)rth + 3;
-	for(i = 0; i < rth->num_scal_dev; i++)
-		sd[i] = (struct scal_detail *)(ptr + (scal_detail_size * i));
+	for(i = 0; i < rth->num_scal_dev; i++, ptr += scal_detail_size)
+		sd[i] = (struct scal_detail *)ptr;
+
+	for(i = 0; i < rth->num_rio_dev; i++, ptr += rio_detail_size)
+		rd[i] = (struct rio_detail *)ptr;

All the casting here scares me. If you're doing this:
(struct scal_detail *)(ptr + (scal_detail_size * i)
and this:
ptr += scal_detail_size
just make it a struct scal_detail * and be done with it. If it walks like a duck...

Signature · 2 lines
Dave Hansen
[email protected]