LinuxLists.cc - [PATCH] x86: APIC: Remove apic_write

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

* Maciej W. Rozycki <[email protected]> wrote:

> Use alternatives to select the workaround for the 11AP Pentium
> erratum for the affected steppings on the fly rather than build time.
> Remove the X86_GOOD_APIC configuration option and replace all the
> calls to apic_write_around() with plain apic_write(), protecting
> accesses to the ESR as appropriate due to the 3AP Pentium erratum.
> Remove apic_read_around() and all its invocations altogether as not
> needed. Remove apic_write_atomic() and all its implementing backends.
> The use of ASM_OUTPUT2() is not strictly needed for input constraints,
> but I have used it for readability's sake.

wow, impressive patch!

> I had the feeling no one else was brave enough to do it, so I went
> ahead and here it is. Verified by checking the generated assembly and
> tested with both a 32-bit and a 64-bit configuration, also with the
> 11AP "feature" forced on and verified with gdb on /proc/kcore to work
> as expected (as an 11AP machines are quite hard to get hands on these
> days). Some script complained about the use of "volatile", but
> apic_write() needs it for the same reason and is effectively a
> replacement for writel(), so I have disregarded it.

kudos for pulling this off :-)

applied to tip/x86/apic. This will interact with tip/x86/x2apic but that
topic is being worked on anyway. The interaction should be trivial.

Ingo

2008-07-18 16:26:08

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, Jul 18, 2008 at 3:52 AM, Ingo Molnar <[email protected]> wrote:
>
> * Maciej W. Rozycki <[email protected]> wrote:
>
>> Use alternatives to select the workaround for the 11AP Pentium
>> erratum for the affected steppings on the fly rather than build time.
>> Remove the X86_GOOD_APIC configuration option and replace all the
>> calls to apic_write_around() with plain apic_write(), protecting
>> accesses to the ESR as appropriate due to the 3AP Pentium erratum.
>> Remove apic_read_around() and all its invocations altogether as not
>> needed. Remove apic_write_atomic() and all its implementing backends.
>> The use of ASM_OUTPUT2() is not strictly needed for input constraints,
>> but I have used it for readability's sake.
>
> wow, impressive patch!
>
>> I had the feeling no one else was brave enough to do it, so I went
>> ahead and here it is. Verified by checking the generated assembly and
>> tested with both a 32-bit and a 64-bit configuration, also with the
>> 11AP "feature" forced on and verified with gdb on /proc/kcore to work
>> as expected (as an 11AP machines are quite hard to get hands on these
>> days). Some script complained about the use of "volatile", but
>> apic_write() needs it for the same reason and is effectively a
>> replacement for writel(), so I have disregarded it.
>
> kudos for pulling this off :-)
>
> applied to tip/x86/apic. This will interact with tip/x86/x2apic but that
> topic is being worked on anyway. The interaction should be trivial.

it seems we should start apic related merging after x2apic get merged?

YH

2008-07-18 17:01:19

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, Jul 18, 2008 at 9:25 AM, Yinghai Lu <[email protected]> wrote:
> On Fri, Jul 18, 2008 at 3:52 AM, Ingo Molnar <[email protected]> wrote:
>>
>> * Maciej W. Rozycki <[email protected]> wrote:
>>
>>> Use alternatives to select the workaround for the 11AP Pentium
>>> erratum for the affected steppings on the fly rather than build time.
>>> Remove the X86_GOOD_APIC configuration option and replace all the
>>> calls to apic_write_around() with plain apic_write(), protecting
>>> accesses to the ESR as appropriate due to the 3AP Pentium erratum.
>>> Remove apic_read_around() and all its invocations altogether as not
>>> needed. Remove apic_write_atomic() and all its implementing backends.
>>> The use of ASM_OUTPUT2() is not strictly needed for input constraints,
>>> but I have used it for readability's sake.
>>
>> wow, impressive patch!
>>
>>> I had the feeling no one else was brave enough to do it, so I went
>>> ahead and here it is. Verified by checking the generated assembly and
>>> tested with both a 32-bit and a 64-bit configuration, also with the
>>> 11AP "feature" forced on and verified with gdb on /proc/kcore to work
>>> as expected (as an 11AP machines are quite hard to get hands on these
>>> days). Some script complained about the use of "volatile", but
>>> apic_write() needs it for the same reason and is effectively a
>>> replacement for writel(), so I have disregarded it.
>>
>> kudos for pulling this off :-)
>>
>> applied to tip/x86/apic. This will interact with tip/x86/x2apic but that
>> topic is being worked on anyway. The interaction should be trivial.
>
> it seems we should start apic related merging after x2apic get merged?
>

