diff mbox series

[RFC,02/23] x86/sgx: Add enum for SGX_CHILD_PRESENT error code

Message ID 2a41e15dfda722dd1e34feeda34ce864cd82361b.1609890536.git.kai.huang@intel.com (mailing list archive)
State New, archived
Headers show
Series KVM SGX virtualization support | expand

Commit Message

Kai Huang Jan. 6, 2021, 1:55 a.m. UTC
From: Sean Christopherson <sean.j.christopherson@intel.com>

SGX virtualization requires to allocate "raw" EPC and use it as "virtual
EPC" for SGX guest.  Unlike EPC used by SGX driver, virtual EPC doesn't
track how EPC pages are used in VM, e.g. (de)construction of enclaves,
so it cannot guarantee EREMOVE success, e.g. it doesn't have a priori
knowledge of which pages are SECS with non-zero child counts.

Add SGX_CHILD_PRESENT for use by SGX virtualization to assert EREMOVE
failures are expected, but only due to SGX_CHILD_PRESENT.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Kai Huang <kai.huang@intel.com>
---
 arch/x86/kernel/cpu/sgx/arch.h | 2 ++
 1 file changed, 2 insertions(+)

Comments

Dave Hansen Jan. 6, 2021, 6:28 p.m. UTC | #1
On 1/5/21 5:55 PM, Kai Huang wrote:
> Add SGX_CHILD_PRESENT for use by SGX virtualization to assert EREMOVE
> failures are expected, but only due to SGX_CHILD_PRESENT.

This dances around the fact that this is an architectural error-code.
Could that be explicit?  Maybe the subject should be:

	Add SGX_CHILD_PRESENT hardware error code
Kai Huang Jan. 6, 2021, 9:40 p.m. UTC | #2
On Wed, 6 Jan 2021 10:28:55 -0800 Dave Hansen wrote:
> On 1/5/21 5:55 PM, Kai Huang wrote:
> > Add SGX_CHILD_PRESENT for use by SGX virtualization to assert EREMOVE
> > failures are expected, but only due to SGX_CHILD_PRESENT.
> 
> This dances around the fact that this is an architectural error-code.
> Could that be explicit?  Maybe the subject should be:
> 
> 	Add SGX_CHILD_PRESENT hardware error code

Sure. I'll do that.
Jarkko Sakkinen Jan. 11, 2021, 11:32 p.m. UTC | #3
On Wed, Jan 06, 2021 at 02:55:19PM +1300, Kai Huang wrote:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> SGX virtualization requires to allocate "raw" EPC and use it as "virtual
> EPC" for SGX guest.  Unlike EPC used by SGX driver, virtual EPC doesn't
> track how EPC pages are used in VM, e.g. (de)construction of enclaves,
> so it cannot guarantee EREMOVE success, e.g. it doesn't have a priori
> knowledge of which pages are SECS with non-zero child counts.
> 
> Add SGX_CHILD_PRESENT for use by SGX virtualization to assert EREMOVE
> failures are expected, but only due to SGX_CHILD_PRESENT.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Kai Huang <kai.huang@intel.com>

Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

/Jarkko
Kai Huang Jan. 12, 2021, 12:16 a.m. UTC | #4
On Tue, 12 Jan 2021 01:32:19 +0200 Jarkko Sakkinen wrote:
> On Wed, Jan 06, 2021 at 02:55:19PM +1300, Kai Huang wrote:
> > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > 
> > SGX virtualization requires to allocate "raw" EPC and use it as "virtual
> > EPC" for SGX guest.  Unlike EPC used by SGX driver, virtual EPC doesn't
> > track how EPC pages are used in VM, e.g. (de)construction of enclaves,
> > so it cannot guarantee EREMOVE success, e.g. it doesn't have a priori
> > knowledge of which pages are SECS with non-zero child counts.
> > 
> > Add SGX_CHILD_PRESENT for use by SGX virtualization to assert EREMOVE
> > failures are expected, but only due to SGX_CHILD_PRESENT.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > Signed-off-by: Kai Huang <kai.huang@intel.com>
> 
> Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

Thanks Jarkko. 

Dave suggested to change patch subject to explicitly call out hardware error
code:
	Add SGX_CHILD_PRESENT hardware error code

I suppose this also works for you, and I can have your Acked-by after I changed
that in v2?

Thanks,
-Kai
Jarkko Sakkinen Jan. 12, 2021, 12:26 a.m. UTC | #5
On Wed, Jan 06, 2021 at 10:28:55AM -0800, Dave Hansen wrote:
> On 1/5/21 5:55 PM, Kai Huang wrote:
> > Add SGX_CHILD_PRESENT for use by SGX virtualization to assert EREMOVE
> > failures are expected, but only due to SGX_CHILD_PRESENT.
> 
> This dances around the fact that this is an architectural error-code.
> Could that be explicit?  Maybe the subject should be:
> 
> 	Add SGX_CHILD_PRESENT hardware error code

Yeah, a valid point. Please, change this.

/Jarkko
Jarkko Sakkinen Jan. 12, 2021, 1:46 a.m. UTC | #6
On Tue, Jan 12, 2021 at 01:16:53PM +1300, Kai Huang wrote:
> On Tue, 12 Jan 2021 01:32:19 +0200 Jarkko Sakkinen wrote:
> > On Wed, Jan 06, 2021 at 02:55:19PM +1300, Kai Huang wrote:
> > > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > > 
> > > SGX virtualization requires to allocate "raw" EPC and use it as "virtual
> > > EPC" for SGX guest.  Unlike EPC used by SGX driver, virtual EPC doesn't
> > > track how EPC pages are used in VM, e.g. (de)construction of enclaves,
> > > so it cannot guarantee EREMOVE success, e.g. it doesn't have a priori
> > > knowledge of which pages are SECS with non-zero child counts.
> > > 
> > > Add SGX_CHILD_PRESENT for use by SGX virtualization to assert EREMOVE
> > > failures are expected, but only due to SGX_CHILD_PRESENT.
> > > 
> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > > Signed-off-by: Kai Huang <kai.huang@intel.com>
> > 
> > Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> Thanks Jarkko. 
> 
> Dave suggested to change patch subject to explicitly call out hardware error
> code:
> 	Add SGX_CHILD_PRESENT hardware error code
> 
> I suppose this also works for you, and I can have your Acked-by after I changed
> that in v2?

Yeah, I agree with that.

/Jarkko
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/sgx/arch.h b/arch/x86/kernel/cpu/sgx/arch.h
index dd7602c44c72..56b0f8ae3f92 100644
--- a/arch/x86/kernel/cpu/sgx/arch.h
+++ b/arch/x86/kernel/cpu/sgx/arch.h
@@ -26,12 +26,14 @@ 
  * enum sgx_return_code - The return code type for ENCLS, ENCLU and ENCLV
  * %SGX_NOT_TRACKED:		Previous ETRACK's shootdown sequence has not
  *				been completed yet.
+ * %SGX_CHILD_PRESENT		Enclave has child pages present in the EPC.
  * %SGX_INVALID_EINITTOKEN:	EINITTOKEN is invalid and enclave signer's
  *				public key does not match IA32_SGXLEPUBKEYHASH.
  * %SGX_UNMASKED_EVENT:		An unmasked event, e.g. INTR, was received
  */
 enum sgx_return_code {
 	SGX_NOT_TRACKED			= 11,
+	SGX_CHILD_PRESENT		= 13,
 	SGX_INVALID_EINITTOKEN		= 16,
 	SGX_UNMASKED_EVENT		= 128,
 };