linux-kernel - RE: [PATCH v8 1/5] x86/sgx: Introduce a counter to count the sgx_(vepc

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: <DM8PR11MB57500C83B0C612C026D1965DE75CA@DM8PR11MB5750.namprd11.prod.outlook.com>
Date: Tue, 22 Jul 2025 06:45:03 +0000
From: "Reshetova, Elena" <elena.reshetova@...el.com>
To: Jarkko Sakkinen <jarkko@...nel.org>
CC: "Hansen, Dave" <dave.hansen@...el.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "Huang, Kai" <kai.huang@...el.com>, "mingo@...nel.org"
	<mingo@...nel.org>, "linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"x86@...nel.org" <x86@...nel.org>, "Mallick, Asit K"
	<asit.k.mallick@...el.com>, "Scarlata, Vincent R"
	<vincent.r.scarlata@...el.com>, "Cai, Chong" <chongc@...gle.com>, "Aktas,
 Erdem" <erdemaktas@...gle.com>, "Annapurve, Vishal" <vannapurve@...gle.com>,
	"Bondarevska, Nataliia" <bondarn@...gle.com>, "Raynor, Scott"
	<scott.raynor@...el.com>
Subject: RE: [PATCH v8 1/5] x86/sgx: Introduce a counter to count the
 sgx_(vepc_)open()

> 
> On Sat, Jul 19, 2025 at 02:15:16PM +0300, Jarkko Sakkinen wrote:
> > On Tue, Jul 15, 2025 at 03:40:18PM +0300, Elena Reshetova wrote:
> > > Currently SGX does not have a global counter to count the
> > > active users from userspace or hypervisor. Implement such a counter,
> > > sgx_usage_count. It will be used by the driver when attempting
> > > to call EUPDATESVN SGX instruction.
> >
...
> > This is essentially a wrapper over pre-existing function. I vote for
> > renaming the pre-existing function as __sgx_vepc_open() and add then a
> > new function calling it:
> >
> > static int sgx_vepc_open(struct inode *inode, struct file *file)
> > {
> > 	int ret;
> >
> > 	ret = sgx_inc_usage_count();
> > 	if (ret)
> > 		return ret;
> >
> > 	ret =  __sgx_vepc_open(inode, file);
> > 	if (ret) {
> > 		sgx_dec_usage_count();
> > 		return ret;
> > 	}
> >
> > 	return 0;
> > }
> >
> > I think this a factor better standing point also when having to dig
> > into this later on (easier to see the big picture) as it has clear
> > split of responsibilities:
> >
> > 1. top layer manages to usage count
> > 2. lower layer allocates vepc structures (which has nothing to do
> >    with the logic of the usage count).
> >
> > This comment applies also to sgx_open().
> 
> I'd also split this into two patches (those are not suggestions for
> short summaries just saying):
> 
> Patch #1: Rename {sgx_open(),sgx_vepc_open()} as
> {__sgx_open,__sgx_vepc_open}
> Patch #2: Add a new function called {sgx_open(),sgx_vepc_open()} and fixup
> the call sites.

Sure, I will test and submit the v9 next with these changes. 

> 
> This is not similar scenario as the one I was complaining with 4/5
> and 5/5 because second patch adds new functions, which just have
> names that were used for different purpose in the past (just
> saying because thought this suggestion might soudn otherwise
> somehow contradictory).

Thank you for elaborating, yes, I understand it is different but
Dave has a different opinion there, so I am not sure which way to
take. Will wait until your discussion with Dave completes on this one. 

Best Regards,
Elena