> git merge tip/x86/x2apic
Auto-merged Documentation/kernel-parameters.txt
Auto-merged arch/x86/Kconfig
Auto-merged arch/x86/kernel/Makefile
CONFLICT (content): Merge conflict in arch/x86/kernel/Makefile
Auto-merged arch/x86/kernel/acpi/boot.c
Auto-merged arch/x86/kernel/apic_32.c
Auto-merged arch/x86/kernel/apic_64.c
Auto-merged arch/x86/kernel/cpu/common_64.c
Auto-merged arch/x86/kernel/genx2apic_uv_x.c
Auto-merged arch/x86/kernel/io_apic_32.c
Auto-merged arch/x86/kernel/io_apic_64.c
Auto-merged arch/x86/kernel/paravirt.c
CONFLICT (content): Merge conflict in arch/x86/kernel/paravirt.c
Auto-merged arch/x86/kernel/setup.c
Auto-merged arch/x86/kernel/smpboot.c
CONFLICT (content): Merge conflict in arch/x86/kernel/smpboot.c
Auto-merged arch/x86/kernel/vmi_32.c
CONFLICT (content): Merge conflict in arch/x86/kernel/vmi_32.c
Auto-merged arch/x86/lguest/boot.c
Auto-merged arch/x86/xen/enlighten.c
CONFLICT (content): Merge conflict in arch/x86/xen/enlighten.c
Auto-merged drivers/pci/Makefile
Auto-merged drivers/pci/intel-iommu.c
Auto-merged include/asm-x86/apic.h
CONFLICT (content): Merge conflict in include/asm-x86/apic.h
Auto-merged include/asm-x86/cpufeature.h
Auto-merged include/asm-x86/genapic_64.h
Auto-merged include/asm-x86/hw_irq.h
Auto-merged include/asm-x86/io_apic.h
Auto-merged include/asm-x86/ipi.h
Auto-merged include/asm-x86/mach-default/mach_apic.h
Auto-merged include/asm-x86/mach-es7000/mach_apic.h
Auto-merged include/asm-x86/paravirt.h
CONFLICT (content): Merge conflict in include/asm-x86/paravirt.h
Auto-merged include/asm-x86/smp.h
Auto-merged include/linux/irq.h
Auto-merged kernel/irq/manage.c
Automatic merge failed; fix conflicts and then commit the result.

also please apply
Fix VMI apic_ops.
from Suresh

to x2apic

YH

2008-07-18 21:11:45

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

* Yinghai Lu <[email protected]> wrote:

> > git merge tip/x86/x2apic
> CONFLICT (content): Merge conflict in arch/x86/kernel/Makefile
> CONFLICT (content): Merge conflict in arch/x86/kernel/paravirt.c
> CONFLICT (content): Merge conflict in arch/x86/kernel/smpboot.c
> CONFLICT (content): Merge conflict in arch/x86/kernel/vmi_32.c
> CONFLICT (content): Merge conflict in arch/x86/xen/enlighten.c
> CONFLICT (content): Merge conflict in include/asm-x86/apic.h
> CONFLICT (content): Merge conflict in include/asm-x86/paravirt.h

that's due to the changes in tip/x86/apic and in tip/x86/uv.

ok, i've just merged x86/apic into x86/x2apic and x86/uv as well, and
pushed out the result.

Note: it's a first raw merge and completely untested. It will now merge
cleanly into tip/master. There are probably a few details missing.

The smp-alternatives based trick that Maciej applied for write-around
can stay in the mem_apic_* methods - modern APICs do not need any
workaround in this area and can just use apic_ops.

> also please apply
> Fix VMI apic_ops.
> from Suresh
>
> to x2apic

done.

