From patchwork Fri Dec 17 02:17:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicholas Piggin X-Patchwork-Id: 1569675 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=l8SNEHul; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=2404:9400:2:0:216:3eff:fee1:b9f1; helo=lists.ozlabs.org; envelope-from=skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=) Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2404:9400:2:0:216:3eff:fee1:b9f1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JFXhB4MfQz9t9b for ; Fri, 17 Dec 2021 13:18:02 +1100 (AEDT) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4JFXhB3Gkmz3c9K for ; Fri, 17 Dec 2021 13:18:02 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=l8SNEHul; dkim-atps=neutral X-Original-To: skiboot@lists.ozlabs.org Delivered-To: skiboot@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gmail.com (client-ip=2607:f8b0:4864:20::1035; helo=mail-pj1-x1035.google.com; envelope-from=npiggin@gmail.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=l8SNEHul; dkim-atps=neutral Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4JFXgn4rPWz30JT for ; Fri, 17 Dec 2021 13:17:41 +1100 (AEDT) Received: by mail-pj1-x1035.google.com with SMTP id fv9-20020a17090b0e8900b001a6a5ab1392so1236528pjb.1 for ; Thu, 16 Dec 2021 18:17:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=EBI49JbRwYu+viDY8O2SR1Dt0myPV9dYG4D3vGsXAA4=; b=l8SNEHulBVlF4OFqazbijY7NukcdfCLuQgjXpDbXFDbIiNIqY4xubew+GqKLfGgfWV t+UzcBXcuWUFabyeg1NJjyKVtPXWYfWzGOmMrk/FOrQetsB06d+kR4COJRuwO7HTq4px ShKTFU1+7hhffW170DLZVGR/qWW5LuGElOc8zIE0USL1G1SZ0MiRRaoNOyOtrdXke/ew X5ZyR/LlTzif5i0jUZs/PJEWzBrxQW33PTZQ35rWkPkg04dmBrfy/Zwni+/pbNowHkp1 5foruRjJVffrCrRtoBa5vVgOvzo3dr8BnbdBxBu3Qt7QkeAB2QZJUe93Slzbt/GF3lbT r7SA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=EBI49JbRwYu+viDY8O2SR1Dt0myPV9dYG4D3vGsXAA4=; b=8MCQpMyoRVY17eCRTr9Ix1tC1H5UZiwmCBMnqlLVyh9Zy5GpSibI26/gdteDE0ERG8 hLE8Kz1cgaqdpygmGnjiTBxAWWdus2GzROYnKH1fRG0uj7KR6qR8DQAHPlieJQ1kgdcS Xn+M9EzS2I/7kRm0WtweJb/ItvPL9q0BvvE8evUnIiMtYG/BGltsW1i1xWcsQZ10UwAU f2t5WAHKSUrQTI62GepK5DvM5MQHltglQucJVs3e4+YlTn98G/rmzRru3lXyrAnNGolF 6qMkfC+Nu5t3onAMoxqEhjPS5U+B1npW03F5NVuey9HOEMe7JL8EmlbyuA8h5JHcVtqj +TWA== X-Gm-Message-State: AOAM531O1wl51n8HUN/WkDdgO7aiEKjif4sNb2x4G0Wxcc3F7Jwh/zDI LzsvWMR3AXvN1IWSd/nP6uscvXkVgqc= X-Google-Smtp-Source: ABdhPJyY5m0SfRDFdpVgM4el8Hg5duBGIHV8ex98HhdVWCjmbC42i2XgoonOwSeBpMUpGl1S2hsAfQ== X-Received: by 2002:a17:90a:cb0d:: with SMTP id z13mr1174020pjt.89.1639707455410; Thu, 16 Dec 2021 18:17:35 -0800 (PST) Received: from bobo.ozlabs.ibm.com (203-219-139-206.static.tpgi.com.au. [203.219.139.206]) by smtp.gmail.com with ESMTPSA id k16sm7982639pfu.183.2021.12.16.18.17.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Dec 2021 18:17:35 -0800 (PST) From: Nicholas Piggin To: skiboot@lists.ozlabs.org Date: Fri, 17 Dec 2021 12:17:19 +1000 Message-Id: <20211217021724.709370-2-npiggin@gmail.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20211217021724.709370-1-npiggin@gmail.com> References: <20211217021724.709370-1-npiggin@gmail.com> MIME-Version: 1.0 Subject: [Skiboot] [PATCH v2 1/6] core/cpu: rewrite idle synchronisation X-BeenThere: skiboot@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list for skiboot development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot" Idle reconfiguration is difficult to follow and verify as correct because it can occur while CPUs are in low-level idle routines. For example pm_enabled can change while CPUs are idle. If nothing else, this can result in "cpu_idle_p9 called pm disabled" messages. This changes the idle reconfiuration to always kick all other CPUs out of idle code first whenever idle settings (pm_enabled, IPIs, sreset, etc.) are to be changed. Signed-off-by: Nicholas Piggin --- core/cpu.c | 256 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 156 insertions(+), 100 deletions(-) diff --git a/core/cpu.c b/core/cpu.c index 6f37358a9..1371cd34e 100644 --- a/core/cpu.c +++ b/core/cpu.c @@ -38,6 +38,7 @@ static struct lock reinit_lock = LOCK_UNLOCKED; static bool radix_supported; static unsigned long hid0_hile; static unsigned long hid0_attn; +static bool reconfigure_idle = false; static bool sreset_enabled; static bool ipi_enabled; static bool pm_enabled; @@ -397,7 +398,7 @@ static unsigned int cpu_idle_p8(enum cpu_wake_cause wake_on) sync(); /* Check for jobs again */ - if (cpu_check_jobs(cpu) || !pm_enabled) + if (cpu_check_jobs(cpu) || reconfigure_idle) goto skip_sleep; /* Setup wakup cause in LPCR: EE (for IPI) */ @@ -412,7 +413,7 @@ static unsigned int cpu_idle_p8(enum cpu_wake_cause wake_on) sync(); /* Check if PM got disabled */ - if (!pm_enabled) + if (reconfigure_idle) goto skip_sleep; /* EE and DEC */ @@ -453,7 +454,7 @@ static unsigned int cpu_idle_p9(enum cpu_wake_cause wake_on) sync(); /* Check for jobs again */ - if (cpu_check_jobs(cpu) || !pm_enabled) + if (cpu_check_jobs(cpu) || reconfigure_idle) goto skip_sleep; /* HV DBELL for IPI */ @@ -466,7 +467,7 @@ static unsigned int cpu_idle_p9(enum cpu_wake_cause wake_on) sync(); /* Check if PM got disabled */ - if (!pm_enabled) + if (reconfigure_idle) goto skip_sleep; /* HV DBELL and DEC */ @@ -540,69 +541,70 @@ static void cpu_idle_pm(enum cpu_wake_cause wake_on) } } -void cpu_idle_job(void) +static struct lock idle_lock = LOCK_UNLOCKED; +static int nr_cpus_idle = 0; + +static void enter_idle(void) { - if (pm_enabled) { - cpu_idle_pm(cpu_wake_on_job); - } else { - struct cpu_thread *cpu = this_cpu(); + for (;;) { + lock(&idle_lock); + if (!reconfigure_idle) { + nr_cpus_idle++; + break; + } + unlock(&idle_lock); + /* Another CPU is reconfiguring idle */ smt_lowest(); - /* Check for jobs again */ - while (!cpu_check_jobs(cpu)) { - if (pm_enabled) - break; + while (reconfigure_idle) barrier(); - } smt_medium(); } + + unlock(&idle_lock); } -void cpu_idle_delay(unsigned long delay) +static void exit_idle(void) { - unsigned long now = mftb(); - unsigned long end = now + delay; - unsigned long min_pm = usecs_to_tb(10); - - if (pm_enabled && delay > min_pm) { -pm: - for (;;) { - if (delay >= 0x7fffffff) - delay = 0x7fffffff; - mtspr(SPR_DEC, delay); + lock(&idle_lock); + assert(nr_cpus_idle > 0); + nr_cpus_idle--; + unlock(&idle_lock); +} - cpu_idle_pm(cpu_wake_on_dec); +static void reconfigure_idle_start(void) +{ + struct cpu_thread *cpu; - now = mftb(); - if (tb_compare(now, end) == TB_AAFTERB) - break; - delay = end - now; - if (!(pm_enabled && delay > min_pm)) - goto no_pm; + /* + * First, make sure we are exclusive in reconfiguring by taking + * reconfigure_idle from false to true. + */ + for (;;) { + lock(&idle_lock); + if (!reconfigure_idle) { + reconfigure_idle = true; + break; } - } else { -no_pm: + unlock(&idle_lock); + + /* Someone else is reconfiguring */ smt_lowest(); - for (;;) { - now = mftb(); - if (tb_compare(now, end) == TB_AAFTERB) - break; - delay = end - now; - if (pm_enabled && delay > min_pm) { - smt_medium(); - goto pm; - } - } + while (reconfigure_idle) + barrier(); smt_medium(); } -} -static void cpu_pm_disable(void) -{ - struct cpu_thread *cpu; - unsigned int timeout; + unlock(&idle_lock); - pm_enabled = false; + /* + * Then kick everyone out of idle. + */ + + /* + * Order earlier store to reconfigure_idle=true vs load from + * cpu->in_sleep and cpu->in_idle. + */ sync(); if (proc_gen == proc_gen_p8) { @@ -617,29 +619,107 @@ static void cpu_pm_disable(void) if (cpu->in_sleep || cpu->in_idle) p9_dbell_send(cpu->pir); } + } - /* This code is racy with cpus entering idle, late ones miss the dbell */ + /* + * Then wait for all other CPUs to leave idle. Now they will see + * reconfigure_idle==true and not re-enter idle. + */ + smt_lowest(); + while (nr_cpus_idle != 0) + barrier(); + smt_medium(); - smt_lowest(); - for_each_available_cpu(cpu) { - timeout = 0x08000000; - while ((cpu->in_sleep || cpu->in_idle) && --timeout) + /* + * Order load of nr_cpus_idle with later loads of data that other + * CPUs might have stored-to before coming out of idle. + */ + lwsync(); +} + +static void reconfigure_idle_end(void) +{ + assert(reconfigure_idle); + lock(&idle_lock); + reconfigure_idle = false; + unlock(&idle_lock); +} + +void cpu_idle_job(void) +{ + struct cpu_thread *cpu = this_cpu(); + + do { + enter_idle(); + + if (pm_enabled) { + cpu_idle_pm(cpu_wake_on_job); + } else { + smt_lowest(); + for (;;) { + if (cpu_check_jobs(cpu)) + break; + if (reconfigure_idle) + break; barrier(); - if (!timeout) { - prlog(PR_DEBUG, "cpu_pm_disable TIMEOUT on cpu 0x%04x to exit idle\n", - cpu->pir); - p9_dbell_send(cpu->pir); - } + } + smt_medium(); } - smt_medium(); - } + + exit_idle(); + + } while (!cpu_check_jobs(cpu)); } -void cpu_set_sreset_enable(bool enabled) +void cpu_idle_delay(unsigned long delay) +{ + unsigned long now = mftb(); + unsigned long end = now + delay; + unsigned long min_pm = usecs_to_tb(10); + + do { + enter_idle(); + + delay = end - now; + + if (pm_enabled && delay > min_pm) { + if (delay >= 0x7fffffff) + delay = 0x7fffffff; + mtspr(SPR_DEC, delay); + + cpu_idle_pm(cpu_wake_on_dec); + } else { + smt_lowest(); + for (;;) { + if (tb_compare(mftb(), end) == TB_AAFTERB) + break; + if (reconfigure_idle) + break; + barrier(); + } + smt_medium(); + } + + exit_idle(); + + now = mftb(); + + } while (tb_compare(now, end) != TB_AAFTERB); +} + +static void recalc_pm_enabled(void) { - if (proc_chip_quirks & QUIRK_AWAN) + if (chip_quirk(QUIRK_AWAN)) return; + if (proc_gen == proc_gen_p8) + pm_enabled = ipi_enabled && sreset_enabled; + else + pm_enabled = ipi_enabled; +} + +void cpu_set_sreset_enable(bool enabled) +{ if (sreset_enabled == enabled) return; @@ -647,28 +727,15 @@ void cpu_set_sreset_enable(bool enabled) /* Public P8 Mambo has broken NAP */ if (chip_quirk(QUIRK_MAMBO_CALLOUTS)) return; + } - sreset_enabled = enabled; - sync(); + reconfigure_idle_start(); - if (!enabled) { - cpu_pm_disable(); - } else { - if (ipi_enabled) - pm_enabled = true; - } + sreset_enabled = enabled; - } else if (proc_gen == proc_gen_p9 || proc_gen == proc_gen_p10) { - sreset_enabled = enabled; - sync(); - /* - * Kick everybody out of PM so they can adjust the PM - * mode they are using (EC=0/1). - */ - cpu_pm_disable(); - if (ipi_enabled) - pm_enabled = true; - } + recalc_pm_enabled(); + + reconfigure_idle_end(); } void cpu_set_ipi_enable(bool enabled) @@ -676,24 +743,13 @@ void cpu_set_ipi_enable(bool enabled) if (ipi_enabled == enabled) return; - if (proc_gen == proc_gen_p8) { - ipi_enabled = enabled; - sync(); - if (!enabled) { - cpu_pm_disable(); - } else { - if (sreset_enabled) - pm_enabled = true; - } + reconfigure_idle_start(); - } else if (proc_gen == proc_gen_p9 || proc_gen == proc_gen_p10) { - ipi_enabled = enabled; - sync(); - if (!enabled) - cpu_pm_disable(); - else if (!chip_quirk(QUIRK_AWAN)) - pm_enabled = true; - } + ipi_enabled = enabled; + + recalc_pm_enabled(); + + reconfigure_idle_end(); } void cpu_process_local_jobs(void)