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]

Message-ID: <8ad4e66a71e6a4e7af1ce08c9776fe2097cfeff6.camel@intel.com>
Date:   Tue, 29 Mar 2022 16:17:18 +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

On Tue, 2022-03-29 at 16:10 +1300, Kai Huang wrote:
> > > > 
> > > > *all cpus* is questionable.
> > > > 
> > > > Say BIOS enabled 8 CPUs: [0 - 7]
> > > > 
> > > > cpu_present_map covers [0 - 5], due to nr_cpus=6
> > > > 
> > > > You compared cpus_booted_once_mask to cpu_present_mask so if
> > > maxcpus
> > > > is set to a number < nr_cpus SEAMRR is considered disabled because you
> > > > cannot verify CPUs between [max_cpus, nr_cpus).
> 
> Sorry my bad.  We can verify CPUs between [max_cpus, nr_cpus).  When any cpu
> within that range becomes online, the detection code is run.  The paranoid
> check
> in seamrr_enabled() is used to check whether all cpus within [max_cpus,
> nr_cpus)
> (if there's any -- cpus within [0, max_cpus) are up during boot) have been
> brought up at least once. 
> 
> > > 
> > > SEAMRR is not considered as disabled in this case, at least in my
> > > intention.
> > 
> > the function is called seamrr_enabled(), and false is returned if above
> > check is not passed. So it is the intention from the code.
> 
> The false is returned if something error is discovered among cpus [0 -
> present_cpus].  It returns true even if we cannot verify [present_cpus,
> bios_enabled_cpus).

Sorry typo.  Not "something error is discovered among cpus [0 - present_cpus)",
but "when there's any cpu within [0 - present_cpus) hasn't been brought up
once".
 
> 
> > 
> > > My
> > > understanding on the spec is if SEAMRR is configured as enabled on one
> > > core
> > > (the
> > > SEAMRR MSRs are core-scope), the SEAMCALL instruction can work on that
> > > core.  It
> > > is TDX's requirement that some SEAMCALL needs to be done on all BIOS-
> > > enabled
> > > CPUs to finish TDX initialization, but not SEAM's.
> > > 
> > > From this perspective, if we forget TDX at this moment but talk about SEAM
> > > alone, it might make sense to not just treat SEAMRR as disabled if kernel
> > > usable
> > > cpus are limited by 'nr_cpus'.  The chance that BIOS misconfigured SEAMRR
> > > is
> > > really rare.  If kernel can detect potential BIOS misconfiguration, it
> > > should do
> > > it.  Otherwise, perhaps it's more reasonable not to just treat SEAM as
> > > disabled.
> > 
> > My problem is just that you didn't adopt consistency policy for CPUs in
> > [maxcpus, nr_cpus) and CPUs in [nr_cpus, nr_bios_enabled_cpus). This is
> > the only trouble to me no matter what policy you want to pursue...
> 
> Let's separate SEAMRR detection and TDX initialization.  The paranoid check is
> only for SEAM detection, but not for TDX initialization.  As I said, it is
> TDX's
> requirement that some SEAMCALL must be run on all bios-enabled cpus, but not
> SEAM's.
> 
>