From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1438C47422 for ; Sun, 21 Jan 2024 11:49:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5F5286B006E; Sun, 21 Jan 2024 06:49:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5A5716B0071; Sun, 21 Jan 2024 06:49:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 46D596B0072; Sun, 21 Jan 2024 06:49:55 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 370E56B006E for ; Sun, 21 Jan 2024 06:49:55 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id A7645140342 for ; Sun, 21 Jan 2024 11:49:53 +0000 (UTC) X-FDA: 81703149066.26.96D55A6 Received: from mail.alien8.de (mail.alien8.de [65.109.113.108]) by imf09.hostedemail.com (Postfix) with ESMTP id 2A0A0140012 for ; Sun, 21 Jan 2024 11:49:50 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=alien8.de header.s=alien8 header.b=PlL2423n; dmarc=pass (policy=none) header.from=alien8.de; spf=pass (imf09.hostedemail.com: domain of bp@alien8.de designates 65.109.113.108 as permitted sender) smtp.mailfrom=bp@alien8.de ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1705837792; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=NjXjwG84zDH31kxmFiKAYsCtuTb3t8NMg29SPEaQwJk=; b=Pmm/UQWbbCyTbes9eZJQLu+fmkojyJBs1kDEOhZ76robvv2IeRqMgHUhtIxJQZqKnNbVKq L6pBlQAqe+T6pKXmNtVX0/4WlKPJJVjdo1HE1/HkwhokGgbi2n6QruFAK91Qn7Mwri37Z3 L5NqUvuXi1KeUsgI9BNfk5GXJN+8fqU= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=alien8.de header.s=alien8 header.b=PlL2423n; dmarc=pass (policy=none) header.from=alien8.de; spf=pass (imf09.hostedemail.com: domain of bp@alien8.de designates 65.109.113.108 as permitted sender) smtp.mailfrom=bp@alien8.de ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705837792; a=rsa-sha256; cv=none; b=x/OX46ykklNyGNPy+fmoHwWQdBpoZUYwpADFO3xfK6SWXzXY7O6q2wNMi1VX4iXqc3KOsk 2VzgpAJZzU1R8rwv3kEW8xKgWq48vNohFdg9OGUIjXSrYP0czHjVzhcqh+kyxDn2i5htfd 97kkjqzwCdAtTsEYl0/zG6SFkpzmCxA= Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.alien8.de (SuperMail on ZX Spectrum 128k) with ESMTP id BA4F240E016C; Sun, 21 Jan 2024 11:49:46 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at mail.alien8.de Received: from mail.alien8.de ([127.0.0.1]) by localhost (mail.alien8.de [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id QufI0F2hHmPV; Sun, 21 Jan 2024 11:49:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=alien8; t=1705837783; bh=NjXjwG84zDH31kxmFiKAYsCtuTb3t8NMg29SPEaQwJk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=PlL2423nlQeFrtrBAT6D+YTiXUCjiejwKQmjF6Qt76EveRL7nDm5xV42nzDyPpmsB U2pcNTFIG2xe0ub4ZvTru7rdlcY93m/hbBhlZhJRHqkFp05pSXS0/ZeNLnsE8ZRc31 XlHCJ/UlTUCCnYgKhdfFGLrZuQL4nHirdgTuLLP4IgF8oMbzvYhiEctbcenlaBnD+L lDZ3y+Kzgxhhke3qG4kDW0dNHc9G3C0SYUgLleJ0NK0GmpXD4sK77WVflFcjixRuXh inu4MhNGDyGsoa6cEpEfXMS6/wxWlTLxAHs5zDT24B8gBqOIMt9cfxmBAX1Ra/sM5c flj1ELyoGmNx+HzY0XMJbQsik2QiItZS9wKXc1SzfmbL6SHkKAJ5YLQmVNOrDuNHvF BolnUhyQiLdf8Av9StDVO5rCwzzssAP3k6bxiIHvQmvBFzH0QRvLF4Zj3Q/nw+anfS UZGQyn18bC6Bynzp/7Nioxkw5MQeyx6AKNzcvoFL14jcrd8VQ8RV4NtTQTELdz0RVR oBvGw1lsBFNa5PngqP0q5m1DE9+sXjwKl8Na+tvmY9XG42Jjs0ENcP2RoFrkeByMdJ iI/s9uwHiDepyvvP7iSBtCvQATuGtkteWIWprDR7wpNfypyv74C7dTTXoALPlENo4q NeXgzZVABJEBCQVA3nYxk4DQ= Received: from zn.tnic (pd953099d.dip0.t-ipconnect.de [217.83.9.157]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature ECDSA (P-256) server-digest SHA256) (No client certificate requested) by mail.alien8.de (SuperMail on ZX Spectrum 128k) with ESMTPSA id 2FEF540E0177; Sun, 21 Jan 2024 11:49:07 +0000 (UTC) Date: Sun, 21 Jan 2024 12:49:00 +0100 From: Borislav Petkov To: Michael Roth Cc: x86@kernel.org, kvm@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@redhat.com, jroedel@suse.de, thomas.lendacky@amd.com, hpa@zytor.com, ardb@kernel.org, pbonzini@redhat.com, seanjc@google.com, vkuznets@redhat.com, jmattson@google.com, luto@kernel.org, dave.hansen@linux.intel.com, slp@redhat.com, pgonda@google.com, peterz@infradead.org, srinivas.pandruvada@linux.intel.com, rientjes@google.com, tobin@ibm.com, vbabka@suse.cz, kirill@shutemov.name, ak@linux.intel.com, tony.luck@intel.com, sathyanarayanan.kuppuswamy@linux.intel.com, alpergun@google.com, jarkko@kernel.org, ashish.kalra@amd.com, nikunj.dadhania@amd.com, pankaj.gupta@amd.com, liam.merwick@oracle.com Subject: Re: [PATCH v1 21/26] crypto: ccp: Add panic notifier for SEV/SNP firmware shutdown on kdump Message-ID: <20240121114900.GLZa0ErBHIqvook5zK@fat_crate.local> References: <20231230161954.569267-1-michael.roth@amd.com> <20231230161954.569267-22-michael.roth@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20231230161954.569267-22-michael.roth@amd.com> X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 2A0A0140012 X-Stat-Signature: c6j14nmoi9hmknpok7ewdz9i6bu7kyxd X-HE-Tag: 1705837790-623806 X-HE-Meta: U2FsdGVkX1+03stVQLsmISjof/gDFuD5sstRWgldRoSsGpmB+y5SUFNgzckan3OXrXTHU9t5jdnO/j5zPcu4DdrdioB+pz4Z3n7RHCon3FZImFDY2+LwLAr7ozMA/0WaiBvf6hpGlxT63IRGGzXuKfz4Q5om2032Kl3n1FhwnnpyHzDzQw99nP56cvtqNfrm0JTM6m8kKW3uCV3k9Hwor3Iu3YInAV1f5RAbXV70liD8BFdfQGNtP+Gyb5z8WNOu0NWYDYnbJZdB+HUWosHmMzNSRoV0pPPfkZtksKt0QZXyABXDQTedf54yW8DN+1fuOkCN8PfJXpVg22TNdobA3jbPssPvADu0IBio/o6BVFdFfB8WkdDNBJZ9mqv17CjdyUVUkiWkhsIcIulAytrf3oznLNTZQztlmMUv3vTE1q/nL4K8jGsU/94p9jjsdeV6+VGCkMl4xZWcJRyRrwUHaXKnqRwo1FwWkHslhE2ih774Xfj3/geH6GcAP8GGbb56+JXN122VEVSCekaNAtTgMDEKbU+ig6pjJuPyIb8MKNNloDkchFBS9tr5BKH49gUZ6/GEifabVoAYNzM7TId/+kAv0FYG1DWtfXyGHbOHVyP4RcpQO8f+xupiO8/ef+Apj81cpqUOCGXl9qD0E0x7L9HsH4go4Kz8QFFOXKbOp+0Coj9e9hJDwAHwaz1ZO5iuE8MOALsP6vzZqPWnoQWH26+NOVw/SMRD3WRjS41oU1nQ5FYDc2zCuenmPCpu21syg2vZWc9SQuUfL7ysAkaNTlmbDczU43axhWqLiUHlkSafzooFLVGJ+bJTSurDCMzpnVRE5e9d79WP341YA28RNrF5llqgIY01LrZA/+FPM4j/NLJ9pCbTkUm8jCjNiZmP45lBhiGX+1dyKjmeAFO4HkB4aUO6h6JoJMS4ORcYQE67HGpEXDM7qpI7W4KrnwGNGSQBDGkw/p7efmzD8ix PKw5/cd5 CCxDZ9GFmGAuuZzeftgTgOIOrovJBrTiXYRKtDMtgahDccL0a/RUe1NpfxQHQQ+Gl9LCB+ImvmiowogPMfv9FNqmZYILXrTUxPTGeZC4eBxGcKd7h8KFHgWfk4Tg09BTOQq8OTWPAAIXdEQtPQRpVIultKSRNNH0SXouwU/kuzbFX93Q58GFkd5vGMFs89WVcducBx82grYzT1mkrYUmE6SRk3kz9n/6h424D X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Sat, Dec 30, 2023 at 10:19:49AM -0600, Michael Roth wrote: > From: Ashish Kalra > > Add a kdump safe version of sev_firmware_shutdown() registered as a > crash_kexec_post_notifier, which is invoked during panic/crash to do > SEV/SNP shutdown. This is required for transitioning all IOMMU pages > to reclaim/hypervisor state, otherwise re-init of IOMMU pages during > crashdump kernel boot fails and panics the crashdump kernel. This > panic notifier runs in atomic context, hence it ensures not to > acquire any locks/mutexes and polls for PSP command completion > instead of depending on PSP command completion interrupt. > > Signed-off-by: Ashish Kalra > [mdr: remove use of "we" in comments] > Signed-off-by: Michael Roth Cleanups ontop, see if the below works too. Especially: * I've zapped the WBINVD before the TMR pages are freed because __sev_snp_shutdown_locked() will WBINVD anyway. * The mutex_is_locked() check in snp_shutdown_on_panic() is silly because the panic notifier runs on one CPU anyway. Thx. --- diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h index 435ba9bc4510..27323203e593 100644 --- a/arch/x86/include/asm/sev.h +++ b/arch/x86/include/asm/sev.h @@ -227,6 +227,7 @@ int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct sn void snp_accept_memory(phys_addr_t start, phys_addr_t end); u64 snp_get_unsupported_features(u64 status); u64 sev_get_status(void); +void kdump_sev_callback(void); #else static inline void sev_es_ist_enter(struct pt_regs *regs) { } static inline void sev_es_ist_exit(void) { } @@ -255,6 +256,7 @@ static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *in static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { } static inline u64 snp_get_unsupported_features(u64 status) { return 0; } static inline u64 sev_get_status(void) { return 0; } +static inline void kdump_sev_callback(void) { } #endif #ifdef CONFIG_KVM_AMD_SEV diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c index 23ede774d31b..64ae3a1e5c30 100644 --- a/arch/x86/kernel/crash.c +++ b/arch/x86/kernel/crash.c @@ -40,6 +40,7 @@ #include #include #include +#include /* Used while preparing memory map entries for second kernel */ struct crash_memmap_data { @@ -59,12 +60,7 @@ static void kdump_nmi_callback(int cpu, struct pt_regs *regs) */ cpu_emergency_stop_pt(); - /* - * for SNP do wbinvd() on remote CPUs to - * safely do SNP_SHUTDOWN on the local CPU. - */ - if (cpu_feature_enabled(X86_FEATURE_SEV_SNP)) - wbinvd(); + kdump_sev_callback(); disable_local_APIC(); } diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c index c67285824e82..dbb2cc6b5666 100644 --- a/arch/x86/kernel/sev.c +++ b/arch/x86/kernel/sev.c @@ -2262,3 +2262,13 @@ static int __init snp_init_platform_device(void) return 0; } device_initcall(snp_init_platform_device); + +void kdump_sev_callback(void) +{ + /* + * Do wbinvd() on remote CPUs when SNP is enabled in order to + * safely do SNP_SHUTDOWN on the the local CPU. + */ + if (cpu_feature_enabled(X86_FEATURE_SEV_SNP)) + wbinvd(); +} diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c index 598878e760bc..c342e5e54e45 100644 --- a/drivers/crypto/ccp/sev-dev.c +++ b/drivers/crypto/ccp/sev-dev.c @@ -161,7 +161,6 @@ static int sev_wait_cmd_ioc(struct sev_device *sev, udelay(10); } - return -ETIMEDOUT; } @@ -1654,7 +1653,7 @@ static int sev_update_firmware(struct device *dev) return ret; } -static int __sev_snp_shutdown_locked(int *error, bool in_panic) +static int __sev_snp_shutdown_locked(int *error, bool panic) { struct sev_device *sev = psp_master->sev_data; struct sev_data_snp_shutdown_ex data; @@ -1673,7 +1672,7 @@ static int __sev_snp_shutdown_locked(int *error, bool in_panic) * In that case, a wbinvd() is done on remote CPUs via the NMI * callback, so only a local wbinvd() is needed here. */ - if (!in_panic) + if (!panic) wbinvd_on_all_cpus(); else wbinvd(); @@ -2199,26 +2198,13 @@ int sev_dev_init(struct psp_device *psp) return ret; } -static void __sev_firmware_shutdown(struct sev_device *sev, bool in_panic) +static void __sev_firmware_shutdown(struct sev_device *sev, bool panic) { int error; __sev_platform_shutdown_locked(NULL); if (sev_es_tmr) { - /* - * The TMR area was encrypted, flush it from the cache - * - * If invoked during panic handling, local interrupts are - * disabled and all CPUs are stopped, so wbinvd_on_all_cpus() - * can't be used. In that case, wbinvd() is done on remote CPUs - * via the NMI callback, so a local wbinvd() is sufficient here. - */ - if (!in_panic) - wbinvd_on_all_cpus(); - else - wbinvd(); - __snp_free_firmware_pages(virt_to_page(sev_es_tmr), get_order(sev_es_tmr_size), true); @@ -2237,7 +2223,7 @@ static void __sev_firmware_shutdown(struct sev_device *sev, bool in_panic) snp_range_list = NULL; } - __sev_snp_shutdown_locked(&error, in_panic); + __sev_snp_shutdown_locked(&error, panic); } static void sev_firmware_shutdown(struct sev_device *sev) @@ -2262,26 +2248,18 @@ void sev_dev_destroy(struct psp_device *psp) psp_clear_sev_irq_handler(psp); } -static int sev_snp_shutdown_on_panic(struct notifier_block *nb, - unsigned long reason, void *arg) +static int snp_shutdown_on_panic(struct notifier_block *nb, + unsigned long reason, void *arg) { struct sev_device *sev = psp_master->sev_data; - /* - * Panic callbacks are executed with all other CPUs stopped, - * so don't wait for sev_cmd_mutex to be released since it - * would block here forever. - */ - if (mutex_is_locked(&sev_cmd_mutex)) - return NOTIFY_DONE; - __sev_firmware_shutdown(sev, true); return NOTIFY_DONE; } -static struct notifier_block sev_snp_panic_notifier = { - .notifier_call = sev_snp_shutdown_on_panic, +static struct notifier_block snp_panic_notifier = { + .notifier_call = snp_shutdown_on_panic, }; int sev_issue_cmd_external_user(struct file *filep, unsigned int cmd, @@ -2322,7 +2300,7 @@ void sev_pci_init(void) "-SNP" : "", sev->api_major, sev->api_minor, sev->build); atomic_notifier_chain_register(&panic_notifier_list, - &sev_snp_panic_notifier); + &snp_panic_notifier); return; err: @@ -2339,5 +2317,5 @@ void sev_pci_exit(void) sev_firmware_shutdown(sev); atomic_notifier_chain_unregister(&panic_notifier_list, - &sev_snp_panic_notifier); + &snp_panic_notifier); } -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette