From nobody Wed Jun 15 15:39:25 2022 X-Original-To: dev-commits-src-main@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 B13B685532E; Wed, 15 Jun 2022 15:39:26 +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 4LNTxp2fhmz4lrh; Wed, 15 Jun 2022 15:39:26 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1655307566; 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=0L1Zji6iA5bSMifIBBhBrAbe8XaPt3yHAV6sxAh4ouw=; b=F1c6z9QZ061OIy985gr9LrWrHV5CY8gISXMWY19FcE47k9X9AtUpnkJSeQKPo3W/54APxC Qi90JfAiv4M4r48O/InjqDOnmJ+47RUFP6Wx2aAqYPK4yZNMV6/c23TpOd5Fdis2Jg8nTc 38ZuBrHuAatJh1HuKF1oed/rjYFCgHMU4w5AtqTNFAPmjh63LQeKtP7fFIwPAaKbKoPJpy MfzoMvWVGM8oh5IuVr+70vttyvOdILhkEslM16TFVRMAKfdk35AqX2wOt3h+D+ZwoOB4cs FtAmBV6BHonPqU7L3joVBvC1pZ1/9Of3DN5j1w+RE3DqKiZsqZu+qqZJMGBtgA== 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 EEDA92151A; Wed, 15 Jun 2022 15:39:25 +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 25FFdP3x085393; Wed, 15 Jun 2022 15:39:25 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 25FFdPtG085392; Wed, 15 Jun 2022 15:39:25 GMT (envelope-from git) Date: Wed, 15 Jun 2022 15:39:25 GMT Message-Id: <202206151539.25FFdPtG085392@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: f6b799a86b8f - main - Fix the test used to wait for AP startup on x86, arm64, riscv List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@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/main X-Git-Reftype: branch X-Git-Commit: f6b799a86b8fb69013c10b072baed4ee6c8db2f9 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1655307566; 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=0L1Zji6iA5bSMifIBBhBrAbe8XaPt3yHAV6sxAh4ouw=; b=rOzK3CaqxCuxgWkaW30S0YROZL91rOYVEHd49g4gMUAz7kS9DgkEWe3F81mD2jyxlPXcAD 7KyyPJwXSae/wswQ15J/DGmLaZNu9su6FP0QzbS9HnXNTJZJeT/pHaSzKrMSwlQMbU6gnZ HvZsH90RQ1zrBhRNK8d+Tf6tAcoUtAvzA/JHMF3R4IlkYFEvmima+YNDfWQoKLbKtCBxsZ vgsTuEqZbMmM2nuV4E7rs56Bgv4Os1280shSF9/BBCFDifqkq/In53t17uxoD9hmjolWFJ cges0FHiq1AhcukxT4yLZHHN6+G7fI/2WUqWwxTBoWfQDc5jOGWxGs5ipiqzDg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1655307566; a=rsa-sha256; cv=none; b=spJ5LQywigcc6/uvCHb+rmTcFs1n8+gINdd3U9f1ZCN9bUSEWOd1enZ57CmaBAnd8YZEEh FkbwPXLA3rCaxn+KPMeekwBwYmCD2g0MQq4rfd1Mk4TYTHmAQ95tHcyT77+3GLb86RAZjC wNO3oSCQah7UrvTitLMBUCZ6fbWpu3Bm6pk1nOq8EAvfEiJFr5ax2CyPiOr7YspIfip2Vq MS2EXwzvE2YP5uUbprUl7ntLamvDuYrptMfUeSZex3X9Q9XOAeeuZj5SNgCaKfIm2eReut CjAnrWz3hnYVIowSxi92wKoEyTy1iybTHLkYUbrhYeVRTp4EXRcAPF0KVy4syg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=f6b799a86b8fb69013c10b072baed4ee6c8db2f9 commit f6b799a86b8fb69013c10b072baed4ee6c8db2f9 Author: Mark Johnston AuthorDate: 2022-06-15 14:29:39 +0000 Commit: Mark Johnston CommitDate: 2022-06-15 15:38:04 +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.") MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D35435 --- 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 74281f9a88e5..adf5592832c5 100644 --- a/sys/arm64/arm64/mp_machdep.c +++ b/sys/arm64/arm64/mp_machdep.c @@ -183,7 +183,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; } @@ -290,11 +290,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_ap_entry(); @@ -305,16 +300,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 c460eb033799..f0048284a0c7 100644 --- a/sys/riscv/riscv/mp_machdep.c +++ b/sys/riscv/riscv/mp_machdep.c @@ -210,7 +210,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); } @@ -284,11 +284,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_ap_entry(); @@ -299,16 +294,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 3ca11700f2f2..6dab7987e208 100644 --- a/sys/x86/x86/mp_x86.c +++ b/sys/x86/x86/mp_x86.c @@ -1129,11 +1129,6 @@ init_secondary_tail(void) kcsan_cpu_init(cpuid); - /* - * Assert that smp_after_idle_runnable condition is reasonable. - */ - MPASS(PCPU_GET(curpcb) == NULL); - sched_ap_entry(); panic("scheduler returned us to %s", __func__); @@ -1143,13 +1138,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); }