lkml.org 
[lkml]   [2009]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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 = {

\
 
 \ /
  Last update: 2009-12-09 16:53    [W:0.015 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site