Ingo

2008-07-18 21:14:16

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, Jul 18, 2008 at 2:03 PM, Ingo Molnar <[email protected]> wrote:
>
> * Yinghai Lu <[email protected]> wrote:
>
>> > git merge tip/x86/x2apic
>> CONFLICT (content): Merge conflict in arch/x86/kernel/Makefile
>> CONFLICT (content): Merge conflict in arch/x86/kernel/paravirt.c
>> CONFLICT (content): Merge conflict in arch/x86/kernel/smpboot.c
>> CONFLICT (content): Merge conflict in arch/x86/kernel/vmi_32.c
>> CONFLICT (content): Merge conflict in arch/x86/xen/enlighten.c
>> CONFLICT (content): Merge conflict in include/asm-x86/apic.h
>> CONFLICT (content): Merge conflict in include/asm-x86/paravirt.h
>
> that's due to the changes in tip/x86/apic and in tip/x86/uv.
>
> ok, i've just merged x86/apic into x86/x2apic and x86/uv as well, and
> pushed out the result.
>
> Note: it's a first raw merge and completely untested. It will now merge
> cleanly into tip/master. There are probably a few details missing.
>
> The smp-alternatives based trick that Maciej applied for write-around
> can stay in the mem_apic_* methods - modern APICs do not need any
> workaround in this area and can just use apic_ops.

CC arch/x86/kernel/apic_64.o
arch/x86/kernel/apic_64.c:170: error: unknown field 'write_atomic'
specified in initializer
arch/x86/kernel/apic_64.c:170: error: 'native_apic_mem_write_atomic'
undeclared here (not in a function)
arch/x86/kernel/apic_64.c:209: error: unknown field 'write_atomic'
specified in initializer
arch/x86/kernel/apic_64.c:209: warning: initialization from
incompatible pointer type
make[1]: *** [arch/x86/kernel/apic_64.o] Error 1
make: *** [arch/x86/kernel] Error 2

YH

2008-07-18 22:03:18

by Maciej W. Rozycki

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, 18 Jul 2008, Yinghai Lu wrote:

> CC arch/x86/kernel/apic_64.o
> arch/x86/kernel/apic_64.c:170: error: unknown field 'write_atomic'
> specified in initializer
> arch/x86/kernel/apic_64.c:170: error: 'native_apic_mem_write_atomic'
> undeclared here (not in a function)
> arch/x86/kernel/apic_64.c:209: error: unknown field 'write_atomic'
> specified in initializer
> arch/x86/kernel/apic_64.c:209: warning: initialization from
> incompatible pointer type
> make[1]: *** [arch/x86/kernel/apic_64.o] Error 1
> make: *** [arch/x86/kernel] Error 2

Just remove the offending initialisers. They are not needed anymore as
the .write method is now used universally including places where
.write_atomic was. I just could not stand watching the thing
proliferate...

Maciej

2008-07-18 22:43:41

by Zachary Amsden

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, 2008-07-18 at 15:02 -0700, Maciej W. Rozycki wrote:
> On Fri, 18 Jul 2008, Yinghai Lu wrote:
>
> > CC arch/x86/kernel/apic_64.o
> > arch/x86/kernel/apic_64.c:170: error: unknown field 'write_atomic'
> > specified in initializer
> > arch/x86/kernel/apic_64.c:170: error: 'native_apic_mem_write_atomic'
> > undeclared here (not in a function)
> > arch/x86/kernel/apic_64.c:209: error: unknown field 'write_atomic'
> > specified in initializer
> > arch/x86/kernel/apic_64.c:209: warning: initialization from
> > incompatible pointer type
> > make[1]: *** [arch/x86/kernel/apic_64.o] Error 1
> > make: *** [arch/x86/kernel] Error 2
>
> Just remove the offending initialisers. They are not needed anymore as
> the .write method is now used universally including places where
> .write_atomic was. I just could not stand watching the thing
> proliferate...

Thank you for cleaning this up Yinghai, but this really begs the
question - is having write_atomic REALLY worthwhile?

Intel themselves have not supported the affected processors since
October 17, 2000, and here we are writing code for it, when the last
processor with such a flaw (C2 or cB1 stepping Pentium) was stamped in
1995, over 13 years ago. It might have been relevant when Linus was
developing SMP support in '96, but it isn't now.

In fact, anyone running a 120 Mhz or 133 Mhz Pentium processor that has
been obsoleted for 8 years should at this point be shot for wasting
energy with their poor megawatt/megaflop ratio, doubly so if they can
claim to be doing so in some kind of SMP scenario which even requires
using an APIC at all.

Seriously, please consider removing all of the flaming crap surrounding
apic_write_around entirely and forcing users who compile for less than
686 processors to just not have official support for local APIC at all,
or just default it off and let people turn it on at own risk.

Zach

2008-07-18 22:49:32

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, Jul 18, 2008 at 03:42:17PM -0700, Zachary Amsden wrote:
> On Fri, 2008-07-18 at 15:02 -0700, Maciej W. Rozycki wrote:
> > On Fri, 18 Jul 2008, Yinghai Lu wrote:
> >
> > > CC arch/x86/kernel/apic_64.o
> > > arch/x86/kernel/apic_64.c:170: error: unknown field 'write_atomic'
> > > specified in initializer
> > > arch/x86/kernel/apic_64.c:170: error: 'native_apic_mem_write_atomic'
> > > undeclared here (not in a function)
> > > arch/x86/kernel/apic_64.c:209: error: unknown field 'write_atomic'
> > > specified in initializer
> > > arch/x86/kernel/apic_64.c:209: warning: initialization from
> > > incompatible pointer type
> > > make[1]: *** [arch/x86/kernel/apic_64.o] Error 1
> > > make: *** [arch/x86/kernel] Error 2
> >
> > Just remove the offending initialisers. They are not needed anymore as
> > the .write method is now used universally including places where
> > .write_atomic was. I just could not stand watching the thing
> > proliferate...
>
> Thank you for cleaning this up Yinghai, but this really begs the
> question - is having write_atomic REALLY worthwhile?

And this thread is about, Maciej's cleanup patch which removed
write_atomic completely :)

Thanks to Maciej who posted this fix. It was on my todo list for
sometime now!

thanks,
suresh

2008-07-18 22:51:18

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, Jul 18, 2008 at 3:49 PM, Suresh Siddha
<[email protected]> wrote:
> On Fri, Jul 18, 2008 at 03:42:17PM -0700, Zachary Amsden wrote:
>> On Fri, 2008-07-18 at 15:02 -0700, Maciej W. Rozycki wrote:
>> > On Fri, 18 Jul 2008, Yinghai Lu wrote:
>> >
>> > > CC arch/x86/kernel/apic_64.o
>> > > arch/x86/kernel/apic_64.c:170: error: unknown field 'write_atomic'
>> > > specified in initializer
>> > > arch/x86/kernel/apic_64.c:170: error: 'native_apic_mem_write_atomic'
>> > > undeclared here (not in a function)
>> > > arch/x86/kernel/apic_64.c:209: error: unknown field 'write_atomic'
>> > > specified in initializer
>> > > arch/x86/kernel/apic_64.c:209: warning: initialization from
>> > > incompatible pointer type
>> > > make[1]: *** [arch/x86/kernel/apic_64.o] Error 1
>> > > make: *** [arch/x86/kernel] Error 2
>> >
>> > Just remove the offending initialisers. They are not needed anymore as
>> > the .write method is now used universally including places where
>> > .write_atomic was. I just could not stand watching the thing
>> > proliferate...
>>
>> Thank you for cleaning this up Yinghai, but this really begs the
>> question - is having write_atomic REALLY worthwhile?
>
> And this thread is about, Maciej's cleanup patch which removed
> write_atomic completely :)
>
> Thanks to Maciej who posted this fix. It was on my todo list for
> sometime now!

do we rebase tip/x86/x2apic or rework this patch?

YH

