Messages in this thread Patch in this message | | | Date | Wed, 9 Dec 2009 18:50:02 +0300 | From | Cyrill Gorcunov <> | Subject | Re: [patch 2/2] x86,apic: Use logical OR in noop-write operation |
| |
On Tue, Dec 08, 2009 at 11:48:38PM +0300, Cyrill Gorcunov wrote: > On Tue, Dec 08, 2009 at 06:53:18PM +0300, Cyrill Gorcunov wrote: > > For apic noop'ified we have to use logical OR statement, > > otherwise any write on systems shipped with 82489DX > > (where apic presence bit can't be retrieved via cpuid) > > will not trigger the warning which is not desired. > > > ... > > Ingo please dont apply this patch. Perhaps we may warn unconditionally > (as Peter marked) which would be more clear approach indeed. Need more > time to check all code flows. > > Sorry for inconvenience. > > -- Cyrill
Here is what done at moment. I've grepped x86 arch for apic_write and except thermal monitoring (the patch is already sent) all other callers do check if apic is active (either via cpu_has_apic or disable_apic). So I think we may safely use unconditional warning if apic_write with disabled apic is called.
Please take a look -- I would be glad to hear any comments/complains.
There is unclear moment for me with "SGI Visual Workstation" which has ack_cobalt_irq and calls for apic_write which could trigger this warning.
-- Cyrill --- x86,apic: Warn on noop_apic_write unconditionally
In apic noop'ified we should never call for write operation. Otherwise it's a caller bug (we should be WARNed about).
Also add a comment on noop_apic_read WARN conditions.
CC: H. Peter Anvin <hpa@zytor.com> CC: Thomas Gleixner <tglx@linutronix.de> Cc: Yinghai Lu <yinghai@kernel.org> Cc: Maciej W. Rozycki <macro@linux-mips.org> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org> ---
arch/x86/kernel/apic/apic_noop.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
Index: linux-2.6.git/arch/x86/kernel/apic/apic_noop.c ===================================================================== --- linux-2.6.git.orig/arch/x86/kernel/apic/apic_noop.c +++ linux-2.6.git/arch/x86/kernel/apic/apic_noop.c @@ -119,15 +119,28 @@ int noop_apicid_to_node(int logical_apic return 0; } +/* + * note that we allow this routine to be called + * under the following conditions: + * - apic was explicitly disabled via boot option + * - on old 486 machines (which has no apic presence bit + * retrieved via cpuid) + * this is done only in a sake of callers code simplicity + */ static u32 noop_apic_read(u32 reg) { - WARN_ON_ONCE((cpu_has_apic && !disable_apic)); + WARN_ON_ONCE(cpu_has_apic && !disable_apic); return 0; } static void noop_apic_write(u32 reg, u32 v) { - WARN_ON_ONCE(cpu_has_apic && !disable_apic); + /* + * If someone is trying to write apic + * register when it is NOOP'ified + * this is a bug on caller side + */ + WARN_ON_ONCE(1); } struct apic apic_noop = {
| |