linux-kernel - Re: [PATCH v5 06/22] x86/virt/tdx: Add skeleton to initialize TDX on demand

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 Jun 2022 10:39:21 +0800
From:   Chao Gao <chao.gao@...el.com>
To:     Kai Huang <kai.huang@...el.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        seanjc@...gle.com, pbonzini@...hat.com, dave.hansen@...el.com,
        len.brown@...el.com, tony.luck@...el.com,
        rafael.j.wysocki@...el.com, reinette.chatre@...el.com,
        dan.j.williams@...el.com, peterz@...radead.org, ak@...ux.intel.com,
        kirill.shutemov@...ux.intel.com,
        sathyanarayanan.kuppuswamy@...ux.intel.com,
        isaku.yamahata@...el.com
Subject: Re: [PATCH v5 06/22] x86/virt/tdx: Add skeleton to initialize TDX on
 demand

On Wed, Jun 22, 2022 at 11:16:29PM +1200, Kai Huang wrote:
>Before the TDX module can be used to create and run TD guests, it must
>be loaded into the isolated region pointed by the SEAMRR and properly
>initialized.  The TDX module is expected to be loaded by BIOS before
>booting to the kernel, and the kernel is expected to detect and
>initialize it.
>
>The TDX module can be initialized only once in its lifetime.  Instead
>of always initializing it at boot time, this implementation chooses an
>on-demand approach to initialize TDX until there is a real need (e.g
>when requested by KVM).  This avoids consuming the memory that must be
>allocated by kernel and given to the TDX module as metadata (~1/256th of
>the TDX-usable memory), and also saves the time of initializing the TDX
>module (and the metadata) when TDX is not used at all.  Initializing the
>TDX module at runtime on-demand also is more flexible to support TDX
>module runtime updating in the future (after updating the TDX module, it
>needs to be initialized again).
>
>Add a placeholder tdx_init() to detect and initialize the TDX module on
>demand, with a state machine protected by mutex to support concurrent
>calls from multiple callers.
>
>The TDX module will be initialized in multi-steps defined by the TDX
>architecture:
>
>  1) Global initialization;
>  2) Logical-CPU scope initialization;
>  3) Enumerate the TDX module capabilities and platform configuration;
>  4) Configure the TDX module about usable memory ranges and global
>     KeyID information;
>  5) Package-scope configuration for the global KeyID;
>  6) Initialize usable memory ranges based on 4).
>
>The TDX module can also be shut down at any time during its lifetime.
>In case of any error during the initialization process, shut down the
>module.  It's pointless to leave the module in any intermediate state
>during the initialization.
>
>Signed-off-by: Kai Huang <kai.huang@...el.com>

Reviewed-by: Chao Gao <chao.gao@...el.com>

One nit below:

>+static int __tdx_init(void)
>+{
>+	int ret;
>+
>+	/*
>+	 * Initializing the TDX module requires running some code on
>+	 * all MADT-enabled CPUs.  If not all MADT-enabled CPUs are
>+	 * online, it's not possible to initialize the TDX module.
>+	 *
>+	 * For simplicity temporarily disable CPU hotplug to prevent
>+	 * any CPU from going offline during the initialization.
>+	 */
>+	cpus_read_lock();
>+
>+	/*
>+	 * Check whether all MADT-enabled CPUs are online and return
>+	 * early with an explicit message so the user can be aware.
>+	 *
>+	 * Note ACPI CPU hotplug is prevented when TDX is enabled, so
>+	 * num_processors always reflects all present MADT-enabled
>+	 * CPUs during boot when disabled_cpus is 0.
>+	 */
>+	if (disabled_cpus || num_online_cpus() != num_processors) {
>+		pr_err("Unable to initialize the TDX module when there's offline CPU(s).\n");
>+		ret = -EINVAL;
>+		goto out;
>+	}
>+
>+	ret = init_tdx_module();
>+	if (ret == -ENODEV) {
>+		pr_info("TDX module is not loaded.\n");

tdx_module_status should be set to TDX_MODULE_NONE here.

>+		goto out;
>+	}