linux-kernel - Re: [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and initializing 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:   Mon, 28 Mar 2022 22:24:05 +1300
From:   Kai Huang <kai.huang@...el.com>
To:     "Tian, Kevin" <kevin.tian@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "Gleixner, Thomas" <thomas.gleixner@...el.com>
Cc:     "Hansen, Dave" <dave.hansen@...el.com>,
        "Christopherson,, Sean" <seanjc@...gle.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "sathyanarayanan.kuppuswamy@...ux.intel.com" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "Luck, Tony" <tony.luck@...el.com>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "Williams, Dan J" <dan.j.williams@...el.com>,
        "Yamahata, Isaku" <isaku.yamahata@...el.com>
Subject: Re: [PATCH v2 04/21] x86/virt/tdx: Add skeleton for detecting and
 initializing TDX on demand

> 
> cpu_present_mask does not always represent BIOS-enabled CPUs due
> to those boot options. Then why do we care whether CPUs in this mask
> (if only representing a subset of BIOS-enabled CPUs) are at least brought
> up once? It will fail at TDH.SYS.CONFIG anyway.

As I said, this is used to make sure SEAMRR has been detected on all cpus, so
that any BIOS misconfiguration on SEAMRR has been detected.  Otherwise,
seamrr_enabled() may not be reliable (theoretically).

Alternatively, I think we can also add check to disable TDX when 'maxcpus' has
been specified, but I think the current way is better.

> 
> btw your comment said that 'maxcpus' is basically an invalid mode
> due to MCE broadcase problem. I didn't find any code to block it when
> MCE is enabled, 

Please see below comment in cpu_smt_allowed():

static inline bool cpu_smt_allowed(unsigned int cpu)
{
	...
        /*
         * On x86 it's required to boot all logical CPUs at least once so
         * that the init code can get a chance to set CR4.MCE on each
         * CPU. Otherwise, a broadcasted MCE observing CR4.MCE=0b on any
         * core will shutdown the machine.
         */
	 return !cpumask_test_cpu(cpu, &cpus_booted_once_mask);
}

> thus wonder the rationale behind and whether that
> rationale can be brought to this series (i.e. no check against those
> conflicting boot options and just let SEAMCALL itself to detect and fail).
> 
> @Thomas, any guidance here?
> 
> Thanks
> Kevin