x86/apic: Dead code in setup_local_APIC()

[Andrew Cooper <andrew.cooper3@citrix.com>]

Hello,

c/s 2640da4ccc "x86/apic: Soft disable APIC before initializing it" had
a (perhaps unintended) consequence for the setup of LVT0.

Later, LVT0's mask bit is sampled to determine whether the BSP should be
configured to accept ExtINT messages.

Because soft reset unconditionally masks the LVT registers, the
following patch could be taken to drop dead code:

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 5f973fed3c9f..b80032d2dfeb 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1723,8 +1723,7 @@ static void setup_local_APIC(void)
        /*
         * TODO: set up through-local-APIC from through-I/O-APIC? --macro
         */
-       value = apic_read(APIC_LVT0) & APIC_LVT_MASKED;
-       if (!cpu && (pic_mode || !value || skip_ioapic_setup)) {
+       if (!cpu && (pic_mode || skip_ioapic_setup)) {
                value = APIC_DM_EXTINT;
                apic_printk(APIC_VERBOSE, "enabled ExtINT on CPU#%d\n",
cpu);
        } else {


However, the comment just out of context above says that ExtINT is
deliberately configured even symmetric-IO mode, in case some interrupts
are using the PIC.  If that is the intended behaviour, then 2640da4ccc
regressed it.

One option would be to sample LVT0.MASK before clearing SPIV.EN, but if
the intention is to allow ExtINT in symmetric-IO mode, then its
configuration shouldn't be based on its previous value.

Thoughts?

~Andrew

(I'm actually debugging why Xen can't find a timer IRQ on this platform,
but its not my system and I'm playing spot-the-difference with Linux
based on some photos of a boot log.  I don't think this difference is
relevant to my bug, but it also doesn't appear to be intentional on the
Linux side either.)