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 DF457C61DA3 for ; Tue, 21 Feb 2023 09:28:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3910A6B0071; Tue, 21 Feb 2023 04:28:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 341F46B0072; Tue, 21 Feb 2023 04:28:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1E1616B0073; Tue, 21 Feb 2023 04:28:30 -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 0DFF36B0071 for ; Tue, 21 Feb 2023 04:28:30 -0500 (EST) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id CF5B581130 for ; Tue, 21 Feb 2023 09:28:29 +0000 (UTC) X-FDA: 80490773538.21.AB5583C Received: from mail-lj1-f181.google.com (mail-lj1-f181.google.com [209.85.208.181]) by imf29.hostedemail.com (Postfix) with ESMTP id C22EA120019 for ; Tue, 21 Feb 2023 09:28:27 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="h/MKpwsw"; spf=pass (imf29.hostedemail.com: domain of zhi.wang.linux@gmail.com designates 209.85.208.181 as permitted sender) smtp.mailfrom=zhi.wang.linux@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1676971707; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=7cMvEWx93T+DKuH/EfNzVMzxjU3ZfkiKTGej0rtS7XU=; b=gT5SZOu2l8zWAJd3QtGvq2d/k+iAfuPwLKUtYVZsZo493hzM1lhnlfCJFQl5rVsNgkbze6 Krc1U0v0icVl4pynB0g9iyFPdemnZJM8kDYN2sTdkafcWUIWdCu8Qjh9UiiPwv++5M+zqL O5O6U8AuJVG50owmR6WNrJRhnoQe3nQ= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="h/MKpwsw"; spf=pass (imf29.hostedemail.com: domain of zhi.wang.linux@gmail.com designates 209.85.208.181 as permitted sender) smtp.mailfrom=zhi.wang.linux@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1676971707; a=rsa-sha256; cv=none; b=ej/EOev6XloEhGsNWM22h7svzSL9F8Wahkpb1fef6dkeLpDQCHHirp7PlQt8iS6eptPzyy mOZ+dBw34NnkRQ82JRqOWGuh4xqxB/xRy4iQSiQwE9E9D98CNiaVXwZJg6BBXvqqBcCiTZ BB9/s78HZ5TifXdZU7LQr3FDTCqUMEE= Received: by mail-lj1-f181.google.com with SMTP id j17so3808925ljq.11 for ; Tue, 21 Feb 2023 01:28:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=7cMvEWx93T+DKuH/EfNzVMzxjU3ZfkiKTGej0rtS7XU=; b=h/MKpwswKXFMPyFd7oBDdGwT1y9EKS9WAjFXU+U9BdYQN554vMrvq7yaIJ2bISdl5q 68rK/XqSK0ZTzubookOAKlQiRkh/Qb+Y8NhBvjuVdNePw8caYBX8chcYUBHO+Txivp94 styxcRkySeYlHXAer1Fcb1AVJGRFD8XAwBVbdMhmVttT+M1PksmkX6OnEr+7/MJxcciD sdGiFEPtgOoRpI9UQnVRKTdt7WL8TCV/H0zeJTdw3HvdESLUNxvHPOInJrwl6qHi9tcH Ks92c0JaxDH8N94zNzsfA/+AuA3X+OhIFaa51UecB9hENAWk21HAG3e29iK6ok7jWNio Fi+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=7cMvEWx93T+DKuH/EfNzVMzxjU3ZfkiKTGej0rtS7XU=; b=qXMkASQftkeQMBg23pwMjl7ByJbOFJkcBKo3XojZDjQrGu8Xg7L7fj6MAwPrprTzz5 fzHIy1MyPWuZ8RAa0E1P08DrGZPEEI2xyZ+810V5UrwFvGkHXtbA60AHmfaYZKgD9tW/ wYT5OVlydtDdw0gym91gEHHYADYTzyT0MiubmlPTTSNJE7z+D46RpoQbU4YXmruGB/S4 b9BqIXNjobGCn7FLHTagSQ3y2eLwqrh0x6jaaqRI6p65+0r3+vTA9nTBDyFDPzBhcoHJ 6Huv8e6MiVqPB/c8zTauZVuhlOscNGndJCF8Cs+ibEvNqMiZqlgrHk7Jts54UwlB6P/g vjtg== X-Gm-Message-State: AO0yUKXh/QgnnXl87WB2y2pCt/sbqjayTf60uJT17Oohi/7/oaLaRvZs 8mx7eFu1iVxuQ55cKAfQaKo= X-Google-Smtp-Source: AK7set84dqJj7wcs9uzj2n8kT//QPqBDPmWF+AwkpmwrrrE3ZAh41j4XgidURaSXeFVF8sErCcmqqw== X-Received: by 2002:a05:651c:897:b0:295:90b2:da6b with SMTP id d23-20020a05651c089700b0029590b2da6bmr206314ljq.0.1676971705705; Tue, 21 Feb 2023 01:28:25 -0800 (PST) Received: from localhost (88-115-161-74.elisa-laajakaista.fi. [88.115.161.74]) by smtp.gmail.com with ESMTPSA id o16-20020a2e9b50000000b002958bb2deacsm123022ljj.46.2023.02.21.01.28.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Feb 2023 01:28:25 -0800 (PST) Date: Tue, 21 Feb 2023 11:28:23 +0200 From: Zhi Wang To: Michael Roth Cc: , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , Brijesh Singh Subject: Re: [PATCH RFC v8 24/56] crypto: ccp: Handle the legacy TMR allocation when SNP is enabled Message-ID: <20230221112823.000063e4@gmail.com> In-Reply-To: <20230220183847.59159-25-michael.roth@amd.com> References: <20230220183847.59159-1-michael.roth@amd.com> <20230220183847.59159-25-michael.roth@amd.com> X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: C22EA120019 X-Rspam-User: X-Stat-Signature: xfyryrogq76usi479ftg5pdrctm5sm4f X-HE-Tag: 1676971707-179219 X-HE-Meta: U2FsdGVkX18gemx5khqgVhhPdyBVRxZ45JUBIiAOn2uw1bOo57qJfl6yZuoxqVtIsxjby+o+JrYq7kqJ2b76kmU3k5aAHOwvJNDJ8u+aIGgou4g0iiFV4Ry/EXXH9FQG0y4CFOswhwE+KhWQDFsD8YxQiq9eTTrQ+g7N9wy7rmqDUX3PuyGT/5lJi5l4V52nwbRim4mW+tuNQ1JJhKto0PA8MehhbWPQfJNVVErIYCL1IrgF7hvlNvoO8FwmM/0gYct288TKTT0321TETTMcMQB3JFfZf1BcQOZXmQEInc9cB455DBf1RNEpcdjgutxaP78xijt4Z/7RCUu9pndcp2BEE15gdq2ZggMCWOxx7QghiJlhAigfQ4aIS+fVFzEoba29UX1s927KA7SskQLukhrHd1kxgPkvaA8U7epcvIXit1x/UBuNru0GBbIeWSuwPI8AYXrbsFNtK56JDdwwU+E9deb9spAkxjCcGtnteK3WKE8YNbYkadOlPCn8Jgi+Ew3+DGS2e6y05I3M8UDkEBzkc4HT7MQQf7PGF1XaGtedEzgfj3olfTdeSUmt3Fkv0oPYx5wLZoxKhzmLEKbubTf4s+qy3r0Ztsjec35QpuQ59eP2uQi7FAEX6tQISA+AY4XO5hdHbwH6X1F+g0Zv6OkdH4C2MO2MkvO1XfMNR9+0QtRXuT5KMUre3NdolSW2P+1b0YrPuabw0K371wgpBzXJZuPkTAJjHOU2qj6Xnhg1zqAOQJ4K/hSYBXKDwQKfbFlq60TC3+NH1FMKjHmXmy5RZfi6DAMDPJQIxo2GJ7mRWDJ6DpM70gZFmdx3irLOdvQ0KkWzgFsa+9Dr3vUI7O6g0Wyg9eLtpynGe8xdFiWSdrat7yWajIiuIAztyujcr+vhBh5WvnYjZ9o2SiuwjnBmzBk06kfhsJGAABx872PFEZhZpV4G25J6c94t5L+Nd4qewLRprVZF2z8+U+m PoI4tlwM 3cLfAqR/NySsqSa27OKD7QV4MbSaiufgQt8gm1vSsS6N/eVsj2Zqan0dc5GlxkxAJHOYTWh9t0tQtZncDx2TvrtrKTZKuS6lLm/VVNWoiK//CaY6oswJS8q/KHXEPGXQi41Ot8sRjEyAZNSKr+MteZVEcmCpDUYoh4f3vFPTnNIi4DSCE/jjxhdh3FYHH1yuUfgbfJgdBT3VSak5tZa1hHsIuCp1NYJsN4Wo1Hjx+YWw8RGU4/Sg9BFIk2IC1RNzmMtnmWJveZjPCuWoYNkYZG6A4iEG0kSC9ui6YvR7iRjyrpRXu8s35PKoKqBGjPVpW29ZYv8nf5N4hCO58P84m9B26jcSha2t5naP/SOuGReUbkYPzH8CZQTnI12Bg3rR6Wil6sSKmqiE+Q6B3iU7z+qacmLFpEQlQtpVayqLTTULeDO+wFlTaPgUTlZk9QFquuYULe0O43rHURTJvoqu100k3OI5081uTcLBN9dNk5D9z/GH82HN2jbFquJVGl3uKRN1X 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: On Mon, 20 Feb 2023 12:38:15 -0600 Michael Roth wrote: > From: Brijesh Singh > > The behavior and requirement for the SEV-legacy command is altered when > the SNP firmware is in the INIT state. See SEV-SNP firmware specification > for more details. > > Allocate the Trusted Memory Region (TMR) as a 2mb sized/aligned region > when SNP is enabled to satisfy new requirements for the SNP. Continue > allocating a 1mb region for !SNP configuration. > > While at it, provide API that can be used by others to allocate a page > that can be used by the firmware. The immediate user for this API will > be the KVM driver. The KVM driver to need to allocate a firmware context > page during the guest creation. The context page need to be updated > by the firmware. See the SEV-SNP specification for further details. > > Co-developed-by: Ashish Kalra > Signed-off-by: Ashish Kalra > Signed-off-by: Brijesh Singh > Signed-off-by: Michael Roth > --- > drivers/crypto/ccp/sev-dev.c | 148 +++++++++++++++++++++++++++++++++-- > include/linux/psp-sev.h | 9 +++ > 2 files changed, 149 insertions(+), 8 deletions(-) > > diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c > index eca4e59b0f44..4c12e98a1219 100644 > --- a/drivers/crypto/ccp/sev-dev.c > +++ b/drivers/crypto/ccp/sev-dev.c > @@ -94,6 +94,13 @@ static void *sev_init_ex_buffer; > */ > struct sev_data_range_list *snp_range_list; > > +/* When SEV-SNP is enabled the TMR needs to be 2MB aligned and 2MB size. */ > +#define SEV_SNP_ES_TMR_SIZE (2 * 1024 * 1024) It would be better to re-use the kernel size definition macros. E.g. SZ_2MB. > + > +static size_t sev_es_tmr_size = SEV_ES_TMR_SIZE; > + > +static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret); > + > static inline bool sev_version_greater_or_equal(u8 maj, u8 min) > { > struct sev_device *sev = psp_master->sev_data; > @@ -216,11 +223,134 @@ void snp_mark_pages_offline(unsigned long pfn, unsigned int npages) > } > EXPORT_SYMBOL_GPL(snp_mark_pages_offline); > > +static int snp_reclaim_pages(unsigned long paddr, unsigned int npages, bool locked) > +{ > + /* Cbit maybe set in the paddr */ This is confusing. I suppose C-bit is treated as a attribute of PTE in the kernel not part of the PA. It means only a PTE might carry a C-bit. The paddr is from __pa(page_address()). It is not extracted from a PTE. Thus, the return from them should never have a C-bit. BTW: Wouldn't it be better to have pfn as input param instead of paddr? The caller has struct page, calling snp_reclaim_pages(page_to_pfn(page), xxxxx) would be much clearer than the current conversion: page_address() (struct page is converted to VA), __pa() (VA is converted to PA) in the caller and then PA is converted to pfn here. > + unsigned long pfn = __sme_clr(paddr) >> PAGE_SHIFT; > + int ret, err, i, n = 0; > + should be unsigned int i, n; as the input param npage is unsigned int. > + if (!pfn_valid(pfn)) { > + pr_err("%s: Invalid PFN %lx\n", __func__, pfn); > + return 0; > + } > + > + for (i = 0; i < npages; i++, pfn++, n++) { > + paddr = pfn << PAGE_SHIFT; > + > + if (locked) > + ret = __sev_do_cmd_locked(SEV_CMD_SNP_PAGE_RECLAIM, &paddr, &err); > + else > + ret = sev_do_cmd(SEV_CMD_SNP_PAGE_RECLAIM, &paddr, &err); > + > + if (ret) > + goto cleanup; > + > + ret = rmp_make_shared(pfn, PG_LEVEL_4K); > + if (ret) > + goto cleanup; > + } > + > + return 0; > + > +cleanup: > + /* > + * If failed to reclaim the page then page is no longer safe to > + * be release back to the system, leak it. > + */ > + snp_mark_pages_offline(pfn, npages - n); > + return ret; > +} > + > +static int rmp_mark_pages_firmware(unsigned long paddr, unsigned int npages, bool locked) The same comment as above. Better take pfn or page instead of paddr with redundant conversions. > +{ > + /* Cbit maybe set in the paddr */ > + unsigned long pfn = __sme_clr(paddr) >> PAGE_SHIFT; > + int rc, n = 0, i; > + > + for (i = 0; i < npages; i++, n++, pfn++) { > + rc = rmp_make_private(pfn, 0, PG_LEVEL_4K, 0, true); > + if (rc) > + goto cleanup; > + } > + > + return 0; > + > +cleanup: > + /* > + * Try unrolling the firmware state changes by > + * reclaiming the pages which were already changed to the > + * firmware state. > + */ > + snp_reclaim_pages(paddr, n, locked); > + > + return rc; > +} > + > +static struct page *__snp_alloc_firmware_pages(gfp_t gfp_mask, int order, bool locked) > +{ > + unsigned long npages = 1ul << order, paddr; > + struct sev_device *sev; > + struct page *page; > + > + if (!psp_master || !psp_master->sev_data) > + return NULL; > + > + page = alloc_pages(gfp_mask, order); > + if (!page) > + return NULL; > + > + /* If SEV-SNP is initialized then add the page in RMP table. */ > + sev = psp_master->sev_data; > + if (!sev->snp_initialized) > + return page; > + > + paddr = __pa((unsigned long)page_address(page)); > + if (rmp_mark_pages_firmware(paddr, npages, locked)) > + return NULL; > + > + return page; > +} > + > +void *snp_alloc_firmware_page(gfp_t gfp_mask) > +{ > + struct page *page; > + > + page = __snp_alloc_firmware_pages(gfp_mask, 0, false); > + > + return page ? page_address(page) : NULL; > +} > +EXPORT_SYMBOL_GPL(snp_alloc_firmware_page); > + > +static void __snp_free_firmware_pages(struct page *page, int order, bool locked) > +{ > + struct sev_device *sev = psp_master->sev_data; > + unsigned long paddr, npages = 1ul << order; > + > + if (!page) > + return; > + > + paddr = __pa((unsigned long)page_address(page)); > + if (sev->snp_initialized && > + snp_reclaim_pages(paddr, npages, locked)) > + return; > + > + __free_pages(page, order); > +} > + > +void snp_free_firmware_page(void *addr) > +{ > + if (!addr) > + return; > + > + __snp_free_firmware_pages(virt_to_page(addr), 0, false); > +} > +EXPORT_SYMBOL_GPL(snp_free_firmware_page); > + > static void *sev_fw_alloc(unsigned long len) > { > struct page *page; > > - page = alloc_pages(GFP_KERNEL, get_order(len)); > + page = __snp_alloc_firmware_pages(GFP_KERNEL, get_order(len), false); > if (!page) > return NULL; > > @@ -468,7 +598,7 @@ static int __sev_init_locked(int *error) > data.tmr_address = __pa(sev_es_tmr); > > data.flags |= SEV_INIT_FLAGS_SEV_ES; > - data.tmr_len = SEV_ES_TMR_SIZE; > + data.tmr_len = sev_es_tmr_size; > } > > return __sev_do_cmd_locked(SEV_CMD_INIT, &data, error); > @@ -491,7 +621,7 @@ static int __sev_init_ex_locked(int *error) > data.tmr_address = __pa(sev_es_tmr); > > data.flags |= SEV_INIT_FLAGS_SEV_ES; > - data.tmr_len = SEV_ES_TMR_SIZE; > + data.tmr_len = sev_es_tmr_size; > } > > return __sev_do_cmd_locked(SEV_CMD_INIT_EX, &data, error); > @@ -982,6 +1112,8 @@ static int __sev_snp_init_locked(int *error) > sev->snp_initialized = true; > dev_dbg(sev->dev, "SEV-SNP firmware initialized\n"); > > + sev_es_tmr_size = SEV_SNP_ES_TMR_SIZE; > + > return rc; > } > > @@ -1499,8 +1631,9 @@ static void sev_firmware_shutdown(struct sev_device *sev) > /* The TMR area was encrypted, flush it from the cache */ > wbinvd_on_all_cpus(); > > - free_pages((unsigned long)sev_es_tmr, > - get_order(SEV_ES_TMR_SIZE)); > + __snp_free_firmware_pages(virt_to_page(sev_es_tmr), > + get_order(sev_es_tmr_size), > + false); > sev_es_tmr = NULL; > } > > @@ -1511,8 +1644,7 @@ static void sev_firmware_shutdown(struct sev_device *sev) > } > > if (snp_range_list) { > - free_pages((unsigned long)snp_range_list, > - get_order(PAGE_SIZE)); > + snp_free_firmware_page(snp_range_list); > snp_range_list = NULL; > } > > @@ -1593,7 +1725,7 @@ void sev_pci_init(void) > } > > /* Obtain the TMR memory area for SEV-ES use */ > - sev_es_tmr = sev_fw_alloc(SEV_ES_TMR_SIZE); > + sev_es_tmr = sev_fw_alloc(sev_es_tmr_size); > if (!sev_es_tmr) > dev_warn(sev->dev, > "SEV: TMR allocation failed, SEV-ES support unavailable\n"); > diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h > index 8edf5c548fbf..d19744807471 100644 > --- a/include/linux/psp-sev.h > +++ b/include/linux/psp-sev.h > @@ -922,6 +922,8 @@ int sev_guest_decommission(struct sev_data_decommission *data, int *error); > int sev_do_cmd(int cmd, void *data, int *psp_ret); > > void *psp_copy_user_blob(u64 uaddr, u32 len); > +void *snp_alloc_firmware_page(gfp_t mask); > +void snp_free_firmware_page(void *addr); > > /** > * sev_mark_pages_offline - insert non-reclaimed firmware/guest pages > @@ -959,6 +961,13 @@ static inline void *psp_copy_user_blob(u64 __user uaddr, u32 len) { return ERR_P > > void snp_mark_pages_offline(unsigned long pfn, unsigned int npages) {} > > +static inline void *snp_alloc_firmware_page(gfp_t mask) > +{ > + return NULL; > +} > + > +static inline void snp_free_firmware_page(void *addr) { } > + > #endif /* CONFIG_CRYPTO_DEV_SP_PSP */ > > #endif /* __PSP_SEV_H__ */