2008-07-18 22:58:44

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, Jul 18, 2008 at 02:03:48PM -0700, Ingo Molnar wrote:
>
> * Yinghai Lu <[email protected]> wrote:
>
> > > git merge tip/x86/x2apic
> > CONFLICT (content): Merge conflict in arch/x86/kernel/Makefile
> > CONFLICT (content): Merge conflict in arch/x86/kernel/paravirt.c
> > CONFLICT (content): Merge conflict in arch/x86/kernel/smpboot.c
> > CONFLICT (content): Merge conflict in arch/x86/kernel/vmi_32.c
> > CONFLICT (content): Merge conflict in arch/x86/xen/enlighten.c
> > CONFLICT (content): Merge conflict in include/asm-x86/apic.h
> > CONFLICT (content): Merge conflict in include/asm-x86/paravirt.h
>
> that's due to the changes in tip/x86/apic and in tip/x86/uv.
>
> ok, i've just merged x86/apic into x86/x2apic and x86/uv as well, and
> pushed out the result.
>
> Note: it's a first raw merge and completely untested. It will now merge
> cleanly into tip/master. There are probably a few details missing.

Ingo, thanks for doing this. While I was testing my merge changes, you posted
yours... anyhow we need this piece, which is missing from your merge.

Signed-off-by: Suresh Siddha <[email protected]>
---
diff --git a/arch/x86/kernel/apic_32.c b/arch/x86/kernel/apic_32.c
index dcb897f..8728f54 100644
--- a/arch/x86/kernel/apic_32.c
+++ b/arch/x86/kernel/apic_32.c
@@ -174,8 +174,8 @@ u32 safe_xapic_wait_icr_idle(void)

void xapic_icr_write(u32 low, u32 id)
{
- apic_write_around(APIC_ICR2, SET_APIC_DEST_FIELD(id));
- apic_write_around(APIC_ICR, low);
+ apic_write(APIC_ICR2, SET_APIC_DEST_FIELD(id));
+ apic_write(APIC_ICR, low);
}

u64 xapic_icr_read(void)
@@ -191,7 +191,6 @@ u64 xapic_icr_read(void)
static struct apic_ops xapic_ops = {
.read = native_apic_mem_read,
.write = native_apic_mem_write,
- .write_atomic = native_apic_mem_write_atomic,
.icr_read = xapic_icr_read,
.icr_write = xapic_icr_write,
.wait_icr_idle = xapic_wait_icr_idle,
diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
index 46e6124..a850bc6 100644
--- a/arch/x86/kernel/apic_64.c
+++ b/arch/x86/kernel/apic_64.c
@@ -167,7 +167,6 @@ u64 xapic_icr_read(void)
static struct apic_ops xapic_ops = {
.read = native_apic_mem_read,
.write = native_apic_mem_write,
- .write_atomic = native_apic_mem_write_atomic,
.icr_read = xapic_icr_read,
.icr_write = xapic_icr_write,
.wait_icr_idle = xapic_wait_icr_idle,
@@ -206,7 +205,6 @@ u64 x2apic_icr_read(void)
static struct apic_ops x2apic_ops = {
.read = native_apic_msr_read,
.write = native_apic_msr_write,
- .write_atomic = native_apic_msr_write,
.icr_read = x2apic_icr_read,
.icr_write = x2apic_icr_write,
.wait_icr_idle = x2apic_wait_icr_idle,

2008-07-18 23:00:27

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, Jul 18, 2008 at 03:51:07PM -0700, Yinghai Lu wrote:
> On Fri, Jul 18, 2008 at 3:49 PM, Suresh Siddha
> <[email protected]> wrote:
> > Thanks to Maciej who posted this fix. It was on my todo list for
> > sometime now!
>
> do we rebase tip/x86/x2apic or rework this patch?

Ingo just did the merge. And I posted a missing piece. So is this enough?

I will do more testing of the merge.

thanks,
suresh

2008-07-18 23:04:38

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, Jul 18, 2008 at 4:00 PM, Suresh Siddha
<[email protected]> wrote:
> On Fri, Jul 18, 2008 at 03:51:07PM -0700, Yinghai Lu wrote:
>> On Fri, Jul 18, 2008 at 3:49 PM, Suresh Siddha
>> <[email protected]> wrote:
>> > Thanks to Maciej who posted this fix. It was on my todo list for
>> > sometime now!
>>
>> do we rebase tip/x86/x2apic or rework this patch?
>
> Ingo just did the merge. And I posted a missing piece. So is this enough?

maciej, patch will go first, and x2apic will goes later, so bisect on
x2apic will be broken.

YH

2008-07-18 23:18:38

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, Jul 18, 2008 at 04:04:29PM -0700, Yinghai Lu wrote:
> On Fri, Jul 18, 2008 at 4:00 PM, Suresh Siddha
> <[email protected]> wrote:
> > On Fri, Jul 18, 2008 at 03:51:07PM -0700, Yinghai Lu wrote:
> >> On Fri, Jul 18, 2008 at 3:49 PM, Suresh Siddha
> >> <[email protected]> wrote:
> >> > Thanks to Maciej who posted this fix. It was on my todo list for
> >> > sometime now!
> >>
> >> do we rebase tip/x86/x2apic or rework this patch?
> >
> > Ingo just did the merge. And I posted a missing piece. So is this enough?
>
> maciej, patch will go first, and x2apic will goes later, so bisect on
> x2apic will be broken.

hm, ok. Ingo? should I send the rebased 30 or so patches. or we can move this
patch later.

thanks,
suresh

2008-07-18 23:44:50

by Zachary Amsden

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Fri, 2008-07-18 at 15:49 -0700, Suresh Siddha wrote:

> > Thank you for cleaning this up Yinghai, but this really begs the
> > question - is having write_atomic REALLY worthwhile?
>
> And this thread is about, Maciej's cleanup patch which removed
> write_atomic completely :)
>
> Thanks to Maciej who posted this fix. It was on my todo list for
> sometime now!

Awesome! I just felt the need to use 'flaming crap' in a sentence
today, and broken APICs seemed like an easy target. Glad to know it's
gone.

Zach

2008-07-19 21:44:13

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

* Zachary Amsden <[email protected]> wrote:

> On Fri, 2008-07-18 at 15:49 -0700, Suresh Siddha wrote:
>
> > > Thank you for cleaning this up Yinghai, but this really begs the
> > > question - is having write_atomic REALLY worthwhile?
> >
> > And this thread is about, Maciej's cleanup patch which removed
> > write_atomic completely :)
> >
> > Thanks to Maciej who posted this fix. It was on my todo list for
> > sometime now!
>
> Awesome! I just felt the need to use 'flaming crap' in a sentence
> today, and broken APICs seemed like an easy target. Glad to know it's
> gone.

it's gone, but without breaking or limiting old hardware: it's done by
moving the code into a quirk. And that's the general direction which we
want to take in such cases - move ugly non-standard stuff into
low-maintenance-overhead quirks.

Ingo

2008-07-19 21:48:17

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

* Suresh Siddha <[email protected]> wrote:

> On Fri, Jul 18, 2008 at 02:03:48PM -0700, Ingo Molnar wrote:
> >
> > * Yinghai Lu <[email protected]> wrote:
> >
> > > > git merge tip/x86/x2apic
> > > CONFLICT (content): Merge conflict in arch/x86/kernel/Makefile
> > > CONFLICT (content): Merge conflict in arch/x86/kernel/paravirt.c
> > > CONFLICT (content): Merge conflict in arch/x86/kernel/smpboot.c
> > > CONFLICT (content): Merge conflict in arch/x86/kernel/vmi_32.c
> > > CONFLICT (content): Merge conflict in arch/x86/xen/enlighten.c
> > > CONFLICT (content): Merge conflict in include/asm-x86/apic.h
> > > CONFLICT (content): Merge conflict in include/asm-x86/paravirt.h
> >
> > that's due to the changes in tip/x86/apic and in tip/x86/uv.
> >
> > ok, i've just merged x86/apic into x86/x2apic and x86/uv as well, and
> > pushed out the result.
> >
> > Note: it's a first raw merge and completely untested. It will now merge
> > cleanly into tip/master. There are probably a few details missing.
>
> Ingo, thanks for doing this. While I was testing my merge changes, you
> posted yours... anyhow we need this piece, which is missing from your
> merge.

thanks, applied and pushed out. Could you check whether the latest
tip/x86/x2apic code, when git-merge-ed into tip/master, is sane on your
box(en)?

Ingo

2008-07-19 21:50:26

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

* Suresh Siddha <[email protected]> wrote:

> On Fri, Jul 18, 2008 at 04:04:29PM -0700, Yinghai Lu wrote:
> > On Fri, Jul 18, 2008 at 4:00 PM, Suresh Siddha
> > <[email protected]> wrote:
> > > On Fri, Jul 18, 2008 at 03:51:07PM -0700, Yinghai Lu wrote:
> > >> On Fri, Jul 18, 2008 at 3:49 PM, Suresh Siddha
> > >> <[email protected]> wrote:
> > >> > Thanks to Maciej who posted this fix. It was on my todo list for
> > >> > sometime now!
> > >>
> > >> do we rebase tip/x86/x2apic or rework this patch?
> > >
> > > Ingo just did the merge. And I posted a missing piece. So is this enough?
> >
> > maciej, patch will go first, and x2apic will goes later, so bisect on
> > x2apic will be broken.
>
> hm, ok. Ingo? should I send the rebased 30 or so patches. or we can
> move this patch later.

hm, yes, please send the rebased patches - as this stuff has some risks
so bisectability is important.

Ingo

2008-07-19 21:54:07

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

* Ingo Molnar <[email protected]> wrote:

> > > maciej, patch will go first, and x2apic will goes later, so bisect
> > > on x2apic will be broken.
> >
> > hm, ok. Ingo? should I send the rebased 30 or so patches. or we can
> > move this patch later.
>
> hm, yes, please send the rebased patches - as this stuff has some
> risks so bisectability is important.

hm, actually - the order is correct so no need to rebase any of this.

We've got two topics: tip/x86/apic and tip/x86/x2apic.

both tip/x86/x2apic and tip/x86/apic were OK standalone. I merged them
together in this commit:

commit 453c1404c5273a30d715e5a83372a78cff70b6d9
Merge: a208f37... 35b6805...
Author: Ingo Molnar <[email protected]>
Date: Fri Jul 18 23:00:05 2008 +0200

Merge branch 'x86/apic' into x86/x2apic

Conflicts:

arch/x86/kernel/paravirt.c
arch/x86/kernel/smpboot.c
arch/x86/kernel/vmi_32.c
arch/x86/lguest/boot.c
arch/x86/xen/enlighten.c
include/asm-x86/apic.h
include/asm-x86/paravirt.h

Signed-off-by: Ingo Molnar <[email protected]>

and that didnt work straight away, so i applied your fixups - but that's
the next commit. So there's only a single commit that is not bisectable
and that's OK.

so unless there's some other breakage i missed, this should be fine and
no need to resend/rebase anything.

Ingo

2008-07-20 12:08:51

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

i've merged tip/x86/x2apic into tip/master, and it seems to be working
fine. One UP build fix was needed for Xen, see below.

Ingo

-------------------------->
commit caf43bf7c6a55e89b6df5179df434d67e24aa32e
Author: Ingo Molnar <[email protected]>
Date: Sun Jul 20 14:06:50 2008 +0200

x86, xen: fix apic_ops build on UP

fix:

arch/x86/xen/enlighten.c:615: error: variable ‘xen_basic_apic_ops’ has initializer but incomplete type
arch/x86/xen/enlighten.c:616: error: unknown field ‘read’ specified in initializer
[...]

Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/xen/enlighten.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 008b7b6..e4d1459 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -35,6 +35,7 @@
#include <xen/page.h>

#include <asm/paravirt.h>
+#include <asm/apic.h>
#include <asm/page.h>
#include <asm/xen/hypercall.h>
#include <asm/xen/hypervisor.h>

2008-07-20 15:05:25

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

* Ingo Molnar <[email protected]> wrote:

> i've merged tip/x86/x2apic into tip/master, and it seems to be working
> fine. One UP build fix was needed for Xen, see below.

and a similar one for UP lguest.

Ingo

---------------->
commit 7be42004065ce4df193aeef5befd26805267d0d9
Author: Ingo Molnar <[email protected]>
Date: Sun Jul 20 17:04:57 2008 +0200

x86, lguest: fix apic_ops build on UP

fix:

arch/x86/lguest/boot.c:816: error: variable ‘lguest_basic_apic_ops’ has initializer but incomplete type
arch/x86/lguest/boot.c:817: error: unknown field ‘read’ specified in initializer
[...]

Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/lguest/boot.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 35c4349..756fc48 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -55,6 +55,7 @@
#include <linux/lguest_launcher.h>
#include <linux/virtio_console.h>
#include <linux/pm.h>
+#include <asm/apic.h>
#include <asm/lguest.h>
#include <asm/paravirt.h>
#include <asm/param.h>

2008-07-21 18:00:27

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Sun, Jul 20, 2008 at 05:08:11AM -0700, Ingo Molnar wrote:
>
> i've merged tip/x86/x2apic into tip/master, and it seems to be working
> fine. One UP build fix was needed for Xen, see below.

Thanks Ingo. I tested tip/master and found an issue (patch attached)
for x2apic support. This is not because of the recent merges we had, but
because of something(where we still access memory based interface after
enabling x2apic mode) that slipped through my earlier tests.

Probably it is a good idea to unmap the memory mapped interface, once we switch
to x2apic mode. That will catch the issues much earlier. I will
post another patch for this.

Meanwhile, I will do more testing of tip/master along with addressing
open issues.
---

[patch] x64, apic: use generic apic_write() for ack_APIC_irq()

ack_APIC_irq() is used at too many generic places (and not just during
irq_chip handling!) to use the native_apic_mem_write(). For ex, this will
break x2apic based systems.

Fix ack_APIC_irq() to use the generic apic_write() even for 64-bit.

Signed-off-by: Suresh Siddha <[email protected]>
---

diff --git a/include/asm-x86/apic.h b/include/asm-x86/apic.h
index 300b65e..1df955f 100644
--- a/include/asm-x86/apic.h
+++ b/include/asm-x86/apic.h
@@ -138,11 +138,7 @@ static inline void ack_APIC_irq(void)
*/

/* Docs say use 0 for future compatibility */
-#ifdef CONFIG_X86_32
apic_write(APIC_EOI, 0);
-#else
- native_apic_mem_write(APIC_EOI, 0);
-#endif
}

