[patch 2/2] x86,apic: Use logical OR in noop-write operation

[Cyrill Gorcunov]

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.

In turn noop'ified read operation remains using logical
AND statement. This will catch _only_ future code bugs
since:

1) The old 32bit systems without apic presence bit use
   disable_apic only as a flag that chip was turned off
   via boot option but not due to MP-table (BIOS) bug where
   we may try to re-enable apic via MSR registers. The further
   SMP setup code already prepared for a such situation
   (ie it disables SMP support) and do not issue write
   operations but still needs to issue apic_read. So instead
   of deforming code with "if" we just allow reads until
   SMP support gets disabled. But even then we still need
   apic_read issued on a such machine at shutdown procedure.

2) x86-64 at moment properly uses cpu_has_apic and turn
   this feature off as only chip is disabled together
   with disable_apic flag.

CC: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Signed-off-by: Cyrill Gorcunov <gorcunov@xxxxxxxxxx>
---

Please review, I hope this explanation is more or less good
though I would be glad to hear complains a well :)

 arch/x86/kernel/apic/apic_noop.c |   30 ++++++++++++++++++++++++++++--
 1 file changed, 28 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,41 @@ int noop_apicid_to_node(int logical_apic
 	return 0;
 }
 
+/*
+ * we use logical AND here just to suppress a
+ * number of WARNs which would take place all
+ * over the code if the kernel is SMP compatible
+ * and we would need to deform code with a lot
+ * of "if" statements then (especially on x86-32
+ * with discrete apic chip)
+ *
+ * note that even if apic was not disabled via
+ * boot option we still may have apic broken
+ * due to buggy BIOS/MP-table/ACPI so we allow
+ * any read operation on a such systems 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;
 }
 
+/*
+ * we don't use logical AND here because otherwise
+ * any writes on 486DXSL (which has no apic presence
+ * bit retrieved via cpuid) will never trigger
+ * this WARN allowing a (possible buggy) code to
+ * continue executing, so consider this WARN as
+ * a strict guard against unpredicted/inaccurate
+ * apic code modifications or/and buggy callers
+ *
+ * in short -- warn every write on enabled apic
+ * (both 82489DX and integrated LAPICs)
+ */
 static void noop_apic_write(u32 reg, u32 v)
 {
-	WARN_ON_ONCE(cpu_has_apic && !disable_apic);
+	WARN_ON_ONCE(cpu_has_apic || !disable_apic);
 }
 
 struct apic apic_noop = {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/