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 DF400C433FE for ; Tue, 22 Nov 2022 16:50:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1A27B6B0071; Tue, 22 Nov 2022 11:50:29 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 153088E0002; Tue, 22 Nov 2022 11:50:29 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F355D8E0001; Tue, 22 Nov 2022 11:50:28 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id E223C6B0071 for ; Tue, 22 Nov 2022 11:50:28 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id B352C14037B for ; Tue, 22 Nov 2022 16:50:28 +0000 (UTC) X-FDA: 80161666536.27.02C7D79 Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by imf18.hostedemail.com (Postfix) with ESMTP id 0E9841C0011 for ; Tue, 22 Nov 2022 16:50:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1669135827; x=1700671827; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=puqtoUPvntcrMHUyIfxv2XTasYKx66NRxAdTJ/YKsJo=; b=OKtHacnL/pgywVXc+J4Evy+evoI44S7A71qxqUkHSfNY1bUHdOmcW0sQ oI31fkQj85wuQgrJYZJJNZ0a+DOONI1JjaF+eC70K3VXowOpPxyZXABqF q0fuQdStMLMzg4RPY1fEKQ1ZjCduXyGVrNbks/R0QrIxEU614j5NFrXg9 CQb31+9HvMiBS3+Ef1JN/zu3H+eLsd3T00Gaz+hjtM/+n6hgvm1+0HUNA kXDD8EY1zuQKuPI38Ii+QsMFi4O/6sTShaOwHXEMbyd9GC/osrDJuuJQB J2fpI7E0kC3qegEEZecqf4h+ajNyDld9/Gj1TagCVGHE2I4qO/2pIjy9D Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10539"; a="293568822" X-IronPort-AV: E=Sophos;i="5.96,184,1665471600"; d="scan'208";a="293568822" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Nov 2022 08:50:25 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10539"; a="592201627" X-IronPort-AV: E=Sophos;i="5.96,184,1665471600"; d="scan'208";a="592201627" Received: from lcano-mobl1.amr.corp.intel.com (HELO [10.255.231.75]) ([10.255.231.75]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Nov 2022 08:50:23 -0800 Message-ID: Date: Tue, 22 Nov 2022 08:50:23 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH v7 02/20] x86/virt/tdx: Detect TDX during kernel boot Content-Language: en-US To: "Huang, Kai" , "kvm@vger.kernel.org" , "linux-kernel@vger.kernel.org" Cc: "Luck, Tony" , "bagasdotme@gmail.com" , "ak@linux.intel.com" , "Wysocki, Rafael J" , "kirill.shutemov@linux.intel.com" , "Christopherson,, Sean" , "Chatre, Reinette" , "pbonzini@redhat.com" , "linux-mm@kvack.org" , "Yamahata, Isaku" , "peterz@infradead.org" , "Shahar, Sagi" , "imammedo@redhat.com" , "Gao, Chao" , "Brown, Len" , "sathyanarayanan.kuppuswamy@linux.intel.com" , "Huang, Ying" , "Williams, Dan J" References: <9db9599fba11490cebbe59cbb7c145e9c119ab0f.camel@intel.com> From: Dave Hansen In-Reply-To: <9db9599fba11490cebbe59cbb7c145e9c119ab0f.camel@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669135827; a=rsa-sha256; cv=none; b=m+N2RHfyBqAThVvsF8PbuMtvK4sUBlP6TV5XKMBKL6DBe53CGYPdA6OBFC+gjCFPd3wId8 Bgs5DSLP7AcND43nv4jvKUov4Q6nJfp8jKEKDiirJf9aIDgC5eEhzJ6j8+ojFZ+HqheK47 xmz6t5ewA8HNSB51rU8KhDr5d6s68f8= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=none ("invalid DKIM record") header.d=intel.com header.s=Intel header.b=OKtHacnL; spf=pass (imf18.hostedemail.com: domain of dave.hansen@intel.com designates 192.55.52.136 as permitted sender) smtp.mailfrom=dave.hansen@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669135827; 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=QbyLFOyELCRX6RlPHKL0d2CxdJFAmOrLELS8Ayd5zpI=; b=C2B6myx3z1Xb8Hbx4vfwnJ/mGYivH85k9s+2HczBeVrj+el8FuekZtEnW6UnPxj43Qr8Hk 0bB3n8Gy6UbmW4jQt3Okx1z84N/lIdRAGmKIl60NmNSzajpbaI0GA3wd9xCz4IM3epTjTz LxJ8gq55rQd9vd/+jveM8lxK62/XcA4= X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 0E9841C0011 Authentication-Results: imf18.hostedemail.com; dkim=none ("invalid DKIM record") header.d=intel.com header.s=Intel header.b=OKtHacnL; spf=pass (imf18.hostedemail.com: domain of dave.hansen@intel.com designates 192.55.52.136 as permitted sender) smtp.mailfrom=dave.hansen@intel.com; dmarc=pass (policy=none) header.from=intel.com X-Rspam-User: X-Stat-Signature: df1pqrm3ktarowh7knnszazqdxunmo44 X-HE-Tag: 1669135826-532515 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 11/22/22 03:28, Huang, Kai wrote: >>> + /* >>> + * KeyID 0 is for TME. MKTME KeyIDs start from 1. TDX private >>> + * KeyIDs start after the last MKTME KeyID. >>> + */ >> >> Is the TME key a "MKTME KeyID"? > > I don't think so. Hardware handles TME KeyID 0 differently from non-0 MKTME > KeyIDs. And PCONFIG only accept non-0 KeyIDs. Let's say we have 4 MKTME hardware bits, we'd have: 0: TME Key 1->3: MKTME Keys 4->7: TDX Private Keys First, the MSR values: > + * IA32_MKTME_KEYID_PARTIONING: > + * Bit [31:0]: Number of MKTME KeyIDs. > + * Bit [63:32]: Number of TDX private KeyIDs. These would be: Bit [ 31:0] = 3 Bit [63:22] = 4 And in the end the variables: tdx_keyid_start would be 4 and tdx_keyid_num would be 4. Right? That's a bit wonky for my brain because I guess I know too much about the internal implementation and how the key space is split up. I guess I (wrongly) expected Bit[31:0]==Bit[63:22]. >>> +static void __init clear_tdx(void) >>> +{ >>> + tdx_keyid_start = tdx_keyid_num = 0; >>> +} >> >> This is where a comment is needed and can actually help. >> >> /* >> * tdx_keyid_start/num indicate that TDX is uninitialized. This >> * is used in TDX initialization error paths to take it from >> * initialized -> uninitialized. >> */ > > Just want to point out after removing the !x2apic_enabled() check, the only > thing need to do here is to detect/record the TDX KeyIDs. > > And the purpose of this TDX boot-time initialization code is to provide > platform_tdx_enabled() function so that kexec() can use. > > To distinguish boot-time TDX initialization from runtime TDX module > initialization, how about change the comment to below? > > static void __init clear_tdx(void) > { > /* > * tdx_keyid_start and nr_tdx_keyids indicate that TDX is not > * enabled by the BIOS. This is used in TDX boot-time > * initializatiton error paths to take it from enabled to not > * enabled. > */ > tdx_keyid_start = nr_tdx_keyids = 0; > } > > [...] I honestly have no idea what "boot-time TDX initialization" is versus "runtime TDX module initialization". This doesn't hel. > And below is the updated patch. How does it look to you? Let's see... ... > +static u32 tdx_keyid_start __ro_after_init; > +static u32 nr_tdx_keyids __ro_after_init; > + > +static int __init record_keyid_partitioning(void) > +{ > + u32 nr_mktme_keyids; > + int ret; > + > + /* > + * IA32_MKTME_KEYID_PARTIONING: > + * Bit [31:0]: Number of MKTME KeyIDs. > + * Bit [63:32]: Number of TDX private KeyIDs. > + */ > + ret = rdmsr_safe(MSR_IA32_MKTME_KEYID_PARTITIONING, &nr_mktme_keyids, > + &nr_tdx_keyids); > + if (ret) > + return -ENODEV; > + > + if (!nr_tdx_keyids) > + return -ENODEV; > + > + /* TDX KeyIDs start after the last MKTME KeyID. */ > + tdx_keyid_start++; tdx_keyid_start is uniniitalized here. So, it'd be 0, then ++'d. Kai, please take a moment and slow down. This isn't a race. I offered some replacement code here, which you've discarded, missed or ignored and in the process broken this code. This approach just wastes reviewer time. It's not working for me. I'm going to make a suggestion (aka. a demand): You can post these patches at most once a week. You get a whole week to (carefully) incorporate reviewer feedback, make the patch better, and post a new version. Need more time? Go ahead and take it. Take as much time as you want.