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 849A2C4332F for ; Tue, 12 Dec 2023 06:53:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 10E6F6B0289; Tue, 12 Dec 2023 01:53:58 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 096956B028B; Tue, 12 Dec 2023 01:53:58 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E51CA6B028C; Tue, 12 Dec 2023 01:53:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id CD2146B0289 for ; Tue, 12 Dec 2023 01:53:57 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 90C17120578 for ; Tue, 12 Dec 2023 06:53:57 +0000 (UTC) X-FDA: 81557251314.12.D95ACCB Received: from mail.alien8.de (mail.alien8.de [65.109.113.108]) by imf27.hostedemail.com (Postfix) with ESMTP id 522AD40002 for ; Tue, 12 Dec 2023 06:53:55 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=alien8.de header.s=alien8 header.b=h9cZgdMO; dmarc=pass (policy=none) header.from=alien8.de; spf=pass (imf27.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=1702364035; 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=ew/p29RsERxODejVJK/prbLDG9ex/vpDFzw9MN3tsq4=; b=CzynOqRIKVKeEEc0aIDcX+16Jn0TsTeeZ9mK1rNZ53TGtD8ZVQ7noQXXv1Ss/SEKbe3uX0 m4KIcn0dsCaX0rMU9thMc9BmIQZSwJYBFH0+Uf3Q6n1//Ku0l/8o8OWKMXRXK1dQM3fgR1 k7lEe1paPNcyedKNMnVc4E5nK5Ew1Ow= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=alien8.de header.s=alien8 header.b=h9cZgdMO; dmarc=pass (policy=none) header.from=alien8.de; spf=pass (imf27.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=1702364035; a=rsa-sha256; cv=none; b=fXNl5QRy6nw+rjMpEpD5XOAvozYKPA9gkWra4WTtXYUviEScNV4J32Ztbjg8DI2jMfx0RZ EFIiMt9c3DTTfb4YdizPPGxhe6ecPJ8+huoGpIn0lx+lCaJWw8pPWI/weicdglGUNK9F3Z rDAZPewj4CH7u6TjVUAg/tiVK6Mu+1A= Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.alien8.de (SuperMail on ZX Spectrum 128k) with ESMTP id 3C0CD40E00CC; Tue, 12 Dec 2023 06:53:51 +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 pDdtV7Aw2AiP; Tue, 12 Dec 2023 06:53:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=alien8; t=1702364029; bh=ew/p29RsERxODejVJK/prbLDG9ex/vpDFzw9MN3tsq4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=h9cZgdMOi4AMc5jH32UrN+EA/9cBM1Asn0f/Pr0gfyds7BT+/0Nl8WWbmiFGhFvnL feiBK24dWx5QmKrugrqG+BSkAIjkeoP1DcOAlr/M2EibJQCDDswuey9R+RGJ5GOBlJ Ke4JiNNqMyPNHcXzaLYMVUsplURYCQ0RwHHdrmahz3AGMay87J/8OPpj7rk4tSpVWx bfie+iEo7Ys5+Ckq33dtr0+wXW99FMSLL8DhoG+btVZdKGytgGHbarIRjBV+p3juCO emoCj3qel7qPCyEugklZ4H9n3HSY76Y0c1YrwTA3+61y+5bPZGqJFN8IVUs8TXIlH1 15YdOLX5pkgvpOoNGJYKIAhEUmxR/QcsRAbrkbmRTt1A7pFTDLyVLHfzguoLIUpanQ U3jCxGi+/3Wop99zzzymNsV1Y++VkPDUgL31pov6Glf5dVUVjgnaylMQwYtGrHWaTU OZNdBr5Y1qV/GiAuBzSZziTAdc7wTsIHmUOOgg3HRTmDcj3tJnVsieftsrIMOPgLbU cPDLPohB32GLlZcSa0sbWJAb/GhiU51Vl6XWnqftK7X4tvj2EqvSTFcD8DXHQDDIqT Z1KwgUMBNnz0+NuhUZwK0k2PDwNSOxfYqwReoBEx4psdy3ftP8v+81+eCYHbtHFkCs WcXNL786gMn32z/o0RXvSjyI= Received: from zn.tnic (pd95304da.dip0.t-ipconnect.de [217.83.4.218]) (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 897BA40E00CB; Tue, 12 Dec 2023 06:53:08 +0000 (UTC) Date: Tue, 12 Dec 2023 07:52:59 +0100 From: Borislav Petkov To: "Kalra, Ashish" Cc: Michael Roth , kvm@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, linux-crypto@vger.kernel.org, x86@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, dovmurik@linux.ibm.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, nikunj.dadhania@amd.com, pankaj.gupta@amd.com, liam.merwick@oracle.com, zhi.a.wang@intel.com, Brijesh Singh , Jarkko Sakkinen Subject: Re: [PATCH v10 14/50] crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP Message-ID: <20231212065259.GAZXgDSwV+zaH8MvB6@fat_crate.local> References: <20231016132819.1002933-1-michael.roth@amd.com> <20231016132819.1002933-15-michael.roth@amd.com> <20231127095937.GLZWRoiaqGlJMX54Xb@fat_crate.local> <20231206170807.GBZXCqd0z8uu2rlhpn@fat_crate.local> <9af9b10f-0ab6-1fe8-eaec-c9f98e14a203@amd.com> <20231209162015.GBZXSTv738J09Htf51@fat_crate.local> <2800d4d6-8ae9-e6c9-287a-301beb0a2f50@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <2800d4d6-8ae9-e6c9-287a-301beb0a2f50@amd.com> X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 522AD40002 X-Stat-Signature: eqs8iwuga61wceif9ntwzh7fb3utkjds X-HE-Tag: 1702364035-854234 X-HE-Meta: U2FsdGVkX1/QhEZfpRl0a4p+GYKZfje3G4BCV9EJB7P8fByKDipxx3UwPIumF4tSw1LVYoXt2A/Fo95L2+PqmNw3tCYsNJwKglMNy0mp5CW/JSdZ7jFhmZ1d+XLO5exqLiRzJJUaPRuNVI4+QdF7tN9JqfoB/ei0gerrHtNzmgI07eg+BjQ1gBWFw57YwFcdvOrLieJf560aEuXv1j4qMAuy03IFYmXTiVB5lsIVuRXRx7481526yU1nHUcw+BmsBHBQTRqHPgTOYhVawrURWn60NWXwQ4pqw/xbGvflgiofmM4axvLBGoD5rBB+2pkXiisH0eJewX0nutWP+QRh7vESm5WYc2V1cqAF79Zcg2Eu8klGWPEO+JQ4gq79RR0Hs5ay8SSJL3GA/4k56RjPYIrsWkfzoAUGAtkJAChrV3sdJyZQVtTeZPW27JhDXhsHy7xIYkOitzXo6n/K8n4jpU5iEt3dySj1JTGECNvTzquhXSGXfGN8S8ReX52UGc7gYfVIzMQVB2hTlMYT1UvDmKc4IjHdCHrq12pmIok4dcXTRu8RfhShlO/pPC54c2Jf6ro/TGIWTmdvfn5gmpHYhfQ7Om9xwXRDzC/W53QItmTSj5qGkjTzsCnxXZS/oUd169w6mpTos96hjhuCqt+W5BTYSVRiGAAdExA4bH4xyy1pHBpnQwg1MjXMoUYyoG5qrrvKNGXhoILvH2QxsxmvVvOXiU131zjGb8uFRLlC8xTBm8AUWWA6dbeB2vW/+tSPk5/4criqvPQFjZEQJIWaMrWMvTQK7xoVgHSCpAHC5feFzQOxROUTIwDhJxQxbhbdOc/0mGIL+QSLLH3U8v64/RsijpFi4VyXDmnhaiTy94ZpaCag714Goig6u5HkPXRdMV90ugn9Rewv2ECb/rA/CKwSyQxyhbkUhyRYcgW6zBpoWmo8FinpUyKc1Hp0SYIYsuuwdvVqC0Exj0Mii8U bmcKrd71 QfZDpZQCSjbplbQkl2P5bW7mp8GHMTLqejN96o8bs+Ynsd/P6PDVuymEm9Imk4KsGs7s5p0U9YrOhtqJ/l6fdt+yXBPVGSCCQf4SP/0nS0UyiWs9ooNi6T9wo5HL+ihN0Fgx7i4WqGWK0W7wKBSpS2ojrqQ47+dw70I30e3EOV3NaGAIRywy6/3VqoPLrUpdfoQrf8dUUPvocTSA= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000619, 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 Mon, Dec 11, 2023 at 03:11:17PM -0600, Kalra, Ashish wrote: > What we need is a mechanism to do legacy SEV/SEV-ES INIT only if a > SEV/SEV-ES guest is being launched, hence, we want an additional parameter > added to sev_platform_init() exported interface so that kvm_amd module can > call this interface during guest launch and indicate if SNP/legacy guest is > being launched. > > That's the reason we want to add the probe parameter to > sev_platform_init(). That's not what your original patch does and nowhere in the whole patchset do I see this new requirement for KVM to be able to control the probing. The probe param is added to ___sev_platform_init_locked() which is called by this new sev_platform_init_on_probe() thing to signal that whatever calls this, it wants the probing. And "whatever" is sev_pci_init() which is called from the bowels of the secure processor drivers. Suffice it to say, this is some sort of an init path. So, it wants to init SNP stuff which is unconditional during driver init - not when KVM starts guests - and probe too on driver init time, *iff* that psp_init_on_probe thing is set. Which looks suspicious to me: "Add psp_init_on_probe module parameter that allows for skipping the PSP's SEV platform initialization during module init. User may decouple module init from PSP init due to use of the INIT_EX support in upcoming patch which allows for users to save PSP's internal state to file." >From b64fa5fc9f44 ("crypto: ccp - Add psp_init_on_probe module parameter"). And reading about INIT_EX, "This command loads the SEV related persistent data from user-supplied data and initializes the platform context." So it sounds like HV vendor wants to supply something itself. But then looking at init_ex_path and open_file_as_root() makes me cringe. I would've never done it this way: we have request_firmware* etc helpers for loading blobs from userspace which are widely used. But then reading 3d725965f836 ("crypto: ccp - Add SEV_INIT_EX support") that increases the cringe factor even more because that also wants to *write* into that file. Maybe there were good reasons to do it this way - it is still yucky for my taste tho... But I digress - whatever you want to do, the right approach is to split the functionality: SNP init legacy SEV init and to call them from a wrapper function around it which determines which ones need to get called depending on that delayed probe thing. Lumping everything together and handing a silly bool downwards is already turning into a mess. Now, looking at sev_guest_init() which calls sev_platform_init() and if you want to pass back'n'forth more information than just that &error pointer, then you can define your own struct sev_platform_init_info or so which you preset before calling sev_platform_init() and pass in a pointer to it. And in it you can stick &error, bool probe or whatever else you need to control what the platform needs to do upon init. And if you need to extend that in the future, you can add new struct members and so on. HTH. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette