linux-kernel - Re: [PATCH 13/25] KVM: TDX: create/destroy VM structure

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: <850ef710eac95a5c36863c94e1b31a8090eb8a2a.camel@intel.com>
Date: Wed, 21 Aug 2024 00:23:42 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "nik.borisov@...e.com" <nik.borisov@...e.com>,
	"seanjc@...gle.com" <seanjc@...gle.com>
CC: "Huang, Kai" <kai.huang@...el.com>, "Li, Xiaoyao" <xiaoyao.li@...el.com>,
	"isaku.yamahata@...il.com" <isaku.yamahata@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Zhao, Yan Y"
	<yan.y.zhao@...el.com>, "sean.j.christopherson@...el.com"
	<sean.j.christopherson@...el.com>, "Yamahata, Isaku"
	<isaku.yamahata@...el.com>, "tony.lindgren@...ux.intel.com"
	<tony.lindgren@...ux.intel.com>
Subject: Re: [PATCH 13/25] KVM: TDX: create/destroy VM structure

On Mon, 2024-08-19 at 18:09 +0300, Nikolay Borisov wrote:
> > +/*
> > + * Some SEAMCALLs acquire the TDX module globally, and can fail with
> > + * TDX_OPERAND_BUSY.  Use a global mutex to serialize these SEAMCALLs.
> > + */
> > +static DEFINE_MUTEX(tdx_lock);
> 
> The way this lock is used is very ugly. So it essentially mimics a lock 
> which already lives in the tdx module. So why not simply gracefully 
> handle the TDX_OPERAND_BUSY return value or change the interface of the 
> module (yeah, it's probably late for this now) so expose the lock. This 
> lock breaks one of the main rules of locking - "Lock data and not code"

Hmm, we would have to make SEAMCALLs to spin on that lock, where as mutexes can
sleep. I suspect that is where it came from. But we are trying to make the code
simple and obviously correct and add optimizations later. This might fit that
pattern, especially since it is just used during VM creation and teardown.