Re: [RFC PATCH v6 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()

From: Dave Hansen <dave@sr71.net>
To: Sean Christopherson <seanjc@google.com>
Cc: Kai Huang <kai.huang@intel.com>,
	linux-sgx@vger.kernel.org, kvm@vger.kernel.org, x86@kernel.org,
	jarkko@kernel.org, luto@kernel.org, rick.p.edgecombe@intel.com,
	haitao.huang@intel.com, pbonzini@redhat.com, bp@alien8.de,
	tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com
Subject: Re: [RFC PATCH v6 03/25] x86/sgx: Wipe out EREMOVE from sgx_free_epc_page()
Date: Fri, 26 Feb 2021 12:12:56 -0800	[thread overview]
Message-ID: <55e0f003-ca2b-24d2-5a23-31a77c5b943d@sr71.net> (raw)
In-Reply-To: <YDlRgtnVS4+KkzUW@google.com>

On 2/26/21 11:52 AM, Sean Christopherson wrote:
>> We must give a more informative message saying that the page is leaked.
>>  Ideally, we'd also make this debuggable by dumping out how many of
>> these pages there have been somewhere.  That can wait, though, until we
>> have some kind of stats coming out of the code (there's nothing now).  A
>> comment to remind us to do this would be nice.
> Eh, having debugged these several times, the WARN_ONCE in sgx_reset_epc_page()
> is probably sufficient.  IIRC, when I hit this, things were either laughably
> broken and every page was failing, or there was another ENCLS failure somewhere
> else that provided additional info.  Not saying don't add more debug info,
> rather that it's probably not a priority.

Minimally, I just want a warning that says, "Whoops, I leaked a page".
Or EREMOVE could even say, "whoops, this *MIGHT* leak a page".

My beef is mostly that "EREMOVE failed" doesn't tell and end user squat
about what this means for their system.  At least if we say "leaked",
they have some inclination that they've got to reboot to get the page back.