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

On Wed, 2023-06-28 at 15:08 +0200, Peter Zijlstra wrote:
> On Tue, Jun 27, 2023 at 02:12:37AM +1200, Kai Huang wrote:
> 
> > +/*
> > + * Do the module global initialization if not done yet.
> > + * It's always called with interrupts and preemption disabled.
> > + */
> > +static int try_init_module_global(void)
> > +{
> > +	unsigned long flags;
> > +	int ret;
> > +
> > +	/*
> > +	 * The TDX module global initialization only needs to be done
> > +	 * once on any cpu.
> > +	 */
> > +	raw_spin_lock_irqsave(&tdx_global_init_lock, flags);
> > +
> > +	if (tdx_global_initialized) {
> > +		ret = 0;
> > +		goto out;
> > +	}
> > +
> > +	/* All '0's are just unused parameters. */
> > +	ret = seamcall(TDH_SYS_INIT, 0, 0, 0, 0, NULL, NULL);
> > +	if (!ret)
> > +		tdx_global_initialized = true;
> > +out:
> > +	raw_spin_unlock_irqrestore(&tdx_global_init_lock, flags);
> > +
> > +	return ret;
> > +}
> > +
> > +/**
> > + * tdx_cpu_enable - Enable TDX on local cpu
> > + *
> > + * Do one-time TDX module per-cpu initialization SEAMCALL (and TDX module
> > + * global initialization SEAMCALL if not done) on local cpu to make this
> > + * cpu be ready to run any other SEAMCALLs.
> > + *
> > + * Call this function with preemption disabled.
> > + *
> > + * Return 0 on success, otherwise errors.
> > + */
> > +int tdx_cpu_enable(void)
> > +{
> > +	int ret;
> > +
> > +	if (!platform_tdx_enabled())
> > +		return -ENODEV;
> > +
> > +	lockdep_assert_preemption_disabled();
> > +
> > +	/* Already done */
> > +	if (__this_cpu_read(tdx_lp_initialized))
> > +		return 0;
> > +
> > +	/*
> > +	 * The TDX module global initialization is the very first step
> > +	 * to enable TDX.  Need to do it first (if hasn't been done)
> > +	 * before the per-cpu initialization.
> > +	 */
> > +	ret = try_init_module_global();
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* All '0's are just unused parameters */
> > +	ret = seamcall(TDH_SYS_LP_INIT, 0, 0, 0, 0, NULL, NULL);
> > +	if (ret)
> > +		return ret;
> 
> And here you do *NOT* have IRQs disabled... so an IRQ can come in here
> and do the above again.
> 
> I suspect that's a completely insane thing to have happen, but the way
> the code is written does not tell me this and might even suggest I
> should worry about it, per the above thing actually disabling IRQs.
> 

I can change lockdep_assert_preemption_disabled() to
lockdep_assert_irqs_disabled(), making this function only being called from IPI.
As Kirill also suggested we can do this way as for now KVM is the only user of
this function and it enables hardware for all cpus via IPI.
>