2003-05-15 02:28:52

by john stultz

[permalink] [raw]
Subject: [PATCH] linux-2.5.69_subarch-fix_A0.patch

All,
This patch fixes a circular dependency (a function in mach_apic.h
requires hard_smp_processor_id() and hard_smp_processor_id() requires
macros from mach_apic.h) that has been in the subarch code for a bit,
but was hacked around with some #ifdefs.

With the inclusion of the generic-subarch the hack was dropped and
bigsmp and summit promptly broke. So this makes things compile again.

thanks
-john


diff -Nru a/include/asm-i386/mach-bigsmp/mach_apic.h b/include/asm-i386/mach-bigsmp/mach_apic.h
--- a/include/asm-i386/mach-bigsmp/mach_apic.h Wed May 14 18:21:20 2003
+++ b/include/asm-i386/mach-bigsmp/mach_apic.h Wed May 14 18:21:20 2003
@@ -40,10 +40,18 @@

#define apicid_cluster(apicid) (apicid & 0xF0)

+static inline unsigned get_apic_id(unsigned long x)
+{
+ return (((x)>>24)&0x0F);
+}
+
+#define GET_APIC_ID(x) get_apic_id(x)
+
static inline unsigned long calculate_ldr(unsigned long old)
{
unsigned long id;
- id = xapic_phys_to_log_apicid(hard_smp_processor_id());
+ id = xapic_phys_to_log_apicid(
+ GET_APIC_LOGICAL_ID(*(unsigned long *)(APIC_BASE+APIC_LDR)));
return ((old & ~APIC_LDR_MASK) | SET_APIC_LOGICAL_ID(id));
}

@@ -128,13 +136,6 @@
}

#define APIC_ID_MASK (0x0F<<24)
-
-static inline unsigned get_apic_id(unsigned long x)
-{
- return (((x)>>24)&0x0F);
-}
-
-#define GET_APIC_ID(x) get_apic_id(x)

