Possible logic error in 'ioapic0: misconfigured as apic' fix?

[Andrew Daugherity]

I was looking at post-6.0 commits to sys/arch/i386/i386 to see if there was
anything obvious that would explain why 6.1 won't boot on a 486 while 6.0
does.
Nothing jumped out, but I did a double-take at this unrelated commit:

====
Stop printing the "ioapic0: misconfigured as apic..." messages.  Basically
all modern machines are "misconfigured".  We simply have to configure them
and it is pointless to complain.
[sys/arch/i386/i386/ioapic.c   r1.38 and
 sys/arch/amd64/amd64/ioapic.c r1.24]
====

The code doesn't seem to do what the commit log says -- the change was:
@@ -353,7 +353,7 @@
  * In case the APIC is not initialized to the correct ID
  * do it now.
  */
- if (apic_id != sc->sc_apicid) {
+ if (mp_verbose && apic_id != sc->sc_apicid) {
  printf("%s: misconfigured as apic %d",
     sc->sc_pic.pic_name, apic_id);
  ioapic_set_id(sc);


This means that without mp_verbose, ioapic_set_id(sc) doesn't get called.
If
we "simply have to configure them" and don't want to complain normally, I
think
this would be more appropriate:

        if (apic_id != sc->sc_apicid) {
                if (mp_verbose)
                        printf("%s: misconfigured as apic %d",
                            sc->sc_pic.pic_name, apic_id);
                ioapic_set_id(sc);
        }

Perhaps I'm missing something and io_apic_set_id() doesn't always need to be
called?  Also, the comments before this block are different on amd64 and
i386,
but I don't know which one is preferred.

Anyway, here's a diff:

diff --git a/sys/arch/amd64/amd64/ioapic.c b/sys/arch/amd64/amd64/ioapic.c
index e8f5e6aa7..427d0b5aa 100644
--- a/sys/arch/amd64/amd64/ioapic.c
+++ b/sys/arch/amd64/amd64/ioapic.c
@@ -356,9 +356,10 @@ ioapic_attach(struct device *parent, struct device
*self, void *aux)
         * Maybe we should record the original ID for interrupt
         * mapping later ...
         */
-       if (mp_verbose && apic_id != sc->sc_apicid) {
-               printf("%s: misconfigured as apic %d",
-                   sc->sc_pic.pic_name, apic_id);
+       if (apic_id != sc->sc_apicid) {
+               if (mp_verbose)
+                       printf("%s: misconfigured as apic %d",
+                           sc->sc_pic.pic_name, apic_id);
                ioapic_set_id(sc);
        }
 #if 0
diff --git a/sys/arch/i386/i386/ioapic.c b/sys/arch/i386/i386/ioapic.c
index f6de0573e..7ec54f9cd 100644
--- a/sys/arch/i386/i386/ioapic.c
+++ b/sys/arch/i386/i386/ioapic.c
@@ -352,9 +352,10 @@ ioapic_attach(struct device *parent, struct device
*self, void *aux)
         * In case the APIC is not initialized to the correct ID
         * do it now.
         */
-       if (mp_verbose && apic_id != sc->sc_apicid) {
-               printf("%s: misconfigured as apic %d",
-                   sc->sc_pic.pic_name, apic_id);
+       if (apic_id != sc->sc_apicid) {
+               if (mp_verbose)
+                       printf("%s: misconfigured as apic %d",
+                           sc->sc_pic.pic_name, apic_id);
                ioapic_set_id(sc);
        }
 #if 0