linux-kernel - Re: [PATCH v12 07/22] x86/virt/tdx: Add skeleton to enable 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: <abbb1036723185108bb57a7946203fff5f6824eb.camel@intel.com>
Date:   Sat, 1 Jul 2023 08:15:06 +0000
From:   "Huang, Kai" <kai.huang@...el.com>
To:     "peterz@...radead.org" <peterz@...radead.org>
CC:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "Williams, Dan J" <dan.j.williams@...el.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        "Hansen, Dave" <dave.hansen@...el.com>,
        "david@...hat.com" <david@...hat.com>,
        "bagasdotme@...il.com" <bagasdotme@...il.com>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Christopherson,, Sean" <seanjc@...gle.com>,
        "Chatre, Reinette" <reinette.chatre@...el.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "Yamahata, Isaku" <isaku.yamahata@...el.com>,
        "nik.borisov@...e.com" <nik.borisov@...e.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "Luck, Tony" <tony.luck@...el.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "Shahar, Sagi" <sagis@...gle.com>,
        "imammedo@...hat.com" <imammedo@...hat.com>,
        "hpa@...or.com" <hpa@...or.com>, "Gao, Chao" <chao.gao@...el.com>,
        "bp@...en8.de" <bp@...en8.de>, "Brown, Len" <len.brown@...el.com>,
        "sathyanarayanan.kuppuswamy@...ux.intel.com" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "Huang, Ying" <ying.huang@...el.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH v12 07/22] x86/virt/tdx: Add skeleton to enable TDX on
 demand

On Fri, 2023-06-30 at 10:09 +0000, Huang, Kai wrote:
> On Fri, 2023-06-30 at 11:22 +0200, Peter Zijlstra wrote:
> > On Thu, Jun 29, 2023 at 12:15:13AM +0000, Huang, Kai wrote:
> > 
> > > > 	Can be called locally or through an IPI function call.
> > > > 
> > > 
> > > Thanks.  As in another reply, if using spinlock is OK, then I think we can say
> > > it will be called either locally or through an IPI function call.  Otherwise, we
> > > do via a new separate function tdx_global_init() and no lock is needed in that
> > > function.  The caller should call it properly.
> > 
> > IPI must use raw_spinlock_t. I'm ok with using raw_spinlock_t if there's
> > actual need for that, but the code as presented didn't -- in comments or
> > otherwise -- make it clear why it was as it was.
> 
> There's no hard requirement as I replied in another email.
> 
> Presumably you prefer the option to have a dedicated tdx_global_init() so we can
> avoid the raw_spinlock_t?
> 

Hmm... didn't have enough coffee.  Sorry after more thinking, I think we need to
avoid tdx_global_init() but do TDH.SYS.INIT within tdx_cpu_enable() with
raw_spinlock_t.  The reason is although KVM will be the first caller of TDX,
there will be other caller of TDX in later phase (e.g., IOMMU TDX support) so we
need to consider race between those callers.

With multiple callers, the tdx_global_init() and tdx_cpu_enable() from them need
to be serialized anyway, and having the additional tdx_global_init() will just
make things more complicated to do.

So I think the simplest way is to use a per-cpu variable to track
TDH.SYS.LP.INIT in tdx_cpu_enable() and only call tdx_cpu_enable() from local
with IRQ disabled or from IPI function call, and use raw_spinlock_t for
TDH.SYS.INIT inside tdx_cpu_enable() to make sure it only gets called once.

I'll clarify this in the changelog and/or comments.

Again sorry for the noise and please let me know for any comments.  Thanks!