static inline unsigned int cpu_mask_to_apicid (unsigned long cpumask)
{
diff -Nru a/include/asm-i386/mach-summit/mach_apic.h b/include/asm-i386/mach-summit/mach_apic.h
--- a/include/asm-i386/mach-summit/mach_apic.h Wed May 14 18:21:20 2003
+++ b/include/asm-i386/mach-summit/mach_apic.h Wed May 14 18:21:20 2003
@@ -48,12 +48,20 @@

extern u8 bios_cpu_apicid[];

+static inline unsigned get_apic_id(unsigned long x)
+{
+ return (((x)>>24)&0xFF);
+}
+
+#define GET_APIC_ID(x) get_apic_id(x)
+
static inline void init_apic_ldr(void)
{
unsigned long val, id;

if (x86_summit)
- id = xapic_phys_to_log_apicid(hard_smp_processor_id());
+ id = xapic_phys_to_log_apicid(
+ GET_APIC_ID(*(unsigned long *)(APIC_BASE+APIC_ID)));
else
id = 1UL << smp_processor_id();
apic_write_around(APIC_DFR, APIC_DFR_VALUE);
@@ -136,13 +144,6 @@
}

#define APIC_ID_MASK (0xFF<<24)
-
-static inline unsigned get_apic_id(unsigned long x)
-{
- return (((x)>>24)&0xFF);
-}
-
-#define GET_APIC_ID(x) get_apic_id(x)

static inline unsigned int cpu_mask_to_apicid (unsigned long cpumask)
{




2003-05-15 06:38:32

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] linux-2.5.69_subarch-fix_A0.patch

On Wed, May 14, 2003 at 07:37:09PM -0700, john stultz wrote:
> All,
> This patch fixes a circular dependency (a function in mach_apic.h
> requires hard_smp_processor_id() and hard_smp_processor_id() requires
> macros from mach_apic.h) that has been in the subarch code for a bit,
> but was hacked around with some #ifdefs.
>
> With the inclusion of the generic-subarch the hack was dropped and
> bigsmp and summit promptly broke. So this makes things compile again.

What broke exactly? I thought worked around this problem for the generic
subarchitecture.

Accessing the APIC directly this way just to work around a bad include
looks quite ugly to me.

-Andi

2003-05-15 17:12:58

by john stultz

[permalink] [raw]
Subject: Re: [PATCH] linux-2.5.69_subarch-fix_A0.patch

On Wed, 2003-05-14 at 23:51, Andi Kleen wrote:
> On Wed, May 14, 2003 at 07:37:09PM -0700, john stultz wrote:
> > All,
> > This patch fixes a circular dependency (a function in mach_apic.h
> > requires hard_smp_processor_id() and hard_smp_processor_id() requires
> > macros from mach_apic.h) that has been in the subarch code for a bit,
> > but was hacked around with some #ifdefs.
> >
> > With the inclusion of the generic-subarch the hack was dropped and
> > bigsmp and summit promptly broke. So this makes things compile again.
>
> What broke exactly? I thought worked around this problem for the generic
> subarchitecture.


GET_APIC_ID was moved into mach_apic.h (it used to be wrapped in
CONFIG_X86_SUMMIT in apicdef.h). However, init_apic_ldr() in
mach_apic.h uses hard_smp_processor_id() (in smp.h), which to complete
the circle, requires GET_APIC_ID from mach_apic.h

You did get around it in the generic subarch (which I love, by the way,
thanks so much for doing that work), but in a roundabout way. (via
#ifdef APIC_DEFINITION trickery).


> Accessing the APIC directly this way just to work around a bad include
> looks quite ugly to me.

Yea, ok. I guess we can move GET_APIC_ID out into its own mach specific
.h file. That will eliminate the circularity as well. I'll respin the
patch later today.

thanks
-john


2003-05-15 17:16:07

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] linux-2.5.69_subarch-fix_A0.patch

> You did get around it in the generic subarch (which I love, by the way,
> thanks so much for doing that work), but in a roundabout way. (via
> #ifdef APIC_DEFINITION trickery).

The best fix is probably to just remove the summit selection and replace
it with the generic architecture.

-Andi

2003-05-15 17:37:48

by john stultz

[permalink] [raw]
Subject: Re: [PATCH] linux-2.5.69_subarch-fix_A0.patch

On Thu, 2003-05-15 at 10:28, Andi Kleen wrote:
> > You did get around it in the generic subarch (which I love, by the way,
> > thanks so much for doing that work), but in a roundabout way. (via
> > #ifdef APIC_DEFINITION trickery).
>
> The best fix is probably to just remove the summit selection and replace
> it with the generic architecture.

I'd agree (long term even more strongly), although along with that I'd
like to be able to pick and choose my subarch. So I can have a kernel
that supports say, PC and BigSMP, but not NUMAQ or whatever. I believe
this is doable with your infrastructure, but I'm not sure how much work
it will take.

And before we make any larger changes, I'd like to just get stuff
compiling first (Keith's apic patch landing about the same time and
broke generic, and generic broke summit so right now neither subarch
builds properly).

thanks
-john




2003-05-15 18:47:18

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] linux-2.5.69_subarch-fix_A0.patch

On Thu, May 15, 2003 at 10:46:03AM -0700, john stultz wrote:
> On Thu, 2003-05-15 at 10:28, Andi Kleen wrote:
> > > You did get around it in the generic subarch (which I love, by the way,
> > > thanks so much for doing that work), but in a roundabout way. (via
> > > #ifdef APIC_DEFINITION trickery).
> >
> > The best fix is probably to just remove the summit selection and replace
> > it with the generic architecture.
>
> I'd agree (long term even more strongly), although along with that I'd
> like to be able to pick and choose my subarch. So I can have a kernel
> that supports say, PC and BigSMP, but not NUMAQ or whatever. I believe
> this is doable with your infrastructure, but I'm not sure how much work
> it will take.

NUMAQ is not supported by the generic subarchitecture anyways.

The only supported architecturs by generic are pc, bigsmp, summit.
In theory you could subselect them, but it's only a few bytes for each
so it's probably not worth the effort. Technically it isn't a big issue,
you would just need to add it to Kconfig (not sure how to do that cleanly),
the Makefile and the probe table.


-Andi

2003-05-15 19:07:13

by William Lee Irwin III

[permalink] [raw]
Subject: Re: [PATCH] linux-2.5.69_subarch-fix_A0.patch

On Thu, May 15, 2003 at 10:46:03AM -0700, john stultz wrote:
>> I'd agree (long term even more strongly), although along with that I'd
>> like to be able to pick and choose my subarch. So I can have a kernel
>> that supports say, PC and BigSMP, but not NUMAQ or whatever. I believe
>> this is doable with your infrastructure, but I'm not sure how much work
>> it will take.

On Thu, May 15, 2003 at 09:00:06PM +0200, Andi Kleen wrote:
> NUMAQ is not supported by the generic subarchitecture anyways.
> The only supported architecturs by generic are pc, bigsmp, summit.
> In theory you could subselect them, but it's only a few bytes for each
> so it's probably not worth the effort. Technically it isn't a big issue,
> you would just need to add it to Kconfig (not sure how to do that cleanly),
> the Makefile and the probe table.

Okay, will fix.


-- wli

2003-05-15 19:19:28

by Martin J. Bligh

[permalink] [raw]
Subject: Re: [PATCH] linux-2.5.69_subarch-fix_A0.patch

> On Thu, May 15, 2003 at 10:46:03AM -0700, john stultz wrote:
>>> I'd agree (long term even more strongly), although along with that I'd
>>> like to be able to pick and choose my subarch. So I can have a kernel
>>> that supports say, PC and BigSMP, but not NUMAQ or whatever. I believe
>>> this is doable with your infrastructure, but I'm not sure how much work
>>> it will take.
>
> On Thu, May 15, 2003 at 09:00:06PM +0200, Andi Kleen wrote:
>> NUMAQ is not supported by the generic subarchitecture anyways.
>> The only supported architecturs by generic are pc, bigsmp, summit.
>> In theory you could subselect them, but it's only a few bytes for each
>> so it's probably not worth the effort. Technically it isn't a big issue,
>> you would just need to add it to Kconfig (not sure how to do that cleanly),
>> the Makefile and the probe table.
>
> Okay, will fix.

Will fix what? Nothing seems to be broken ...

If you mean putting NUMA-Q under generic subarch, please don't.
If you mean "selectable sub-bits of generic subarch" that seems utterly
pointless to me, the whole point of this is to get a single kernel
image for distros for all machines ...

M.

2003-05-15 19:37:24

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] linux-2.5.69_subarch-fix_A0.patch

> Okay, will fix.

What do you want to fix? Really - an subarchitecture costs 1-2 K,
it's not worth any effort to make it more finegrained.

-Andi

2003-05-15 19:43:14

by William Lee Irwin III

[permalink] [raw]
Subject: Re: [PATCH] linux-2.5.69_subarch-fix_A0.patch

At some point in the past, I wrote:
>> Okay, will fix.

On Thu, May 15, 2003 at 09:50:13PM +0200, Andi Kleen wrote:
> What do you want to fix? Really - an subarchitecture costs 1-2 K,
> it's not worth any effort to make it more finegrained.

Finegrained I didn't care about. I meant the subarch hooks.


-- wli

2003-05-15 19:46:50

by Martin J. Bligh

[permalink] [raw]
Subject: Re: [PATCH] linux-2.5.69_subarch-fix_A0.patch

>>> Okay, will fix.
>
> On Thu, May 15, 2003 at 09:50:13PM +0200, Andi Kleen wrote:
>> What do you want to fix? Really - an subarchitecture costs 1-2 K,
>> it's not worth any effort to make it more finegrained.
>
> Finegrained I didn't care about. I meant the subarch hooks.

For what? They work already AFAICS, apart from the issue that John
is already dealing with.

M.