From nobody Wed Jun 29 14:40:56 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 46FCC86D7DF; Wed, 29 Jun 2022 14:40:57 +0000 (UTC) (envelope-from git@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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4LY3zr69dXz4hp2; Wed, 29 Jun 2022 14:40:56 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1656513656; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=JpjTo0uyQf0Yu+HJiQ+kZx6QPOKfSu+3IRf8ym55Hjg=; b=IMFTH+cM8DsS/EUPI84pqCsvEy+/hHf1bWzeqluz+BdLexmzSRc8apY4sFOHQOB2P35QD0 FIjWSCciPNbuQmBz14wkpe1p5DYXr3MlrGHzyzmHMwCe0bRMmIVQ6IIm5DuFV7L/FmzGXz dz9d+EVAYyD/zGIa4SgYcGBzc3+IXS8ka+giB8nAhVClMvMgwbii5Uyer3dxa0APHmTpss 3j5x7MWSuLEImsKAa8O4BAA/h2I1jO77Zw6igLjURqq/mXTKmMafAnnFMO4ZbxWrsfI8s9 RvvA2507CxXoGDn+4+JIDm/FozfPiQ5iDkUMxPheg/c5HZLR4Du46jUeC2JoDg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 8C5511399D; Wed, 29 Jun 2022 14:40:56 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 25TEeuOq031741; Wed, 29 Jun 2022 14:40:56 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 25TEeuFn031740; Wed, 29 Jun 2022 14:40:56 GMT (envelope-from git) Date: Wed, 29 Jun 2022 14:40:56 GMT Message-Id: <202206291440.25TEeuFn031740@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: c31c881f7eee - stable/13 - Fix the test used to wait for AP startup on x86, arm64, riscv List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: c31c881f7eee79ab7c5f820952cbf22af88da0db Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1656513656; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=JpjTo0uyQf0Yu+HJiQ+kZx6QPOKfSu+3IRf8ym55Hjg=; b=puI8JrwEisG8rZjWNuoF7NiM0g8ieAKyOtYibPwmgqOTFVyZUAh4VEGilr4EQZVgm5f43D 3i09El2I7ABSc7rzjvA6FWBPDpw9r+CxF2XzPc30zfSWFCt9DuTR/lsI4sfAkTF978uRKY zlUp3L5mHHF/plDbRtCOJTOgfsvp5Z+o6ne8lF++MnDPFM2sADorcHUR/SErSLIw4Z+BuZ C+ykbCFmR/tcCbUQ6kbcRwJ9AQ0x7jS13BztFdE8uGKj7jf+BLnv2y7FgtqGXX6Y2ozw15 o4RB0KIR6cTRSI4a5+Obji07t6DMUXTd0VbiuhPUyoLZQUEG7su0MUhlCnpA/g== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1656513656; a=rsa-sha256; cv=none; b=ulZHSe5Q83meODxX6qxSr38Iene1wNyCJYpPrhE4/P7kcjq7LOoDCiLMWVKUYy1jBzki8E /ltI2p6ASyO/qgToakcR9OSSN23ZTEiIQooBouFR1TNI5Cr5ystY6x8PBXIF8fUQrbMD70 SvoGE9WpEnr9t6QBE9j8e10ZShY41rZ6cQ6+DQFQgKFHwaM+ZA55hJA4LduwHCpv3vNiZJ /JGXimwSdj6kvDMrdh2LHuqUAC3wnPi1KOnVALIUrhI61A/ulpmFvw9/XZ65IEH16n1Df/ ea0dyPRz9+RxB3l6LcTgxXzGvST8+XLnWfruCV8FGt+ybJ0zo87vdjU5l2H3YA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=c31c881f7eee79ab7c5f820952cbf22af88da0db commit c31c881f7eee79ab7c5f820952cbf22af88da0db Author: Mark Johnston AuthorDate: 2022-06-15 14:29:39 +0000 Commit: Mark Johnston CommitDate: 2022-06-29 14:13:44 +0000 Fix the test used to wait for AP startup on x86, arm64, riscv On arm64, testing pc_curpcb != NULL is not correct since pc_curpcb is set in pmap_switch() while the bootstrap stack is still in use. As a result, smp_after_idle_runnable() can free the boot stack prematurely. Take a different approach: use smp_rendezvous() to wait for all APs to acknowledge an interrupt. Since APs must not enable interrupts until they've entered the scheduler, i.e., switched off the boot stack, this provides the right guarantee without depending as much on the implementation of cpu_throw(). And, this approach applies to all platforms, so convert x86 and riscv as well. Reported by: mmel Tested by: mmel Reviewed by: kib Fixes: 8db2e8fd16c4 ("Remove the secondary_stacks array in arm64 and riscv kernels.") Sponsored by: The FreeBSD Foundation (cherry picked from commit f6b799a86b8fb69013c10b072baed4ee6c8db2f9) --- sys/arm64/arm64/mp_machdep.c | 27 +++++++++++++++------------ sys/riscv/riscv/mp_machdep.c | 27 +++++++++++++++------------ sys/x86/x86/mp_x86.c | 22 +++++++++++++--------- 3 files changed, 43 insertions(+), 33 deletions(-) diff --git a/sys/arm64/arm64/mp_machdep.c b/sys/arm64/arm64/mp_machdep.c index b42f65b9e399..3e47c60088a8 100644 --- a/sys/arm64/arm64/mp_machdep.c +++ b/sys/arm64/arm64/mp_machdep.c @@ -182,7 +182,7 @@ release_aps(void *dummy __unused) started = 0; for (i = 0; i < 2000; i++) { - if (smp_started) { + if (atomic_load_acq_int(&smp_started) != 0) { printf("done\n"); return; } @@ -287,11 +287,6 @@ init_secondary(uint64_t cpu) kcsan_cpu_init(cpu); - /* - * Assert that smp_after_idle_runnable condition is reasonable. - */ - MPASS(PCPU_GET(curpcb) == NULL); - /* Enter the scheduler */ sched_throw(NULL); @@ -302,16 +297,24 @@ init_secondary(uint64_t cpu) static void smp_after_idle_runnable(void *arg __unused) { - struct pcpu *pc; int cpu; + if (mp_ncpus == 1) + return; + + KASSERT(smp_started != 0, ("%s: SMP not started yet", __func__)); + + /* + * Wait for all APs to handle an interrupt. After that, we know that + * the APs have entered the scheduler at least once, so the boot stacks + * are safe to free. + */ + smp_rendezvous(smp_no_rendezvous_barrier, NULL, + smp_no_rendezvous_barrier, NULL); + for (cpu = 1; cpu < mp_ncpus; cpu++) { - if (bootstacks[cpu] != NULL) { - pc = pcpu_find(cpu); - while (atomic_load_ptr(&pc->pc_curpcb) == NULL) - cpu_spinwait(); + if (bootstacks[cpu] != NULL) kmem_free((vm_offset_t)bootstacks[cpu], PAGE_SIZE); - } } } SYSINIT(smp_after_idle_runnable, SI_SUB_SMP, SI_ORDER_ANY, diff --git a/sys/riscv/riscv/mp_machdep.c b/sys/riscv/riscv/mp_machdep.c index 6a66b7eb80f7..e038b65d9aba 100644 --- a/sys/riscv/riscv/mp_machdep.c +++ b/sys/riscv/riscv/mp_machdep.c @@ -211,7 +211,7 @@ release_aps(void *dummy __unused) sbi_send_ipi(mask.__bits); for (i = 0; i < 2000; i++) { - if (smp_started) + if (atomic_load_acq_int(&smp_started)) return; DELAY(1000); } @@ -285,11 +285,6 @@ init_secondary(uint64_t hart) mtx_unlock_spin(&ap_boot_mtx); - /* - * Assert that smp_after_idle_runnable condition is reasonable. - */ - MPASS(PCPU_GET(curpcb) == NULL); - /* Enter the scheduler */ sched_throw(NULL); @@ -300,16 +295,24 @@ init_secondary(uint64_t hart) static void smp_after_idle_runnable(void *arg __unused) { - struct pcpu *pc; int cpu; + if (mp_ncpus == 1) + return; + + KASSERT(smp_started != 0, ("%s: SMP not started yet", __func__)); + + /* + * Wait for all APs to handle an interrupt. After that, we know that + * the APs have entered the scheduler at least once, so the boot stacks + * are safe to free. + */ + smp_rendezvous(smp_no_rendezvous_barrier, NULL, + smp_no_rendezvous_barrier, NULL); + for (cpu = 1; cpu <= mp_maxid; cpu++) { - if (bootstacks[cpu] != NULL) { - pc = pcpu_find(cpu); - while (atomic_load_ptr(&pc->pc_curpcb) == NULL) - cpu_spinwait(); + if (bootstacks[cpu] != NULL) kmem_free((vm_offset_t)bootstacks[cpu], PAGE_SIZE); - } } } SYSINIT(smp_after_idle_runnable, SI_SUB_SMP, SI_ORDER_ANY, diff --git a/sys/x86/x86/mp_x86.c b/sys/x86/x86/mp_x86.c index 4854331af17c..289885fa6213 100644 --- a/sys/x86/x86/mp_x86.c +++ b/sys/x86/x86/mp_x86.c @@ -1103,11 +1103,6 @@ init_secondary_tail(void) kcsan_cpu_init(cpuid); - /* - * Assert that smp_after_idle_runnable condition is reasonable. - */ - MPASS(PCPU_GET(curpcb) == NULL); - sched_throw(NULL); panic("scheduler returned us to %s", __func__); @@ -1117,13 +1112,22 @@ init_secondary_tail(void) static void smp_after_idle_runnable(void *arg __unused) { - struct pcpu *pc; int cpu; + if (mp_ncpus == 1) + return; + + KASSERT(smp_started != 0, ("%s: SMP not started yet", __func__)); + + /* + * Wait for all APs to handle an interrupt. After that, we know that + * the APs have entered the scheduler at least once, so the boot stacks + * are safe to free. + */ + smp_rendezvous(smp_no_rendezvous_barrier, NULL, + smp_no_rendezvous_barrier, NULL); + for (cpu = 1; cpu < mp_ncpus; cpu++) { - pc = pcpu_find(cpu); - while (atomic_load_ptr(&pc->pc_curpcb) == NULL) - cpu_spinwait(); kmem_free((vm_offset_t)bootstacks[cpu], kstack_pages * PAGE_SIZE); }