2002-10-14 22:27:32

by Martin J. Bligh

[permalink] [raw]
Subject: [PATCH] Summit support for 2.5 - now with subarch! [4/5]

Adds detection for summit machines from the MPS tables.
Prints a handy-dandy debug message telling you what kind of twisted
machine the kernel thinks you have.

diff -purN -X /home/mbligh/.diff.exclude subarch-3/arch/i386/kernel/mpparse.c subarch-4/arch/i386/kernel/mpparse.c
--- subarch-3/arch/i386/kernel/mpparse.c Fri Oct 11 21:21:37 2002
+++ subarch-4/arch/i386/kernel/mpparse.c Mon Oct 14 11:15:27 2002
@@ -30,6 +30,7 @@
#include <asm/mpspec.h>
#include <asm/pgalloc.h>
#include <asm/io_apic.h>
+#include "mach_apic.h"

/* Have we found an MP table */
int smp_found_config;
@@ -69,6 +70,8 @@ static unsigned int __initdata num_proce
/* Bitmask of physically existing CPUs */
unsigned long phys_cpu_present_map;

+int summit_x86 = 0;
+
/*
* Intel MP BIOS table parsing routines:
*/
@@ -356,6 +359,7 @@ static void __init smp_read_mpc_oem(stru
static int __init smp_read_mpc(struct mp_config_table *mpc)
{
char str[16];
+ char oem[10];
int count=sizeof(*mpc);
unsigned char *mpt=((unsigned char *)mpc)+count;

@@ -380,14 +384,16 @@ static int __init smp_read_mpc(struct mp
printk(KERN_ERR "SMP mptable: null local APIC address!\n");
return 0;
}
- memcpy(str,mpc->mpc_oem,8);
- str[8]=0;
- printk("OEM ID: %s ",str);
+ memcpy(oem,mpc->mpc_oem,8);
+ oem[8]=0;
+ printk("OEM ID: %s ",oem);

memcpy(str,mpc->mpc_productid,12);
str[12]=0;
printk("Product ID: %s ",str);

+ summit_check(oem, str);
+
printk("APIC at: 0x%lX\n",mpc->mpc_lapic);

/*
@@ -465,6 +471,7 @@ static int __init smp_read_mpc(struct mp
}
++mpc_record;
}
+ clustered_apic_check();
if (!num_processors)
printk(KERN_ERR "SMP mptable: no processors registered!\n");
return num_processors;
diff -purN -X /home/mbligh/.diff.exclude subarch-3/arch/i386/mach-generic/mach_apic.h subarch-4/arch/i386/mach-generic/mach_apic.h
--- subarch-3/arch/i386/mach-generic/mach_apic.h Mon Oct 14 11:00:46 2002
+++ subarch-4/arch/i386/mach-generic/mach_apic.h Mon Oct 14 11:18:22 2002
@@ -20,4 +20,14 @@ static inline unsigned long calculate_ld
#define APIC_BROADCAST_ID 0x0F
#define check_apicid_used(bitmap, apicid) (bitmap & (1 << apicid))

+static inline void summit_check(char *oem, char *productid)
+{
+}
+
+static inline void clustered_apic_check(void)
+{
+ printk("Enabling APIC mode: %s. Using %d I/O APICs\n",
+ (clustered_apic_mode ? "NUMA-Q" : "Flat"), nr_ioapics);
+}
+
#endif /* __ASM_MACH_APIC_H */
diff -purN -X /home/mbligh/.diff.exclude subarch-3/arch/i386/mach-summit/mach_apic.h subarch-4/arch/i386/mach-summit/mach_apic.h
--- subarch-3/arch/i386/mach-summit/mach_apic.h Mon Oct 14 11:01:00 2002
+++ subarch-4/arch/i386/mach-summit/mach_apic.h Mon Oct 14 11:18:08 2002
@@ -26,4 +26,16 @@ static inline unsigned long calculate_ld
#define APIC_BROADCAST_ID (x86_summit ? 0xFF : 0x0F)
#define check_apicid_used(bitmap, apicid) (0)

+static inline void summit_check(char *oem, char *productid)
+{
+ if (!strncmp(oem, "IBM ENSW", 8) && !strncmp(str, "VIGIL SMP", 9))
+ x86_summit = 1;
+}
+
+static inline void clustered_apic_check(void)
+{
+ printk("Enabling APIC mode: %s. Using %d I/O APICs\n",
+ (x86_summit ? "Summit" : "Flat"), nr_ioapics);
+}
+
#endif /* __ASM_MACH_APIC_H */


2002-10-14 23:05:21

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] Summit support for 2.5 - now with subarch! [4/5]

> + summit_check(oem, str);

This is more a type check hook. Since it might potentially be used to catch
other machine types, shouldn't it have a more generic name
(mptable_string_hook or something)?

> +int summit_x86 = 0;

This should really be in a .c file in mach-summit. I know a single line file
with just a variable in it is a bit strange, but the principle of the subarch
stuff is to have anything subarch specific (which this is) in mach-<subarch>.

James


2002-10-14 23:31:28

by Martin J. Bligh

[permalink] [raw]
Subject: Re: [PATCH] Summit support for 2.5 - now with subarch! [4/5]

>> + summit_check(oem, str);
>
> This is more a type check hook. Since it might potentially be used to catch
> other machine types, shouldn't it have a more generic name
> (mptable_string_hook or something)?

OK, I can change that.

>> +int summit_x86 = 0;
>
> This should really be in a .c file in mach-summit. I know a single line file
> with just a variable in it is a bit strange, but the principle of the subarch
> stuff is to have anything subarch specific (which this is) in mach-<subarch>.

That's pretty pointless for one variable. I think you're taking things
to ridiculous extremes.

M.


2002-10-15 00:13:22

by Martin J. Bligh

[permalink] [raw]
Subject: Re: [PATCH] Summit support for 2.5 - now with subarch! [4/5]

This just changes summit_check to be called mptable_machine_detect
to keep people happy.

M.

diff -urpN -X /home/fletch/.diff.exclude subarch-5/arch/i386/kernel/mpparse.c subarch-6/arch/i386/kernel/mpparse.c
--- subarch-5/arch/i386/kernel/mpparse.c Mon Oct 14 17:02:46 2002
+++ subarch-6/arch/i386/kernel/mpparse.c Mon Oct 14 17:08:23 2002
@@ -394,7 +394,7 @@ static int __init smp_read_mpc(struct mp
str[12]=0;
printk("Product ID: %s ",str);

- summit_check(oem, str);
+ mptable_machine_detect(oem, str);

printk("APIC at: 0x%lX\n",mpc->mpc_lapic);

diff -urpN -X /home/fletch/.diff.exclude subarch-5/arch/i386/mach-generic/mach_apic.h subarch-6/arch/i386/mach-generic/mach_apic.h
--- subarch-5/arch/i386/mach-generic/mach_apic.h Mon Oct 14 17:02:46 2002
+++ subarch-6/arch/i386/mach-generic/mach_apic.h Mon Oct 14 17:09:00 2002
@@ -20,7
+20,7 @@ static inline unsigned long calculate_ld
#define APIC_BROADCAST_ID 0x0F
#define check_apicid_used(bitmap, apicid) (bitmap & (1 << apicid))

-static inline void summit_check(char *oem, char *productid)
+static inline void mptable_machine_detect(char *oem, char *productid)
{
}

diff -urpN -X /home/fletch/.diff.exclude subarch-5/arch/i386/mach-summit/mach_apic.h subarch-6/arch/i386/mach-summit/mach_apic.h
--- subarch-5/arch/i386/mach-summit/mach_apic.h Mon Oct 14 17:02:46 2002
+++ subarch-6/arch/i386/mach-summit/mach_apic.h Mon Oct 14 17:09:19 2002
@@ -26,7 +26,7 @@ static inline unsigned long calculate_ld
#define APIC_BROADCAST_ID (x86_summit ? 0xFF : 0x0F)
#define check_apicid_used(bitmap, apicid) (0)

-static inline void
summit_check(char *oem, char *productid)
+static inline void mptable_machine_detect(char *oem, char *productid)
{
if (!strncmp(oem, "IBM ENSW", 8) && !strncmp(str, "VIGIL SMP", 9))
x86_summit = 1;

2002-10-15 17:28:38

by Martin J. Bligh

[permalink] [raw]
Subject: Re: [PATCH] Summit support for 2.5 - now with subarch! [4/5]

>> That's pretty pointless for one variable. I think you're taking things
>> to ridiculous extremes.
>
> OK, I agree that a single .c file for one variable is very extreme. I think
> you also would agree with me that if it had been ten variables and an exported
> function then it should live in a separate .c file in the summit specific code.

Yup.

> If you can promise me that summit will never need an extra variable or
> exported function as the code evolves from now until the end of the
> architecture then I can live with summit_x86 in the main line.

I don't think it'll ever need it, but if it does, I'll create it ;-)

M.

2002-10-15 17:24:15

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] Summit support for 2.5 - now with subarch! [4/5]

> > This should really be in a .c file in mach-summit. I know a single line file
> > with just a variable in it is a bit strange, but the principle of the subarch
> > stuff is to have anything subarch specific (which this is) in mach-<subarch>.

> That's pretty pointless for one variable. I think you're taking things
> to ridiculous extremes.

OK, I agree that a single .c file for one variable is very extreme. I think
you also would agree with me that if it had been ten variables and an exported
function then it should live in a separate .c file in the summit specific code.

My concern is that there will come a day when the summit code is enhanced to
add the extra nine variables and the function. Since there's nowhere in
mach-summit to add them, they get added to smpboot.c. Now we have a go around
on linux-kernel about why they should be in a separate .c file.

You see the issue: I code by looking at how someone else did it, so if we're
setting a precedent then it should be done correctly rather than catching and
correcting a mistake we expect someone will now make.

If you can promise me that summit will never need an extra variable or
exported function as the code evolves from now until the end of the
architecture then I can live with summit_x86 in the main line.

James