2003-01-13 18:35:55

by Protasevich, Natalie

[permalink] [raw]
Subject: APIC version

I have a little patch here for the APIC version.
Without it, I get version 0x10 instead of 0x14 for Fosters/Gallatins
(booting with ACPI):

--- mpparse.c.org 2003-01-13 11:32:18.000000000 -0700
+++ mpparse.c 2003-01-13 11:33:26.000000000 -0700
@@ -773,6 +773,8 @@
if (boot_cpu_physical_apicid == -1U)
boot_cpu_physical_apicid = GET_APIC_ID(apic_read(APIC_ID));

+ apic_version[smp_processor_id()] =
GET_APIC_VERSION(apic_read(APIC_LVR));
+
Dprintk("Boot CPU = %d\n", boot_cpu_physical_apicid);
}

@@ -795,7 +797,7 @@

processor.mpc_type = MP_PROCESSOR;
processor.mpc_apicid = id;
- processor.mpc_apicver = 0x10; /* TBD: lapic version */
+ processor.mpc_apicver = apic_version[smp_processor_id()];
processor.mpc_cpuflag = (enabled ? CPU_ENABLED : 0);
processor.mpc_cpuflag |= (boot_cpu ? CPU_BOOTPROCESSOR : 0);
processor.mpc_cpufeature = (boot_cpu_data.x86 << 8) |


It seems to work OK for me, although you may find some implications...
Anyway -

Thanks,

--Natalie


2003-01-13 18:50:54

by Nakajima, Jun

[permalink] [raw]
Subject: RE: APIC version

The entries in acpi_version[] are indexed by the APIC id, not smp_processor_id(). So you can overwrite acpi_version[] for a different processor.

Jun


> -----Original Message-----
> From: Protasevich, Natalie [mailto:[email protected]]
> Sent: Monday, January 13, 2003 10:44 AM
> To: 'Martin J. Bligh'; Pallipadi, Venkatesh
> Cc: 'William Lee Irwin III'; Nakajima, Jun; 'Christoph Hellwig'; 'James
> Cleverdon'; 'Linux Kernel'; Protasevich, Natalie
> Subject: APIC version
>
> I have a little patch here for the APIC version.
> Without it, I get version 0x10 instead of 0x14 for Fosters/Gallatins
> (booting with ACPI):
>
> --- mpparse.c.org 2003-01-13 11:32:18.000000000 -0700
> +++ mpparse.c 2003-01-13 11:33:26.000000000 -0700
> @@ -773,6 +773,8 @@
> if (boot_cpu_physical_apicid == -1U)
> boot_cpu_physical_apicid = GET_APIC_ID(apic_read(APIC_ID));
>
> + apic_version[smp_processor_id()] =
> GET_APIC_VERSION(apic_read(APIC_LVR));
> +
> Dprintk("Boot CPU = %d\n", boot_cpu_physical_apicid);
> }
>
> @@ -795,7 +797,7 @@
>
> processor.mpc_type = MP_PROCESSOR;
> processor.mpc_apicid = id;
> - processor.mpc_apicver = 0x10; /* TBD: lapic version */
> + processor.mpc_apicver = apic_version[smp_processor_id()];
> processor.mpc_cpuflag = (enabled ? CPU_ENABLED : 0);
> processor.mpc_cpuflag |= (boot_cpu ? CPU_BOOTPROCESSOR : 0);
> processor.mpc_cpufeature = (boot_cpu_data.x86 << 8) |
>
>
> It seems to work OK for me, although you may find some implications...
> Anyway -
>
> Thanks,
>
> --Natalie

2003-01-13 19:25:58

by Martin J. Bligh

[permalink] [raw]
Subject: RE: APIC version

> The entries in acpi_version[] are indexed by the APIC id, not smp_processor_id(). So you can overwrite acpi_version[] for a different processor.

>> + apic_version[smp_processor_id()] =

Ugh.

It's indexed by the apic ID, not the cpu id. They're not 1-1.


2003-01-13 19:31:59

by Protasevich, Natalie

[permalink] [raw]
Subject: RE: APIC version

Sould it be hard_smp_processor_id()?

