linux-kernel - x86/apic: Dead code in setup_local

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <0c2c3380-4e2f-5cbb-41eb-38057f008c5f@citrix.com>
Date:   Fri, 13 Mar 2020 21:32:19 +0000
From:   Andrew Cooper <andrew.cooper3@...rix.com>
To:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: x86/apic: Dead code in setup_local_APIC()

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.)