Re: [PATCH v2 1/2] x86/sgx: Do not update sgx_nr_free_pages in sgx_setup_epc_section()

From: Jarkko Sakkinen <jarkko@kernel.org>
To: Borislav Petkov <bp@alien8.de>
Cc: linux-sgx@vger.kernel.org,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] x86/sgx: Do not update sgx_nr_free_pages in sgx_setup_epc_section()
Date: Thu, 8 Apr 2021 12:22:56 +0300	[thread overview]
Message-ID: <YG7LcLFKokoGzslp@kernel.org> (raw)
In-Reply-To: <20210408085640.GD10192@zn.tnic>

On Thu, Apr 08, 2021 at 10:56:40AM +0200, Borislav Petkov wrote:
> On Thu, Apr 08, 2021 at 11:48:46AM +0300, Jarkko Sakkinen wrote:
> > The regression is that the sgx_nr_free_pages is also incremented by
> > sgx_free_epc_pages(), and thus it ends up having double the number of
> > pages available.
> 
> So when you add a new EPC section with sgx_setup_epc_section(), those
> new pages in "nr_pages" are initially not going to be accounted
> anywhere? Or is that sgx_nr_all_pages? And you do that in your second
> patch...
> 
> But those new pages coming in *are* free pages so they should be in the
> free pages count too, IMHO.

They are not in the "free_page_list" before sanitization process has put
them to there. So in that way the count is also better in sync with this
fix.

It was even before NUMA patches kind of out-of-sync, i.e. free pages count
was incremented before putting them to the free list, but it didn't matter
that much because sanitization is fairly fast and it only prevented ksgxd
to trigger small time after the booth.

I think I'll send the fix for this now as a separate patch with a better
descriptino so that I can use more time to the attributes and implement
that properly at once.

> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

/Jarkko