-----Original Message-----
From: Nakajima, Jun [mailto:[email protected]]
Sent: Monday, January 13, 2003 12:00 PM
To: Protasevich, Natalie; Martin J. Bligh; Pallipadi, Venkatesh
Cc: William Lee Irwin III; Christoph Hellwig; James Cleverdon; Linux
Kernel
Subject: RE: APIC version


The entries in acpi_version[] are indexed by the APIC id, not
smp_processor_id(). So you can overwrite acpi_version[] for a different
processor.

Jun


> -----Original Message-----
> From: Protasevich, Natalie [mailto:[email protected]]
> Sent: Monday, January 13, 2003 10:44 AM
> To: 'Martin J. Bligh'; Pallipadi, Venkatesh
> Cc: 'William Lee Irwin III'; Nakajima, Jun; 'Christoph Hellwig'; 'James
> Cleverdon'; 'Linux Kernel'; Protasevich, Natalie
> Subject: APIC version
>
> I have a little patch here for the APIC version.
> Without it, I get version 0x10 instead of 0x14 for Fosters/Gallatins
> (booting with ACPI):
>
> --- mpparse.c.org 2003-01-13 11:32:18.000000000 -0700
> +++ mpparse.c 2003-01-13 11:33:26.000000000 -0700
> @@ -773,6 +773,8 @@
> if (boot_cpu_physical_apicid == -1U)
> boot_cpu_physical_apicid = GET_APIC_ID(apic_read(APIC_ID));
>
> + apic_version[smp_processor_id()] =
> GET_APIC_VERSION(apic_read(APIC_LVR));
> +
> Dprintk("Boot CPU = %d\n", boot_cpu_physical_apicid);
> }
>
> @@ -795,7 +797,7 @@
>
> processor.mpc_type = MP_PROCESSOR;
> processor.mpc_apicid = id;
> - processor.mpc_apicver = 0x10; /* TBD: lapic version */
> + processor.mpc_apicver = apic_version[smp_processor_id()];
> processor.mpc_cpuflag = (enabled ? CPU_ENABLED : 0);
> processor.mpc_cpuflag |= (boot_cpu ? CPU_BOOTPROCESSOR : 0);
> processor.mpc_cpufeature = (boot_cpu_data.x86 << 8) |
>
>
> It seems to work OK for me, although you may find some implications...
> Anyway -
>
> Thanks,
>
> --Natalie

2003-01-13 19:46:47

by Protasevich, Natalie

[permalink] [raw]
Subject: RE: APIC version


The thing is, this array never gets filled with correct values for XAPICs in
case when we go through ACPI, not MP.
I only care about it because that is how I used to distinguish Cascades from
Fosters on ES7000 (which I need to do very early, before I get to APIC mode
setup).

On the other hand, I just noticed that I could go with smp_num_siblings for
this purpose... so if this issue is too shady as of now, i will survive, I
guess...:)

--Natalie


-----Original Message-----
From: Martin J. Bligh [mailto:[email protected]]
Sent: Monday, January 13, 2003 12:26 PM
To: Nakajima, Jun; Protasevich, Natalie; Pallipadi, Venkatesh
Cc: William Lee Irwin III; Christoph Hellwig; James Cleverdon; Linux
Kernel
Subject: RE: APIC version


> The entries in acpi_version[] are indexed by the APIC id, not
smp_processor_id(). So you can overwrite acpi_version[] for a different
processor.

>> + apic_version[smp_processor_id()] =

Ugh.

It's indexed by the apic ID, not the cpu id. They're not 1-1.

2003-01-13 20:04:22

by Nakajima, Jun

[permalink] [raw]
Subject: RE: APIC version

The only thing you need to do is
processor.mpc_type = MP_PROCESSOR;
processor.mpc_apicid = id;
- processor.mpc_apicver = 0x10; /* TBD: lapic version */
+ processor.mpc_apicver = GET_APIC_VERSION(apic_read(APIC_LVR));
processor.mpc_cpuflag = (enabled ? CPU_ENABLED : 0);
processor.mpc_cpuflag |= (boot_cpu ? CPU_BOOTPROCESSOR : 0);
processor.mpc_cpufeature = (boot_cpu_data.x86 << 8) |

Jun

