From owner-svn-src-head@freebsd.org Fri Jul 24 19:54:16 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 3FD9737228C; Fri, 24 Jul 2020 19:54:16 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BD0Jm0xJ8z4Bv8; Fri, 24 Jul 2020 19:54:16 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 042F7FC38; Fri, 24 Jul 2020 19:54:16 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 06OJsFW9081113; Fri, 24 Jul 2020 19:54:15 GMT (envelope-from mav@FreeBSD.org) Received: (from mav@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 06OJsFvh081112; Fri, 24 Jul 2020 19:54:15 GMT (envelope-from mav@FreeBSD.org) Message-Id: <202007241954.06OJsFvh081112@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mav set sender to mav@FreeBSD.org using -f From: Alexander Motin Date: Fri, 24 Jul 2020 19:54:15 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r363490 - head/sys/x86/x86 X-SVN-Group: head X-SVN-Commit-Author: mav X-SVN-Commit-Paths: head/sys/x86/x86 X-SVN-Commit-Revision: 363490 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Jul 2020 19:54:16 -0000 Author: mav Date: Fri Jul 24 19:54:15 2020 New Revision: 363490 URL: https://svnweb.freebsd.org/changeset/base/363490 Log: Make lapic_ipi_vectored(APIC_IPI_DEST_SELF) NMI safe. Sending IPI to self or all CPUs does not require write into upper part of the ICR, prone to races. Previously the code disabled interrupts, but it was not enough for NMIs. Instead of that when possible write only lower part of the register, or use special SELF IPI register in x2APIC mode. This also removes ICR reads used to preserve reserved bits on write. It was there from the beginning, but I failed to find explanation why, neither I see Linux doing it. Specification even tells that ICR content may be lost in deep C-states, so if hardware does not bother to preserve it, why should we? MFC after: 2 weeks Sponsored by: iXsystems, Inc. Modified: head/sys/x86/x86/local_apic.c Modified: head/sys/x86/x86/local_apic.c ============================================================================== --- head/sys/x86/x86/local_apic.c Fri Jul 24 19:52:52 2020 (r363489) +++ head/sys/x86/x86/local_apic.c Fri Jul 24 19:54:15 2020 (r363490) @@ -254,22 +254,6 @@ lapic_write32_nofence(enum LAPIC_REGISTERS reg, uint32 #ifdef SMP static uint64_t -lapic_read_icr(void) -{ - uint64_t v; - uint32_t vhi, vlo; - - if (x2apic_mode) { - v = rdmsr(MSR_APIC_000 + LAPIC_ICR_LO); - } else { - vhi = lapic_read32(LAPIC_ICR_HI); - vlo = lapic_read32(LAPIC_ICR_LO); - v = ((uint64_t)vhi << 32) | vlo; - } - return (v); -} - -static uint64_t lapic_read_icr_lo(void) { @@ -279,6 +263,7 @@ lapic_read_icr_lo(void) static void lapic_write_icr(uint32_t vhi, uint32_t vlo) { + register_t saveintr; uint64_t v; if (x2apic_mode) { @@ -286,10 +271,32 @@ lapic_write_icr(uint32_t vhi, uint32_t vlo) mfence(); wrmsr(MSR_APIC_000 + LAPIC_ICR_LO, v); } else { + saveintr = intr_disable(); lapic_write32(LAPIC_ICR_HI, vhi); lapic_write32(LAPIC_ICR_LO, vlo); + intr_restore(saveintr); } } + +static void +lapic_write_icr_lo(uint32_t vlo) +{ + + if (x2apic_mode) { + mfence(); + wrmsr(MSR_APIC_000 + LAPIC_ICR_LO, vlo); + } else { + lapic_write32(LAPIC_ICR_LO, vlo); + } +} + +static void +lapic_write_self_ipi(uint32_t vector) +{ + + KASSERT(x2apic_mode, ("SELF IPI write in xAPIC mode")); + wrmsr(MSR_APIC_000 + LAPIC_SELF_IPI, vector); +} #endif /* SMP */ static void @@ -1991,9 +1998,7 @@ native_lapic_ipi_wait(int delay) static void native_lapic_ipi_raw(register_t icrlo, u_int dest) { - uint64_t icr; - uint32_t vhi, vlo; - register_t saveintr; + uint32_t icrhi; /* XXX: Need more sanity checking of icrlo? */ KASSERT(x2apic_mode || lapic_map != NULL, @@ -2004,35 +2009,15 @@ native_lapic_ipi_raw(register_t icrlo, u_int dest) KASSERT((icrlo & APIC_ICRLO_RESV_MASK) == 0, ("%s: reserved bits set in ICR LO register", __func__)); - /* Set destination in ICR HI register if it is being used. */ - if (!x2apic_mode) { - saveintr = intr_disable(); - icr = lapic_read_icr(); - } - if ((icrlo & APIC_DEST_MASK) == APIC_DEST_DESTFLD) { - if (x2apic_mode) { - vhi = dest; - } else { - vhi = icr >> 32; - vhi &= ~APIC_ID_MASK; - vhi |= dest << APIC_ID_SHIFT; - } + if (x2apic_mode) + icrhi = dest; + else + icrhi = dest << APIC_ID_SHIFT; + lapic_write_icr(icrhi, icrlo); } else { - vhi = 0; + lapic_write_icr_lo(icrlo); } - - /* Program the contents of the IPI and dispatch it. */ - if (x2apic_mode) { - vlo = icrlo; - } else { - vlo = icr; - vlo &= APIC_ICRLO_RESV_MASK; - vlo |= icrlo; - } - lapic_write_icr(vhi, vlo); - if (!x2apic_mode) - intr_restore(saveintr); } #define BEFORE_SPIN 50000 @@ -2048,33 +2033,38 @@ native_lapic_ipi_vectored(u_int vector, int dest) KASSERT((vector & ~APIC_VECTOR_MASK) == 0, ("%s: invalid vector %d", __func__, vector)); - icrlo = APIC_DESTMODE_PHY | APIC_TRIGMOD_EDGE | APIC_LEVEL_ASSERT; - - /* - * NMI IPIs are just fake vectors used to send a NMI. Use special rules - * regarding NMIs if passed, otherwise specify the vector. - */ - if (vector >= IPI_NMI_FIRST) - icrlo |= APIC_DELMODE_NMI; - else - icrlo |= vector | APIC_DELMODE_FIXED; destfield = 0; switch (dest) { case APIC_IPI_DEST_SELF: - icrlo |= APIC_DEST_SELF; + if (x2apic_mode && vector < IPI_NMI_FIRST) { + lapic_write_self_ipi(vector); + return; + } + icrlo = APIC_DEST_SELF; break; case APIC_IPI_DEST_ALL: - icrlo |= APIC_DEST_ALLISELF; + icrlo = APIC_DEST_ALLISELF; break; case APIC_IPI_DEST_OTHERS: - icrlo |= APIC_DEST_ALLESELF; + icrlo = APIC_DEST_ALLESELF; break; default: + icrlo = 0; KASSERT(x2apic_mode || (dest & ~(APIC_ID_MASK >> APIC_ID_SHIFT)) == 0, ("%s: invalid destination 0x%x", __func__, dest)); destfield = dest; } + + /* + * NMI IPIs are just fake vectors used to send a NMI. Use special rules + * regarding NMIs if passed, otherwise specify the vector. + */ + if (vector >= IPI_NMI_FIRST) + icrlo |= APIC_DELMODE_NMI; + else + icrlo |= vector | APIC_DELMODE_FIXED; + icrlo |= APIC_DESTMODE_PHY | APIC_TRIGMOD_EDGE | APIC_LEVEL_ASSERT; /* Wait for an earlier IPI to finish. */ if (!lapic_ipi_wait(BEFORE_SPIN)) {