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 CC322C4332F for ; Tue, 22 Nov 2022 19:14:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 40A586B0073; Tue, 22 Nov 2022 14:14:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3B9B76B0074; Tue, 22 Nov 2022 14:14:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2A9626B0075; Tue, 22 Nov 2022 14:14:19 -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 1B5616B0073 for ; Tue, 22 Nov 2022 14:14:19 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id E1A9BAB118 for ; Tue, 22 Nov 2022 19:14:18 +0000 (UTC) X-FDA: 80162028996.22.23C8C90 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by imf06.hostedemail.com (Postfix) with ESMTP id A45D4180007 for ; Tue, 22 Nov 2022 19:14:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1669144457; x=1700680457; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=se+OD9QShnHz7aQaFNLHMirO/kbdEBdvxBYXfCZOqXk=; b=DdudftnEi/+o5aEIXm4zrSjisUGLWV0B3MDj9kINhaFyo0meLjPi1yDv C0qh0l2/Q4zb01hzj7jgg122aIGvHN60UHu/a0J+Zirn0HT/PMHRxH2U7 04yUhE0MBX/eLNeHfzsAh7JrBR5d6oNerBURlXsfX1RLWUMTNAkjzOCRn v03zFK0tSJLv9Zj+mbK0sIaDtMj3lLqJOO+Iv1MDFjsWSwCpI+1ZZB4K3 39thwlwHQapJaROE/MAAuZRNWXH6C4bjaqwGHGH+l/XvzeX4oFUnoXLBG dBfnupPJ33wXajmgKj1t/j6eP1HlacAOkJlKqAxNleTHj6gCDImUag1zz g==; X-IronPort-AV: E=McAfee;i="6500,9779,10539"; a="297251092" X-IronPort-AV: E=Sophos;i="5.96,184,1665471600"; d="scan'208";a="297251092" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Nov 2022 11:14:15 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10539"; a="766439301" X-IronPort-AV: E=Sophos;i="5.96,184,1665471600"; d="scan'208";a="766439301" Received: from coltsavx-mobl1.amr.corp.intel.com (HELO [10.255.0.114]) ([10.255.0.114]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Nov 2022 11:14:13 -0800 Message-ID: <2cbd2fe3-91ea-49e6-d684-b1fab012b44e@intel.com> Date: Tue, 22 Nov 2022 11:14:11 -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 07/20] x86/virt/tdx: Do TDX module global initialization Content-Language: en-US To: Kai Huang , linux-kernel@vger.kernel.org, kvm@vger.kernel.org Cc: linux-mm@kvack.org, seanjc@google.com, pbonzini@redhat.com, dan.j.williams@intel.com, rafael.j.wysocki@intel.com, kirill.shutemov@linux.intel.com, ying.huang@intel.com, reinette.chatre@intel.com, len.brown@intel.com, tony.luck@intel.com, peterz@infradead.org, ak@linux.intel.com, isaku.yamahata@intel.com, chao.gao@intel.com, sathyanarayanan.kuppuswamy@linux.intel.com, bagasdotme@gmail.com, sagis@google.com, imammedo@redhat.com References: <40824ec3e3dc759705dcfa1cb2929d18c12b417a.1668988357.git.kai.huang@intel.com> From: Dave Hansen In-Reply-To: <40824ec3e3dc759705dcfa1cb2929d18c12b417a.1668988357.git.kai.huang@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=none ("invalid DKIM record") header.d=intel.com header.s=Intel header.b=DdudftnE; spf=pass (imf06.hostedemail.com: domain of dave.hansen@intel.com designates 134.134.136.126 as permitted sender) smtp.mailfrom=dave.hansen@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669144458; a=rsa-sha256; cv=none; b=sInmU5lhFXgPQ5EIxcpkWsGsrDO0aY/89YvG8nNJR5KFEqoMeor71JxLT29E/PGmSpiUav SARs4EqVxrga+7DLCOx94977/ZFUT1PT89X2UO1ZPORsp/M1HLU06bnGhhzjAZ+HEIqL0E kjtXBqfD591FbZWOkXSYZ4Izn7uzlLU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669144458; 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=3qk03En4rOvf/uc+aeh7ReUp0z9Q7elcknhuexvmRqE=; b=drVUSAou7AZd1CqmDKS9GyeB9JUbUk0RCJVqEnrAx3It3IQjoIWQFRNw3RYKIEU9bbY+OX b/0VL6GcrcDGghAjO99ieJltVFXiH+EFLAQOu7f90C78OTpCgnneYBcncGmbx1o4WZKFQb e2skWjtFzWJy3edM6lUoY6oydZCG57I= X-Stat-Signature: 414aw1amtg9qjat8h1on5jfxnhytub11 X-Rspamd-Server: rspam08 X-Rspam-User: Authentication-Results: imf06.hostedemail.com; dkim=none ("invalid DKIM record") header.d=intel.com header.s=Intel header.b=DdudftnE; spf=pass (imf06.hostedemail.com: domain of dave.hansen@intel.com designates 134.134.136.126 as permitted sender) smtp.mailfrom=dave.hansen@intel.com; dmarc=pass (policy=none) header.from=intel.com X-Rspamd-Queue-Id: A45D4180007 X-HE-Tag: 1669144457-765857 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/20/22 16:26, Kai Huang wrote: > The first step of initializing the module is to call TDH.SYS.INIT once > on any logical cpu to do module global initialization. Do the module > global initialization. > > It also detects the TDX module, as seamcall() returns -ENODEV when the > module is not loaded. Part of making a good patch set is telling a bit of a story. In patch 4, you laid out 6 steps necessary to initialize TDX. On top of that, there is infrastructure It would be great to lay that out in a way that folks can actually follow along. For instance, it would be great to tell the reader here that this patch is an inflection point. It is transitioning out of the infrastructure (patches 1->6) and into the actual "multi-steps" of initialization that the module spec requires. This patch is *TOTALLY* different from the one before it because it actually _starts_ to do something useful. But, you wouldn't know it from the changelog. > arch/x86/virt/vmx/tdx/tdx.c | 19 +++++++++++++++++-- > arch/x86/virt/vmx/tdx/tdx.h | 1 + > 2 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c > index 5db1a05cb4bd..f292292313bd 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.c > +++ b/arch/x86/virt/vmx/tdx/tdx.c > @@ -208,8 +208,23 @@ static void seamcall_on_each_cpu(struct seamcall_ctx *sc) > */ > static int init_tdx_module(void) > { > - /* The TDX module hasn't been detected */ > - return -ENODEV; > + int ret; > + > + /* > + * Call TDH.SYS.INIT to do the global initialization of > + * the TDX module. It also detects the module. > + */ > + ret = seamcall(TDH_SYS_INIT, 0, 0, 0, 0, NULL, NULL); > + if (ret) > + goto out; Please also note that the 0's are all just unused parameters. They mean nothing. > + > + /* > + * Return -EINVAL until all steps of TDX module initialization > + * process are done. > + */ > + ret = -EINVAL; > +out: > + return ret; > } It might be a bit unconventional, but can you imagine how well it would tell the story if this comment said: /* * TODO: * - Logical-CPU scope initialization (TDH_SYS_INIT_LP) * - Enumerate capabilities and platform configuration (TDH_SYS_CONFIG) ... */ and then each of the following patches that *did* those things removed the TODO line from the list. That TODO list could have been added in patch 4. > static void shutdown_tdx_module(void) > diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h > index 215cc1065d78..0b415805c921 100644 > --- a/arch/x86/virt/vmx/tdx/tdx.h > +++ b/arch/x86/virt/vmx/tdx/tdx.h > @@ -15,6 +15,7 @@ > /* > * TDX module SEAMCALL leaf functions > */ > +#define TDH_SYS_INIT 33 > #define TDH_SYS_LP_SHUTDOWN 44 > > /*