> -----Original Message-----
> From: Protasevich, Natalie [mailto:[email protected]]
> Sent: Monday, January 13, 2003 11:55 AM
> To: 'Martin J. Bligh'; Nakajima, Jun; Protasevich, Natalie; Pallipadi,
> Venkatesh
> Cc: William Lee Irwin III; Christoph Hellwig; James Cleverdon; Linux
> Kernel
> Subject: RE: APIC version
>
>
> The thing is, this array never gets filled with correct values for XAPICs
> in
> case when we go through ACPI, not MP.
> I only care about it because that is how I used to distinguish Cascades
> from
> Fosters on ES7000 (which I need to do very early, before I get to APIC
> mode
> setup).
>
> On the other hand, I just noticed that I could go with smp_num_siblings
> for
> this purpose... so if this issue is too shady as of now, i will survive, I
> guess...:)
>
> --Natalie
>
>
> -----Original Message-----
> From: Martin J. Bligh [mailto:[email protected]]
> Sent: Monday, January 13, 2003 12:26 PM
> To: Nakajima, Jun; Protasevich, Natalie; Pallipadi, Venkatesh
> Cc: William Lee Irwin III; Christoph Hellwig; James Cleverdon; Linux
> Kernel
> Subject: RE: APIC version
>
>
> > The entries in acpi_version[] are indexed by the APIC id, not
> smp_processor_id(). So you can overwrite acpi_version[] for a different
> processor.
>
> >> + apic_version[smp_processor_id()] =
>
> Ugh.
>
> It's indexed by the apic ID, not the cpu id. They're not 1-1.

2003-01-13 20:17:04

by Protasevich, Natalie

[permalink] [raw]
Subject: RE: APIC version

