Message ID | 20231026170330.4657-2-mario.limonciello@amd.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:d641:0:b0:403:3b70:6f57 with SMTP id cy1csp26677vqb; Thu, 26 Oct 2023 10:04:17 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGRRUIqPMD4D8iFq1vFQ1eYTxRvm2OdKd38cIEuAUzGQcHyu3vKiF+jVP9+LDUGwOsdJDsB X-Received: by 2002:a81:4143:0:b0:5a7:d86c:988 with SMTP id f3-20020a814143000000b005a7d86c0988mr5540ywk.28.1698339857692; Thu, 26 Oct 2023 10:04:17 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1698339857; cv=pass; d=google.com; s=arc-20160816; b=ZsaS7CUmEfj8F3j8z4XRxL7TAm+2qa0lX4dA41WbXZnYzQ5UFlQyjC/iymwmrl+v73 On1cO6LCgFH6McNMsqEiCfjh8JjI+U32Konb1fujsYeHllrHJnLsiDT8CSHwm7O658si eBBP0uvsOzzJqPvuxINVCO0KzMUPhQ63Zu6TFP2fd6Sj5aHHJVPEtKkRYfenXCSb+UJ7 Ua0EWUWwUOBlbuFvC+02xHuOf+ieJrmyz8C1K7XeIAWKmY9m25pUSnLfKbVafs8GkJUF UXGSBBosv36TrXlPtBZrMek9VDtiXvtLFs/4JteDGjttD169PyNJ2JLuchGp7POXh9GR hKZA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=Hm0xvk61lC36xzyQyVliI6spl0vKmNv0Vdnc0gQng1M=; fh=U03yDx5+moqkMhn093cJN4duio0SI1zMzIuKNFWiSIw=; b=MFJjy+oDDQorK8WpKef8OpPujUiUOr0JBCDzkiqBNqnxBBRFv93ApWDaNCPCIQecJo ciZ7PrEJX2WZzy2OfSBRfnzwMzWBMnDUx8u4Y+lsngfdbx7cCUlmu9JceoSivp2tyPJ/ OYWX8K9X0p21EfHe40CnEAlwEnU5GK4SU8BIm1WbfBbV/VausN1vFypwg0U3zNwnvA+E rE2/Llwqymc0En/GbNKcmrUnuThKnDwU0G9qCUmALyZyj7rTAP8hCVALFZyQvrzOA2/W +sKfiNSrgqxx+e8iySLM0b7TSOjtKQYtG/0e+xNN5f1eZ2ZQ3tsMX/HYFIP6U15D14rM 1Vqg== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@amd.com header.s=selector1 header.b=cfmw1tWn; arc=pass (i=1 spf=pass spfdomain=amd.com dmarc=pass fromdomain=amd.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amd.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id w15-20020a81a20f000000b005a4d521ff80si15471434ywg.569.2023.10.26.10.04.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Oct 2023 10:04:17 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@amd.com header.s=selector1 header.b=cfmw1tWn; arc=pass (i=1 spf=pass spfdomain=amd.com dmarc=pass fromdomain=amd.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amd.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id A04C1821821A; Thu, 26 Oct 2023 10:04:07 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231738AbjJZRD6 (ORCPT <rfc822;aposhian.dev@gmail.com> + 26 others); Thu, 26 Oct 2023 13:03:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54910 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345135AbjJZRDw (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 26 Oct 2023 13:03:52 -0400 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12on2084.outbound.protection.outlook.com [40.107.243.84]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4498F91; Thu, 26 Oct 2023 10:03:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kGri0r0O9QUEcvMerlWZ0YKQtomqdvsorBlVANBM4q50DGsdDnokgtjrTq0G6c3iuzBV5kaWG8TVoKHaasAS5bbd9bWWWATSDRbzT/1k1lO2S6fIM/kdJFSzNwSdBgBA1BLmxAENceNNNhQ4B/Frr/05OOwV5SGO7tTjhMQAe4GH2kzjGegVYsSH8GyYckTvL4wzmnECRPHaiJgjd30WoR88yDAzrWU+AlH1CnZzbZSrx1QK2gXybHAz1gOUZDBAusOOiLiAPdzgn+teEqBIcsPz4iKV2S8hpQVhlMVHDLMFYlM3b1GrUZSmSodA19zVCOS52CYiMRjubv6CgcNm2A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Hm0xvk61lC36xzyQyVliI6spl0vKmNv0Vdnc0gQng1M=; b=c8f4yXJUa9FYrhKzG7Q8rQQ4PcadBruKsdOFe3PKpgtMqvw4G3lGSqI9EnKnyiVR/dM8PA1XbWIM/nscQrjtYgzevfSOUHtO72+57o2pLNpg9D+5Yv453iuIOuFLrAyu97ZMi1ZHCuYOzT4sz95UOekdfF7VQMAb8KImArcA0Zdk20eKE0DF6LsOg+xmvoNdhYLZZbxtl5ViieXoCqHHWVY0YoeXrSUHRT4AidAxsqrYRR9IlF80Gwl+vHTRIC+MpIpUM8aEjPQiDEi6eQR57Fa+opP+iDluHA62yiRR1dsCu1ivvPAyfIb3U8ynZjKWEA0XC2waSUaG71vaidjlaQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=infradead.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Hm0xvk61lC36xzyQyVliI6spl0vKmNv0Vdnc0gQng1M=; b=cfmw1tWnKoWslbVV6QMx/o6Wx1fMmHY9t6zywOh8v3nUbUzn1O+xevZV4OH8sNXr8J1pF2gmKu16oEaQD8nStvnM2nYp2AtIRPPE4fcAmVE1365pDQWMZ3xfYlMrHBcBrBGhxQmo3NR1ERIMnvZo+46OvWZQ6LwyRPi6dMtoeYk= Received: from BL1PR13CA0127.namprd13.prod.outlook.com (2603:10b6:208:2bb::12) by SA0PR12MB4591.namprd12.prod.outlook.com (2603:10b6:806:9d::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6907.33; Thu, 26 Oct 2023 17:03:47 +0000 Received: from BL02EPF0001A0FE.namprd03.prod.outlook.com (2603:10b6:208:2bb:cafe::72) by BL1PR13CA0127.outlook.office365.com (2603:10b6:208:2bb::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6933.19 via Frontend Transport; Thu, 26 Oct 2023 17:03:46 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by BL02EPF0001A0FE.mail.protection.outlook.com (10.167.242.105) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.6933.15 via Frontend Transport; Thu, 26 Oct 2023 17:03:46 +0000 Received: from AUS-P9-MLIMONCI.amd.com (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.32; Thu, 26 Oct 2023 12:03:44 -0500 From: Mario Limonciello <mario.limonciello@amd.com> To: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Thomas Gleixner <tglx@linutronix.de>, Borislav Petkov <bp@alien8.de>, "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@kernel.org> CC: Arnaldo Carvalho de Melo <acme@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, "Namhyung Kim" <namhyung@kernel.org>, Ian Rogers <irogers@google.com>, Adrian Hunter <adrian.hunter@intel.com>, Dave Hansen <dave.hansen@linux.intel.com>, "H . Peter Anvin" <hpa@zytor.com>, "Rafael J . Wysocki" <rafael@kernel.org>, "Len Brown" <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>, David Woodhouse <dwmw@amazon.co.uk>, Sandipan Das <sandipan.das@amd.com>, "open list:PERFORMANCE EVENTS SUBSYSTEM" <linux-perf-users@vger.kernel.org>, "open list:PERFORMANCE EVENTS SUBSYSTEM" <linux-kernel@vger.kernel.org>, "open list:SUSPEND TO RAM" <linux-pm@vger.kernel.org>, "open list:ACPI" <linux-acpi@vger.kernel.org>, Mario Limonciello <mario.limonciello@amd.com> Subject: [PATCH v2 1/2] x86: Enable x2apic during resume from suspend if used previously Date: Thu, 26 Oct 2023 12:03:29 -0500 Message-ID: <20231026170330.4657-2-mario.limonciello@amd.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20231026170330.4657-1-mario.limonciello@amd.com> References: <20231026170330.4657-1-mario.limonciello@amd.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.180.168.240] X-ClientProxiedBy: SATLEXMB03.amd.com (10.181.40.144) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BL02EPF0001A0FE:EE_|SA0PR12MB4591:EE_ X-MS-Office365-Filtering-Correlation-Id: 54e9b4de-72d2-44f8-b405-08dbd645845d X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: tnPFkUkNWnS/9xviZ6lyI7EySxkxsHRnn3h9B/Rty51wf/xaLBM0WGZfYFyhV5x6WeibDEDvB9KwY5LoAAOvIzVLQf56QPLfz8Jbd7Cw0g3+pEk9jZASIZw7TBc7kNaAMoiZIlRakVMQDmebOfuDRep6m6E1U1sF6qPZnS4MXqFwg31sIeZcnsaNwcfuzJIXcTllKyuMiDCTYT5SrbPL6pI1B+xTMsLLFMLD9YT7QsFBbXNihGkrsdi/7lUhv329ezLAXgFB3fZt6ZXTrP89W96WZP9QH/P3oCFHeXw3rgFmvmP3nFAr6RADPknYYir3L/YQFx10TRb4h9D+kuqCEc2SyJxEWWjmLJM6ORqxik2f0IvUoU/oV+C3tmWF2y7s91z+/S7ov5K2xSsgEluRuwb3B5ZMKEy268MKksvhbuMJ48khijCzFBmq9LNANQYJTWZi20kuQ0n9pUTBCAHMGo9NgGAgbAdOuiO0jGaIXByDnW9hrUnqJCnFwiwh+7Hfy5UFWobIdbXKahEZ4nBBAK92LG/Gtsg9eB9X7RLh6glmcD2Gq9StbH9KAyC9Ws2/rqbKxt6O0ApQQEa4xAByZrsiVJ8CB3wW2mnEROdkd6hKuJkOcLUR8nKXOQUiTOGoDMfbZdYTP+8Lt6HVEcnu2/KzCbfWNSKIleQXZZ6isW/z1hqtCuczH5rzLSwt5TOekWf80BWVP+Ck68bmuBjzadnJr63GCpktTGC6DkdgH5WoJ7cAGhHPbityfL0bYxijBRROiivtvMsdohlk/KcuDQ== X-Forefront-Antispam-Report: CIP:165.204.84.17;CTRY:US;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:SATLEXMB04.amd.com;PTR:InfoDomainNonexistent;CAT:NONE;SFS:(13230031)(4636009)(396003)(39860400002)(376002)(346002)(136003)(230922051799003)(1800799009)(186009)(64100799003)(82310400011)(451199024)(40470700004)(46966006)(36840700001)(41300700001)(44832011)(36756003)(2906002)(5660300002)(110136005)(81166007)(356005)(2616005)(54906003)(82740400003)(40460700003)(70586007)(478600001)(7696005)(1076003)(426003)(40480700001)(6666004)(47076005)(83380400001)(336012)(70206006)(36860700001)(86362001)(4326008)(8676002)(7416002)(15650500001)(316002)(8936002)(26005)(16526019)(36900700001);DIR:OUT;SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2023 17:03:46.4409 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 54e9b4de-72d2-44f8-b405-08dbd645845d X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: BL02EPF0001A0FE.namprd03.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR12MB4591 X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO, RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 26 Oct 2023 10:04:07 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780838414332780611 X-GMAIL-MSGID: 1780838414332780611 |
Series |
Fixes for s3 with parallel bootup
|
|
Commit Message
Mario Limonciello
Oct. 26, 2023, 5:03 p.m. UTC
If x2apic was enabled during boot with parallel startup
it will be needed during resume from suspend to ram as well.
Store whether to enable into the smpboot_control global variable
and during startup re-enable it if necessary.
This fixes resume from suspend on workstation CPUs with x2apic
enabled.
It will also work on systems with one maxcpus=1 but still using
x2apic since x2apic is also re-enabled in lapic_resume().
Cc: stable@vger.kernel.org # 6.5
Fixes: 0c7ffa32dbd6 ("x86/smpboot/64: Implement arch_cpuhp_init_parallel_bringup() and enable it")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
* Clarify it's in workstations in commit message
* Fix style issues in comment and curly braces
---
arch/x86/include/asm/smp.h | 1 +
arch/x86/kernel/acpi/sleep.c | 13 +++++++++----
arch/x86/kernel/head_64.S | 15 +++++++++++++++
3 files changed, 25 insertions(+), 4 deletions(-)
Comments
On Thu, Oct 26, 2023 at 7:03 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > If x2apic was enabled during boot with parallel startup > it will be needed during resume from suspend to ram as well. > > Store whether to enable into the smpboot_control global variable > and during startup re-enable it if necessary. > > This fixes resume from suspend on workstation CPUs with x2apic > enabled. > > It will also work on systems with one maxcpus=1 but still using > x2apic since x2apic is also re-enabled in lapic_resume(). > > Cc: stable@vger.kernel.org # 6.5 > Fixes: 0c7ffa32dbd6 ("x86/smpboot/64: Implement arch_cpuhp_init_parallel_bringup() and enable it") > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> Acked-by: Rafael J. Wysocki <rafael@kernel.org> > --- > v1->v2: > * Clarify it's in workstations in commit message > * Fix style issues in comment and curly braces > --- > arch/x86/include/asm/smp.h | 1 + > arch/x86/kernel/acpi/sleep.c | 13 +++++++++---- > arch/x86/kernel/head_64.S | 15 +++++++++++++++ > 3 files changed, 25 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h > index c31c633419fe..86584ffaebc3 100644 > --- a/arch/x86/include/asm/smp.h > +++ b/arch/x86/include/asm/smp.h > @@ -190,6 +190,7 @@ extern unsigned long apic_mmio_base; > #endif /* !__ASSEMBLY__ */ > > /* Control bits for startup_64 */ > +#define STARTUP_ENABLE_X2APIC 0x40000000 > #define STARTUP_READ_APICID 0x80000000 > > /* Top 8 bits are reserved for control */ > diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c > index 6dfecb27b846..10d8921b4bb8 100644 > --- a/arch/x86/kernel/acpi/sleep.c > +++ b/arch/x86/kernel/acpi/sleep.c > @@ -11,6 +11,7 @@ > #include <linux/dmi.h> > #include <linux/cpumask.h> > #include <linux/pgtable.h> > +#include <asm/apic.h> > #include <asm/segment.h> > #include <asm/desc.h> > #include <asm/cacheflush.h> > @@ -129,12 +130,16 @@ int x86_acpi_suspend_lowlevel(void) > */ > current->thread.sp = (unsigned long)temp_stack + sizeof(temp_stack); > /* > - * Ensure the CPU knows which one it is when it comes back, if > - * it isn't in parallel mode and expected to work that out for > - * itself. > + * Ensure x2apic is re-enabled if necessary and the CPU knows which > + * one it is when it comes back, if it isn't in parallel mode and > + * expected to work that out for itself. > */ > - if (!(smpboot_control & STARTUP_PARALLEL_MASK)) > + if (smpboot_control & STARTUP_PARALLEL_MASK) { > + if (x2apic_enabled()) > + smpboot_control |= STARTUP_ENABLE_X2APIC; > + } else { > smpboot_control = smp_processor_id(); > + } > #endif > initial_code = (unsigned long)wakeup_long64; > saved_magic = 0x123456789abcdef0L; > diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S > index ea6995920b7a..300901af9fa3 100644 > --- a/arch/x86/kernel/head_64.S > +++ b/arch/x86/kernel/head_64.S > @@ -237,9 +237,14 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL) > * CPU number is encoded in smpboot_control. > * > * Bit 31 STARTUP_READ_APICID (Read APICID from APIC) > + * Bit 30 STARTUP_ENABLE_X2APIC (Enable X2APIC mode) > * Bit 0-23 CPU# if STARTUP_xx flags are not set > */ > movl smpboot_control(%rip), %ecx > + > + testl $STARTUP_ENABLE_X2APIC, %ecx > + jnz .Lenable_x2apic > + > testl $STARTUP_READ_APICID, %ecx > jnz .Lread_apicid > /* > @@ -249,6 +254,16 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL) > andl $(~STARTUP_PARALLEL_MASK), %ecx > jmp .Lsetup_cpu > > +.Lenable_x2apic: > + /* Enable X2APIC if disabled */ > + mov $MSR_IA32_APICBASE, %ecx > + rdmsr > + testl $X2APIC_ENABLE, %eax > + jnz .Lread_apicid_msr > + orl $X2APIC_ENABLE, %eax > + wrmsr > + jmp .Lread_apicid_msr > + > .Lread_apicid: > /* Check whether X2APIC mode is already enabled */ > mov $MSR_IA32_APICBASE, %ecx > -- > 2.34.1 >
Mario! On Thu, Oct 26 2023 at 12:03, Mario Limonciello wrote: > If x2apic was enabled during boot with parallel startup > it will be needed during resume from suspend to ram as well. Lacks an explanation why it is needed. > Store whether to enable into the smpboot_control global variable > and during startup re-enable it if necessary. > > This fixes resume from suspend on workstation CPUs with x2apic > enabled. You completely fail to describe the failure mode. > It will also work on systems with one maxcpus=1 but still using > x2apic since x2apic is also re-enabled in lapic_resume(). This sentence makes no sense. What's so special about maxcpus=1? Absolutely nothing. lapic_resume() is a syscore op and is always invoked on the CPU which handles suspend/resume. The point is that __x2apic_enable() which is invoked from there becomes a NOOP because X2APIC is already enabled. So what are you trying to tell me? I really appreciate your enthusiasm of chasing and fixing bugs, but your change logs and explanations are really making it hard to keep that appreciation up. > /* > - * Ensure the CPU knows which one it is when it comes back, if > - * it isn't in parallel mode and expected to work that out for > - * itself. > + * Ensure x2apic is re-enabled if necessary and the CPU knows which > + * one it is when it comes back, if it isn't in parallel mode and > + * expected to work that out for itself. The x2apic comment is misplaced. It should be above the x2apic conditional as it has nothing to do with the initial condition because even if X2APIC is enabled then the parallel startup might be disabled. Go and read this comment 3 month from now and try to make sense of it. > */ > - if (!(smpboot_control & STARTUP_PARALLEL_MASK)) > + if (smpboot_control & STARTUP_PARALLEL_MASK) { > + if (x2apic_enabled()) > + smpboot_control |= STARTUP_ENABLE_X2APIC; This bit is sticky after resume, so any subsequent CPU hotplug operation will see it as well. This lacks an explanation why this is correct and why this flag is not set early during boot before the APs are brought up. > + } else { > smpboot_control = smp_processor_id(); > + } > #endif > initial_code = (unsigned long)wakeup_long64; > saved_magic = 0x123456789abcdef0L; > diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S > index ea6995920b7a..300901af9fa3 100644 > --- a/arch/x86/kernel/head_64.S > +++ b/arch/x86/kernel/head_64.S > @@ -237,9 +237,14 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL) > * CPU number is encoded in smpboot_control. > * > * Bit 31 STARTUP_READ_APICID (Read APICID from APIC) > + * Bit 30 STARTUP_ENABLE_X2APIC (Enable X2APIC mode) > * Bit 0-23 CPU# if STARTUP_xx flags are not set > */ > movl smpboot_control(%rip), %ecx > + > + testl $STARTUP_ENABLE_X2APIC, %ecx > + jnz .Lenable_x2apic Why is this tested here? The test clearly belongs into .Lread_apicid > + > testl $STARTUP_READ_APICID, %ecx > jnz .Lread_apicid > /* > @@ -249,6 +254,16 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL) > andl $(~STARTUP_PARALLEL_MASK), %ecx > jmp .Lsetup_cpu > > +.Lenable_x2apic: > + /* Enable X2APIC if disabled */ > + mov $MSR_IA32_APICBASE, %ecx > + rdmsr > + testl $X2APIC_ENABLE, %eax > + jnz .Lread_apicid_msr > + orl $X2APIC_ENABLE, %eax > + wrmsr > + jmp .Lread_apicid_msr And this part just moves in front of .Lread_apicid_msr and spares the jump at the end. > .Lread_apicid: > /* Check whether X2APIC mode is already enabled */ > mov $MSR_IA32_APICBASE, %ecx That aside, I'm still failing to see the actual failure scenario due to the utter void in the change log. The kernel has two mechanisms which end up with X2APIC enabled: 1) BIOS has it enabled already, which is required for any machine which has more than 255 CPUs, but BIOS can decide to enable X2APIC even with less than 256 CPUs. 2) BIOS has not enabled it, but the kernel enables it because the CPU supports it. The major difference is: #1 prevents the MMIO fixmap for the APIC to be installed #2 installs the fixmap but does not use it. It's never torn down. Let's look at these two cases in the light of resume: #1 If the BIOS enabled X2APIC mode then the only way how this can fail on resume is that the BIOS did not enable X2APIC mode in the resume path before going back into the kernel and due to the non-existent MMIO mapping there is no way to read the APIC ID. #2 It does not matter whether the BIOS enabled X2APIC mode during resume because the MMIO mapping exists and the APIC ID read via MMIO should be identical to the APIC ID read via the X2APIC MSR. If not, then there is something fundamentally wrong. How is this working during the initial bringup of the APs? #1 If the APs do not have X2APIC enabled by the BIOS then they will crash and burn during bringup due to non-existent MMIO mapping. #2 The APs can read their APIC ID just fine via MMIO and it obviously is the same as the APIC ID after the bringup enabled X2APIC mode. Otherwise the kernel would not work at all. So the only reasonable explanation for the failure is that the BIOS fails to reenable X2APIC mode on resume either on the CPU which handles suspend/resume or on the subsequent AP bringups, which is not clear at all due to the bit being sticky and the changelog being full of void in that aspect. That said the sticky bit is wrong for the following case with older CPUs where X2APIC requires up to date microcode loaded to work correctly: boot maxcpus=4 load microcode // Same sequence applies for AP (CPU1-3) enable x2apic suspend set X2APIC enable bit in smpboot_control resume bringup CPU4 enable X2APIC early --> fail due to lack of microcode fix Whether this affects the APIC ID readout or not, which we don't know and even if we consider this case to be academic, it's still fundamentally wrong from a correctness point of view. So without proper information about the failure scenario this "fix" is simply going nowhere. Please provide the following information: - dmesg of the initial boot up to 'smp: Bringing up secondary CPUs ...' - A proper description of the failure case: - Is the CPU which handles suspend/resume failing? - Is a subsequent AP bringup failing? - Is the failure due to the lack of MMIO mapping ? - Is the failure due to a bogus APIC ID retrieved via MMIO ? Thanks for making me do your homework (again), tglx
On 10/27/2023 16:31, Thomas Gleixner wrote: > Mario! > > On Thu, Oct 26 2023 at 12:03, Mario Limonciello wrote: > >> If x2apic was enabled during boot with parallel startup >> it will be needed during resume from suspend to ram as well. > > Lacks an explanation why it is needed. > >> Store whether to enable into the smpboot_control global variable >> and during startup re-enable it if necessary. >> >> This fixes resume from suspend on workstation CPUs with x2apic >> enabled. > > You completely fail to describe the failure mode. > >> It will also work on systems with one maxcpus=1 but still using >> x2apic since x2apic is also re-enabled in lapic_resume(). > > This sentence makes no sense. What's so special about maxcpus=1? > > Absolutely nothing. > > lapic_resume() is a syscore op and is always invoked on the CPU which > handles suspend/resume. The point is that __x2apic_enable() which is > invoked from there becomes a NOOP because X2APIC is already enabled. > > So what are you trying to tell me? > > I really appreciate your enthusiasm of chasing and fixing bugs, but your > change logs and explanations are really making it hard to keep that > appreciation up. > >> /* >> - * Ensure the CPU knows which one it is when it comes back, if >> - * it isn't in parallel mode and expected to work that out for >> - * itself. >> + * Ensure x2apic is re-enabled if necessary and the CPU knows which >> + * one it is when it comes back, if it isn't in parallel mode and >> + * expected to work that out for itself. > > The x2apic comment is misplaced. It should be above the x2apic > conditional as it has nothing to do with the initial condition because > even if X2APIC is enabled then the parallel startup might be disabled. > > Go and read this comment 3 month from now and try to make sense of it. > >> */ >> - if (!(smpboot_control & STARTUP_PARALLEL_MASK)) >> + if (smpboot_control & STARTUP_PARALLEL_MASK) { >> + if (x2apic_enabled()) >> + smpboot_control |= STARTUP_ENABLE_X2APIC; > > This bit is sticky after resume, so any subsequent CPU hotplug operation > will see it as well. > > This lacks an explanation why this is correct and why this flag is not > set early during boot before the APs are brought up. > >> + } else { >> smpboot_control = smp_processor_id(); >> + } >> #endif >> initial_code = (unsigned long)wakeup_long64; >> saved_magic = 0x123456789abcdef0L; >> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S >> index ea6995920b7a..300901af9fa3 100644 >> --- a/arch/x86/kernel/head_64.S >> +++ b/arch/x86/kernel/head_64.S >> @@ -237,9 +237,14 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL) >> * CPU number is encoded in smpboot_control. >> * >> * Bit 31 STARTUP_READ_APICID (Read APICID from APIC) >> + * Bit 30 STARTUP_ENABLE_X2APIC (Enable X2APIC mode) >> * Bit 0-23 CPU# if STARTUP_xx flags are not set >> */ >> movl smpboot_control(%rip), %ecx >> + >> + testl $STARTUP_ENABLE_X2APIC, %ecx >> + jnz .Lenable_x2apic > > Why is this tested here? The test clearly belongs into .Lread_apicid > >> + >> testl $STARTUP_READ_APICID, %ecx >> jnz .Lread_apicid >> /* >> @@ -249,6 +254,16 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL) >> andl $(~STARTUP_PARALLEL_MASK), %ecx >> jmp .Lsetup_cpu >> >> +.Lenable_x2apic: >> + /* Enable X2APIC if disabled */ >> + mov $MSR_IA32_APICBASE, %ecx >> + rdmsr >> + testl $X2APIC_ENABLE, %eax >> + jnz .Lread_apicid_msr >> + orl $X2APIC_ENABLE, %eax >> + wrmsr >> + jmp .Lread_apicid_msr > > And this part just moves in front of .Lread_apicid_msr and spares > the jump at the end. > >> .Lread_apicid: >> /* Check whether X2APIC mode is already enabled */ >> mov $MSR_IA32_APICBASE, %ecx > > That aside, I'm still failing to see the actual failure scenario due to > the utter void in the change log. > > The kernel has two mechanisms which end up with X2APIC enabled: > > 1) BIOS has it enabled already, which is required for any machine > which has more than 255 CPUs, but BIOS can decide to enable > X2APIC even with less than 256 CPUs. > > 2) BIOS has not enabled it, but the kernel enables it because the > CPU supports it. > > The major difference is: > > #1 prevents the MMIO fixmap for the APIC to be installed > > #2 installs the fixmap but does not use it. It's never torn down. > > Let's look at these two cases in the light of resume: > > #1 If the BIOS enabled X2APIC mode then the only way how this can > fail on resume is that the BIOS did not enable X2APIC mode in the > resume path before going back into the kernel and due to the > non-existent MMIO mapping there is no way to read the APIC ID. > > #2 It does not matter whether the BIOS enabled X2APIC mode during > resume because the MMIO mapping exists and the APIC ID read via > MMIO should be identical to the APIC ID read via the X2APIC MSR. > > If not, then there is something fundamentally wrong. > > How is this working during the initial bringup of the APs? > > #1 If the APs do not have X2APIC enabled by the BIOS then they will > crash and burn during bringup due to non-existent MMIO mapping. > > #2 The APs can read their APIC ID just fine via MMIO and it > obviously is the same as the APIC ID after the bringup enabled > X2APIC mode. Otherwise the kernel would not work at all. > > So the only reasonable explanation for the failure is that the BIOS > fails to reenable X2APIC mode on resume either on the CPU which handles > suspend/resume or on the subsequent AP bringups, which is not clear at > all due to the bit being sticky and the changelog being full of void in > that aspect. > > That said the sticky bit is wrong for the following case with older CPUs > where X2APIC requires up to date microcode loaded to work correctly: > > boot maxcpus=4 > load microcode // Same sequence applies for AP (CPU1-3) > enable x2apic > suspend > set X2APIC enable bit in smpboot_control > resume > bringup CPU4 > enable X2APIC early --> fail due to lack of microcode fix > > Whether this affects the APIC ID readout or not, which we don't know and > even if we consider this case to be academic, it's still fundamentally > wrong from a correctness point of view. > > So without proper information about the failure scenario this "fix" is > simply going nowhere. > > Please provide the following information: > > - dmesg of the initial boot up to 'smp: Bringing up secondary CPUs ...' > > - A proper description of the failure case: > > - Is the CPU which handles suspend/resume failing? > > - Is a subsequent AP bringup failing? > > - Is the failure due to the lack of MMIO mapping ? > > - Is the failure due to a bogus APIC ID retrieved via MMIO ? > > Thanks for making me do your homework (again), > > tglx Hi Thomas, Thank you for looking this over. I've reviewed it with the internal team and confirmed there was a BIOS bug where the MSR wasn't restored after the S3 cycle completed. The BIOS team has fixed it. Thanks, #regzbot invalid: BIOS bug
On Tue, Oct 31 2023 at 13:53, Mario Limonciello wrote: > Thank you for looking this over. I've reviewed it with the internal > team and confirmed there was a BIOS bug where the MSR wasn't restored > after the S3 cycle completed. The BIOS team has fixed it. Why am I not surprised? Thanks for the heads up! tglx
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h index c31c633419fe..86584ffaebc3 100644 --- a/arch/x86/include/asm/smp.h +++ b/arch/x86/include/asm/smp.h @@ -190,6 +190,7 @@ extern unsigned long apic_mmio_base; #endif /* !__ASSEMBLY__ */ /* Control bits for startup_64 */ +#define STARTUP_ENABLE_X2APIC 0x40000000 #define STARTUP_READ_APICID 0x80000000 /* Top 8 bits are reserved for control */ diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c index 6dfecb27b846..10d8921b4bb8 100644 --- a/arch/x86/kernel/acpi/sleep.c +++ b/arch/x86/kernel/acpi/sleep.c @@ -11,6 +11,7 @@ #include <linux/dmi.h> #include <linux/cpumask.h> #include <linux/pgtable.h> +#include <asm/apic.h> #include <asm/segment.h> #include <asm/desc.h> #include <asm/cacheflush.h> @@ -129,12 +130,16 @@ int x86_acpi_suspend_lowlevel(void) */ current->thread.sp = (unsigned long)temp_stack + sizeof(temp_stack); /* - * Ensure the CPU knows which one it is when it comes back, if - * it isn't in parallel mode and expected to work that out for - * itself. + * Ensure x2apic is re-enabled if necessary and the CPU knows which + * one it is when it comes back, if it isn't in parallel mode and + * expected to work that out for itself. */ - if (!(smpboot_control & STARTUP_PARALLEL_MASK)) + if (smpboot_control & STARTUP_PARALLEL_MASK) { + if (x2apic_enabled()) + smpboot_control |= STARTUP_ENABLE_X2APIC; + } else { smpboot_control = smp_processor_id(); + } #endif initial_code = (unsigned long)wakeup_long64; saved_magic = 0x123456789abcdef0L; diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S index ea6995920b7a..300901af9fa3 100644 --- a/arch/x86/kernel/head_64.S +++ b/arch/x86/kernel/head_64.S @@ -237,9 +237,14 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL) * CPU number is encoded in smpboot_control. * * Bit 31 STARTUP_READ_APICID (Read APICID from APIC) + * Bit 30 STARTUP_ENABLE_X2APIC (Enable X2APIC mode) * Bit 0-23 CPU# if STARTUP_xx flags are not set */ movl smpboot_control(%rip), %ecx + + testl $STARTUP_ENABLE_X2APIC, %ecx + jnz .Lenable_x2apic + testl $STARTUP_READ_APICID, %ecx jnz .Lread_apicid /* @@ -249,6 +254,16 @@ SYM_INNER_LABEL(secondary_startup_64_no_verify, SYM_L_GLOBAL) andl $(~STARTUP_PARALLEL_MASK), %ecx jmp .Lsetup_cpu +.Lenable_x2apic: + /* Enable X2APIC if disabled */ + mov $MSR_IA32_APICBASE, %ecx + rdmsr + testl $X2APIC_ENABLE, %eax + jnz .Lread_apicid_msr + orl $X2APIC_ENABLE, %eax + wrmsr + jmp .Lread_apicid_msr + .Lread_apicid: /* Check whether X2APIC mode is already enabled */ mov $MSR_IA32_APICBASE, %ecx