extern int lapic_get_maxlvt(void);

2008-07-22 13:30:23

by Yong Wang

[permalink] [raw]

Subject: Re: [PATCH] x86: APIC: Remove apic_write_around(); use alternatives

On Mon, Jul 21, 2008 at 10:59:41AM -0700, Suresh Siddha wrote:
> Thanks Ingo. I tested tip/master and found an issue (patch attached)
> for x2apic support. This is not because of the recent merges we had, but
> because of something(where we still access memory based interface after
> enabling x2apic mode) that slipped through my earlier tests.
>
> Probably it is a good idea to unmap the memory mapped interface, once we switch
> to x2apic mode. That will catch the issues much earlier. I will
> post another patch for this.
>
> Meanwhile, I will do more testing of tip/master along with addressing
> open issues.
> ---
>
> [patch] x64, apic: use generic apic_write() for ack_APIC_irq()
>
> ack_APIC_irq() is used at too many generic places (and not just during
> irq_chip handling!) to use the native_apic_mem_write(). For ex, this will
> break x2apic based systems.
>
> Fix ack_APIC_irq() to use the generic apic_write() even for 64-bit.
>
> Signed-off-by: Suresh Siddha <[email protected]>
> ---
>
> diff --git a/include/asm-x86/apic.h b/include/asm-x86/apic.h
> index 300b65e..1df955f 100644
> --- a/include/asm-x86/apic.h
> +++ b/include/asm-x86/apic.h
> @@ -138,11 +138,7 @@ static inline void ack_APIC_irq(void)
> */
>
> /* Docs say use 0 for future compatibility */
> -#ifdef CONFIG_X86_32
> apic_write(APIC_EOI, 0);
> -#else
> - native_apic_mem_write(APIC_EOI, 0);
> -#endif
> }
>

ACK. I tested this patch on my x2apic capable box and it works well. Without this patch, kernel built from tip/x86/x2apic cannot boot in x2apic mode and all APs got stuck in smp_callin->calibrate_delay and failed to boot. After applying this patch, system boots well and all processors got up and running successfully.

2008-07-22 13:49:05