Wow, this is pretty brilliant, Jun! In cases like this one always thinks
"Why this didn't occur to me, this is so obvious..."
(Sadly I noticed that I tend to go "round and about" sometimes instead of
direct :(
I hope this will get incorporated in the source tree.

Thanks,

--Natalie


-----Original Message-----
From: Nakajima, Jun [mailto:[email protected]]
Sent: Monday, January 13, 2003 1:13 PM
To: Protasevich, Natalie; Martin J. Bligh; Pallipadi, Venkatesh
Cc: William Lee Irwin III; Christoph Hellwig; James Cleverdon; Linux
Kernel
Subject: RE: APIC version


The only thing you need to do is
processor.mpc_type = MP_PROCESSOR;
processor.mpc_apicid = id;
- processor.mpc_apicver = 0x10; /* TBD: lapic version */
+ processor.mpc_apicver = GET_APIC_VERSION(apic_read(APIC_LVR));
processor.mpc_cpuflag = (enabled ? CPU_ENABLED : 0);
processor.mpc_cpuflag |= (boot_cpu ? CPU_BOOTPROCESSOR : 0);
processor.mpc_cpufeature = (boot_cpu_data.x86 << 8) |

Jun

>

2003-01-13 22:44:36

by Zwane Mwaikambo

[permalink] [raw]
Subject: RE: APIC version

On Mon, 13 Jan 2003, Nakajima, Jun wrote:

> The entries in acpi_version[] are indexed by the APIC id, not
> smp_processor_id(). So you can overwrite acpi_version[] for a different
> processor.

Is it possible to use smp_processor_id instead to avoid wasting memory
for the sparse APIC id case?

Zwane
--
function.linuxpower.ca

2003-01-13 23:06:38

by Protasevich, Natalie

[permalink] [raw]
Subject: RE: APIC version

As for apic_version[] indexing in general, I am also for smp_processor_id,
although it could take a few changes in apic.c, mpparse.c, and especially
smpboot.c. Speaking of es7000, its APIC ID's are always huge, defined by
fixed topology. For those like us, the array would shrink from MAX_APICS
(256) to NR_CPUS (32, maybe 64). I wonder if it would do any good for (true)
NUMA.

--Natalie

-----Original Message-----
From: Zwane Mwaikambo [mailto:[email protected]]
Sent: Monday, January 13, 2003 3:53 PM
To: Nakajima, Jun
Cc: Protasevich, Natalie; Martin J. Bligh; Pallipadi, Venkatesh; William
Lee Irwin III; Christoph Hellwig; James Cleverdon; Linux Kernel
Subject: RE: APIC version


On Mon, 13 Jan 2003, Nakajima, Jun wrote:

> The entries in acpi_version[] are indexed by the APIC id, not
> smp_processor_id(). So you can overwrite acpi_version[] for a different
> processor.

Is it possible to use smp_processor_id instead to avoid wasting memory
for the sparse APIC id case?

Zwane
--
function.linuxpower.ca

2003-01-13 23:21:32

by Martin J. Bligh

[permalink] [raw]
Subject: RE: APIC version

>> The entries in acpi_version[] are indexed by the APIC id, not
>> smp_processor_id(). So you can overwrite acpi_version[] for a different
>> processor.
>
> Is it possible to use smp_processor_id instead to avoid wasting memory
> for the sparse APIC id case?

No, the array is set up in mpparse.c before we know the real processor
numbers.

M.

2003-01-13 23:26:32

by Nakajima, Jun

[permalink] [raw]
Subject: RE: APIC version

I don't think the array apic_version[] is very helpful in the first place.
I suspect it can be __init.

Jun

> -----Original Message-----
> From: Protasevich, Natalie [mailto:[email protected]]
> Sent: Monday, January 13, 2003 3:15 PM
> To: 'Zwane Mwaikambo'; Nakajima, Jun
> Cc: Protasevich, Natalie; Martin J. Bligh; Pallipadi, Venkatesh; William
> Lee Irwin III; Christoph Hellwig; James Cleverdon; Linux Kernel
> Subject: RE: APIC version
>
> As for apic_version[] indexing in general, I am also for smp_processor_id,
> although it could take a few changes in apic.c, mpparse.c, and especially
> smpboot.c. Speaking of es7000, its APIC ID's are always huge, defined by
> fixed topology. For those like us, the array would shrink from MAX_APICS
> (256) to NR_CPUS (32, maybe 64). I wonder if it would do any good for
> (true)
> NUMA.
>
> --Natalie
>
> -----Original Message-----
> From: Zwane Mwaikambo [mailto:[email protected]]
> Sent: Monday, January 13, 2003 3:53 PM
> To: Nakajima, Jun
> Cc: Protasevich, Natalie; Martin J. Bligh; Pallipadi, Venkatesh; William
> Lee Irwin III; Christoph Hellwig; James Cleverdon; Linux Kernel
> Subject: RE: APIC version
>
>
> On Mon, 13 Jan 2003, Nakajima, Jun wrote:
>
> > The entries in acpi_version[] are indexed by the APIC id, not
> > smp_processor_id(). So you can overwrite acpi_version[] for a different
> > processor.
>
> Is it possible to use smp_processor_id instead to avoid wasting memory
> for the sparse APIC id case?
>
> Zwane
> --
> function.linuxpower.ca

2003-01-13 23:44:38

by Protasevich, Natalie

[permalink] [raw]
Subject: RE: APIC version

If you index it by 4-bit GET_APIC_ID() (not GET_APIC_LOGICAL_ID()), i.e.
hard_smp_processor_id(), you can get away with it.

Of course, it is possible that it can just be "don't care":

>I don't think the array apic_version[] is very helpful in the first place.
>I suspect it can be __init.

>Jun

On the other hand, it might be needed if imagine hot plug CPU case.


--Natalie



-----Original Message-----
From: Martin J. Bligh [mailto:[email protected]]
Sent: Monday, January 13, 2003 4:23 PM
To: Zwane Mwaikambo; Nakajima, Jun
Cc: Protasevich, Natalie; Pallipadi, Venkatesh; William Lee Irwin III;
Christoph Hellwig; James Cleverdon; Linux Kernel
Subject: RE: APIC version


>> The entries in acpi_version[] are indexed by the APIC id, not
>> smp_processor_id(). So you can overwrite acpi_version[] for a different
>> processor.
>
> Is it possible to use smp_processor_id instead to avoid wasting memory
> for the sparse APIC id case?

No, the array is set up in mpparse.c before we know the real processor
numbers.

M.

2003-01-13 23:52:53

by Martin J. Bligh

[permalink] [raw]
Subject: RE: APIC version

> If you index it by 4-bit GET_APIC_ID() (not GET_APIC_LOGICAL_ID()), i.e.
> hard_smp_processor_id(), you can get away with it.

Not on clustered mode platforms - the physical address is not unique.

> Of course, it is possible that it can just be "don't care":

;-)

M.

2003-01-14 00:01:07

by Protasevich, Natalie

[permalink] [raw]
Subject: RE: APIC version


>> If you index it by 4-bit GET_APIC_ID() (not GET_APIC_LOGICAL_ID()), i.e.
>> hard_smp_processor_id(), you can get away with it.

Oops, you're right! what was I thinking...

2003-01-14 03:12:33

by Nakajima, Jun

[permalink] [raw]
Subject: RE: APIC version

We can gather that info at runtime from the processors, when we really need it. And I don't think we have performance issues this that.

Jun

> -----Original Message-----
> From: Protasevich, Natalie [mailto:[email protected]]
> Sent: Monday, January 13, 2003 3:49 PM
> To: 'Martin J. Bligh'; Zwane Mwaikambo; Nakajima, Jun
> Cc: Protasevich, Natalie; Pallipadi, Venkatesh; William Lee Irwin III;
> Christoph Hellwig; James Cleverdon; Linux Kernel
> Subject: RE: APIC version
>
> If you index it by 4-bit GET_APIC_ID() (not GET_APIC_LOGICAL_ID()), i.e.
> hard_smp_processor_id(), you can get away with it.
>
> Of course, it is possible that it can just be "don't care":
>
> >I don't think the array apic_version[] is very helpful in the first
> place.
> >I suspect it can be __init.
>
> >Jun
>
> On the other hand, it might be needed if imagine hot plug CPU case.
>
>
> --Natalie
>
>
>
> -----Original Message-----
> From: Martin J. Bligh [mailto:[email protected]]
> Sent: Monday, January 13, 2003 4:23 PM
> To: Zwane Mwaikambo; Nakajima, Jun
> Cc: Protasevich, Natalie; Pallipadi, Venkatesh; William Lee Irwin III;
> Christoph Hellwig; James Cleverdon; Linux Kernel
> Subject: RE: APIC version
>
>
> >> The entries in acpi_version[] are indexed by the APIC id, not
> >> smp_processor_id(). So you can overwrite acpi_version[] for a different
> >> processor.
> >
> > Is it possible to use smp_processor_id instead to avoid wasting memory
> > for the sparse APIC id case?
>
> No, the array is set up in mpparse.c before we know the real processor
> numbers.
>
> M.

2003-01-14 03:47:23

by Protasevich, Natalie

[permalink] [raw]
Subject: RE: APIC version

>We can gather that info at runtime from the processors, when we really need
it. And I don't think we have
>performance issues this that.

True - it has to be only done once per CPU bring-up.

To investigate all the corners of this issue: is it possible now, tomorrow,
on in the future to mix Intel processors on the same machine? Isn't it
enough really to collect the APIC version of boot CPU and just use it for
the rest? Or do we have to leave the opportunity for such occasion in the
code?


-Natalie

2003-01-14 04:35:32

by Martin J. Bligh

[permalink] [raw]
Subject: RE: APIC version

> True - it has to be only done once per CPU bring-up.
>
> To investigate all the corners of this issue: is it possible now, tomorrow,
> on in the future to mix Intel processors on the same machine?

Yes. We can mix PPro 180s up with P3 900s, for a long time now.

> enough really to collect the APIC version of boot CPU and just use it for
> the rest?

Nope.

M.

2003-01-14 04:42:59

by Protasevich, Natalie

[permalink] [raw]
Subject: RE: APIC version

OK, Martin. You win. I am satisfied and I give up. I guess I like
apic_version[] the way it is now :)

--N
-----Original Message-----
From: Martin J. Bligh [mailto:[email protected]]
Sent: Monday, January 13, 2003 9:44 PM
To: Protasevich, Natalie; 'Nakajima, Jun'; Zwane Mwaikambo
Cc: Pallipadi, Venkatesh; Linux Kernel
Subject: RE: APIC version


> True - it has to be only done once per CPU bring-up.
>
> To investigate all the corners of this issue: is it possible now,
tomorrow,
> on in the future to mix Intel processors on the same machine?

Yes. We can mix PPro 180s up with P3 900s, for a long time now.

> enough really to collect the APIC version of boot CPU and just use it for
> the rest?

Nope.

M.