Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <4bff50c3-4065-65a3-b698-28f2391bb776@linux.intel.com>
Date:   Sat, 23 Apr 2022 18:27:43 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.7.0
Subject: Re: [PATCH v3 08/21] x86/virt/tdx: Do logical-cpu scope TDX module
 initialization
Content-Language: en-US
To:     Kai Huang <kai.huang@intel.com>, linux-kernel@vger.kernel.org,
        kvm@vger.kernel.org
Cc:     seanjc@google.com, pbonzini@redhat.com, dave.hansen@intel.com,
        len.brown@intel.com, tony.luck@intel.com,
        rafael.j.wysocki@intel.com, reinette.chatre@intel.com,
        dan.j.williams@intel.com, peterz@infradead.org, ak@linux.intel.com,
        kirill.shutemov@linux.intel.com, isaku.yamahata@intel.com
References: <cover.1649219184.git.kai.huang@intel.com>
 <35081dba60ef61c313c2d7334815247248b8d1da.1649219184.git.kai.huang@intel.com>
From:   Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@linux.intel.com>
In-Reply-To: <35081dba60ef61c313c2d7334815247248b8d1da.1649219184.git.kai.huang@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 4/5/22 9:49 PM, Kai Huang wrote:
> Logical-cpu scope initialization requires calling TDH.SYS.LP.INIT on all
> BIOS-enabled cpus, otherwise the TDH.SYS.CONFIG SEAMCALL will fail.

IIUC, this change handles logical CPU initialization part of TDX module
initialization. So why talk about TDH.SYS.CONFIG failure here? Are they
related?

> TDH.SYS.LP.INIT can be called concurrently on all cpus.

IMO, if you move the following paragraph to the beginning, it is easier
to understand "what" and "why" part of this change.
> 
> Following global initialization, do the logical-cpu scope initialization
> by calling TDH.SYS.LP.INIT on all online cpus.  Whether all BIOS-enabled
> cpus are online is not checked here for simplicity.  The caller of
> tdx_init() should guarantee all BIOS-enabled cpus are online.

Include specification reference for TDX module initialization and
TDH.SYS.LP.INIT.

In TDX module spec, section 22.2.35 (TDH.SYS.LP.INIT Leaf), mentions
some environment requirements. I don't see you checking here for it?
Is this already met?


-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer