[PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function to KVM guest

[PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function to KVM guest

[Kai Huang]

The new Asynchronous Exit (AEX) notification mechanism (AEX-notify)
allows one enclave to receive a notification in the ERESUME after the
enclave exit due to an AEX.  EDECCSSA is a new SGX user leaf function
(ENCLU[EDECCSSA]) to facilitate the AEX notification handling.  The new
EDECCSSA is enumerated via CPUID(EAX=0x12,ECX=0x0):EAX[11].

Besides Allowing reporting the new AEX-notify attribute to KVM guests,
also allow reporting the new EDECCSSA user leaf function to KVM guests
so the guest can fully utilize the AEX-notify mechanism.

Similar to existing X86_FEATURE_SGX1 and X86_FEATURE_SGX2, introduce a
new scattered X86_FEATURE_SGX_EDECCSSA bit for the new EDECCSSA, and
report it in KVM's supported CPUIDs so the userspace hypervisor (i.e.
Qemu) can enable it for the guest.

Note there's no additional enabling work required to allow guest to use
the new EDECCSSA.  KVM is not able to trap ENCLU anyway.

More background about how do AEX-notify and EDECCSSA work:

SGX maintains a Current State Save Area Frame (CSSA) for each enclave
thread.  When AEX happens, the enclave thread context is saved to the
CSSA and the CSSA is increased by 1.  For a normal ERESUME which doesn't
deliver AEX notification, it restores the saved thread context from the
previously saved SSA and decreases the CSSA.  If AEX-notify is enabled
for one enclave, the ERESUME acts differently.  Instead of restoring the
saved thread context and decreasing the CSSA, it acts like EENTER which
doesn't decrease the CSSA but establishes a clean slate thread context
using the CSSA for the enclave to handle the notification.  After some
handling, the enclave must discard the "new-established" SSA and switch
back to the previously saved SSA (upon AEX).  Otherwise, the enclave
will run out of SSA space upon further AEXs and eventually fail to run.

To solve this problem, the new EDECCSSA essentially decreases the CSSA.
It can be used by the enclave notification handler to switch back to the
previous saved SSA when needed, i.e. after it handles the notification.

Signed-off-by: Kai Huang <kai.huang@intel.com>
---

Hi Dave,

This patch, along with your patch to expose AEX-notify attribute bit to
guest, have been tested that both AEX-notify and EDECCSSA work in the VM.
Feel free to merge this patch.

v1->v2:

 - Rebase to latest tip/x86/sgx
 - Add scattered X86_FEATURE_SGX_EDECCSSA bit CPUID handling
 - Add X86_FEATURE_SGX_EDECCSSA to cpuid dependency table.
 - Slightly changed commit message.

---
 arch/x86/include/asm/cpufeatures.h | 1 +
 arch/x86/kernel/cpu/cpuid-deps.c   | 1 +
 arch/x86/kernel/cpu/scattered.c    | 1 +
 arch/x86/kvm/cpuid.c               | 2 +-
 arch/x86/kvm/reverse_cpuid.h       | 3 +++
 5 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 235dc85c91c3..ccdd35adae9e 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -304,6 +304,7 @@
 #define X86_FEATURE_UNRET		(11*32+15) /* "" AMD BTB untrain return */
 #define X86_FEATURE_USE_IBPB_FW		(11*32+16) /* "" Use IBPB during runtime firmware calls */
 #define X86_FEATURE_RSB_VMEXIT_LITE	(11*32+17) /* "" Fill RSB on VM exit when EIBRS is enabled */
+#define X86_FEATURE_SGX_EDECCSSA	(11*32+18) /* "" SGX EDECCSSA user leaf function */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
 #define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* AVX VNNI instructions */
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index c881bcafba7d..d95221117129 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -75,6 +75,7 @@ static const struct cpuid_dep cpuid_deps[] = {
 	{ X86_FEATURE_SGX_LC,			X86_FEATURE_SGX	      },
 	{ X86_FEATURE_SGX1,			X86_FEATURE_SGX       },
 	{ X86_FEATURE_SGX2,			X86_FEATURE_SGX1      },
+	{ X86_FEATURE_SGX_EDECCSSA,		X86_FEATURE_SGX1      },
 	{ X86_FEATURE_XFD,			X86_FEATURE_XSAVES    },
 	{ X86_FEATURE_XFD,			X86_FEATURE_XGETBV1   },
 	{ X86_FEATURE_AMX_TILE,			X86_FEATURE_XFD       },
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index fd44b54c90d5..0bb339857985 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -40,6 +40,7 @@ static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_PER_THREAD_MBA,	CPUID_ECX,  0, 0x00000010, 3 },
 	{ X86_FEATURE_SGX1,		CPUID_EAX,  0, 0x00000012, 0 },
 	{ X86_FEATURE_SGX2,		CPUID_EAX,  1, 0x00000012, 0 },
+	{ X86_FEATURE_SGX_EDECCSSA,	CPUID_EAX, 11, 0x00000012, 0 },
 	{ X86_FEATURE_HW_PSTATE,	CPUID_EDX,  7, 0x80000007, 0 },
 	{ X86_FEATURE_CPB,		CPUID_EDX,  9, 0x80000007, 0 },
 	{ X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 75dcf7a72605..c21b4a5dc8fa 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -644,7 +644,7 @@ void kvm_set_cpu_caps(void)
 	);
 
 	kvm_cpu_cap_init_scattered(CPUID_12_EAX,
-		SF(SGX1) | SF(SGX2)
+		SF(SGX1) | SF(SGX2) | SF(SGX_EDECCSSA)
 	);
 
 	kvm_cpu_cap_mask(CPUID_8000_0001_ECX,
diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
index a19d473d0184..4e5b8444f161 100644
--- a/arch/x86/kvm/reverse_cpuid.h
+++ b/arch/x86/kvm/reverse_cpuid.h
@@ -23,6 +23,7 @@ enum kvm_only_cpuid_leafs {
 /* Intel-defined SGX sub-features, CPUID level 0x12 (EAX). */
 #define KVM_X86_FEATURE_SGX1		KVM_X86_FEATURE(CPUID_12_EAX, 0)
 #define KVM_X86_FEATURE_SGX2		KVM_X86_FEATURE(CPUID_12_EAX, 1)
+#define KVM_X86_FEATURE_SGX_EDECCSSA	KVM_X86_FEATURE(CPUID_12_EAX, 11)
 
 struct cpuid_reg {
 	u32 function;
@@ -78,6 +79,8 @@ static __always_inline u32 __feature_translate(int x86_feature)
 		return KVM_X86_FEATURE_SGX1;
 	else if (x86_feature == X86_FEATURE_SGX2)
 		return KVM_X86_FEATURE_SGX2;
+	else if (x86_feature == X86_FEATURE_SGX_EDECCSSA)
+		return KVM_X86_FEATURE_SGX_EDECCSSA;
 
 	return x86_feature;
 }

base-commit: ee56a283988d739c25d2d00ffb22707cb487ab47
-- 
2.37.1

Re: [PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function to KVM guest

[Jarkko Sakkinen]

Nit: shouldn't be this be x86/kvm?

On Thu, Aug 18, 2022 at 02:38:29PM +1200, Kai Huang wrote:
> The new Asynchronous Exit (AEX) notification mechanism (AEX-notify)
> allows one enclave to receive a notification in the ERESUME after the
> enclave exit due to an AEX.  EDECCSSA is a new SGX user leaf function
> (ENCLU[EDECCSSA]) to facilitate the AEX notification handling.  The new
> EDECCSSA is enumerated via CPUID(EAX=0x12,ECX=0x0):EAX[11].
> 
> Besides Allowing reporting the new AEX-notify attribute to KVM guests,
> also allow reporting the new EDECCSSA user leaf function to KVM guests
> so the guest can fully utilize the AEX-notify mechanism.
> 
> Similar to existing X86_FEATURE_SGX1 and X86_FEATURE_SGX2, introduce a
> new scattered X86_FEATURE_SGX_EDECCSSA bit for the new EDECCSSA, and
> report it in KVM's supported CPUIDs so the userspace hypervisor (i.e.
> Qemu) can enable it for the guest.
> 
> Note there's no additional enabling work required to allow guest to use
> the new EDECCSSA.  KVM is not able to trap ENCLU anyway.
> 
> More background about how do AEX-notify and EDECCSSA work:
> 
> SGX maintains a Current State Save Area Frame (CSSA) for each enclave
> thread.  When AEX happens, the enclave thread context is saved to the
> CSSA and the CSSA is increased by 1.  For a normal ERESUME which doesn't
> deliver AEX notification, it restores the saved thread context from the
> previously saved SSA and decreases the CSSA.  If AEX-notify is enabled
> for one enclave, the ERESUME acts differently.  Instead of restoring the
> saved thread context and decreasing the CSSA, it acts like EENTER which
> doesn't decrease the CSSA but establishes a clean slate thread context
> using the CSSA for the enclave to handle the notification.  After some
> handling, the enclave must discard the "new-established" SSA and switch
> back to the previously saved SSA (upon AEX).  Otherwise, the enclave
> will run out of SSA space upon further AEXs and eventually fail to run.
> 
> To solve this problem, the new EDECCSSA essentially decreases the CSSA.
> It can be used by the enclave notification handler to switch back to the
> previous saved SSA when needed, i.e. after it handles the notification.
> 
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> ---
> 
> Hi Dave,
> 
> This patch, along with your patch to expose AEX-notify attribute bit to
> guest, have been tested that both AEX-notify and EDECCSSA work in the VM.
> Feel free to merge this patch.
> 
> v1->v2:
> 
>  - Rebase to latest tip/x86/sgx
>  - Add scattered X86_FEATURE_SGX_EDECCSSA bit CPUID handling
>  - Add X86_FEATURE_SGX_EDECCSSA to cpuid dependency table.
>  - Slightly changed commit message.
> 
> ---
>  arch/x86/include/asm/cpufeatures.h | 1 +
>  arch/x86/kernel/cpu/cpuid-deps.c   | 1 +
>  arch/x86/kernel/cpu/scattered.c    | 1 +
>  arch/x86/kvm/cpuid.c               | 2 +-
>  arch/x86/kvm/reverse_cpuid.h       | 3 +++
>  5 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 235dc85c91c3..ccdd35adae9e 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -304,6 +304,7 @@
>  #define X86_FEATURE_UNRET		(11*32+15) /* "" AMD BTB untrain return */
>  #define X86_FEATURE_USE_IBPB_FW		(11*32+16) /* "" Use IBPB during runtime firmware calls */
>  #define X86_FEATURE_RSB_VMEXIT_LITE	(11*32+17) /* "" Fill RSB on VM exit when EIBRS is enabled */
> +#define X86_FEATURE_SGX_EDECCSSA	(11*32+18) /* "" SGX EDECCSSA user leaf function */
>  
>  /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
>  #define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* AVX VNNI instructions */
> diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
> index c881bcafba7d..d95221117129 100644
> --- a/arch/x86/kernel/cpu/cpuid-deps.c
> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> @@ -75,6 +75,7 @@ static const struct cpuid_dep cpuid_deps[] = {
>  	{ X86_FEATURE_SGX_LC,			X86_FEATURE_SGX	      },
>  	{ X86_FEATURE_SGX1,			X86_FEATURE_SGX       },
>  	{ X86_FEATURE_SGX2,			X86_FEATURE_SGX1      },
> +	{ X86_FEATURE_SGX_EDECCSSA,		X86_FEATURE_SGX1      },
>  	{ X86_FEATURE_XFD,			X86_FEATURE_XSAVES    },
>  	{ X86_FEATURE_XFD,			X86_FEATURE_XGETBV1   },
>  	{ X86_FEATURE_AMX_TILE,			X86_FEATURE_XFD       },
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index fd44b54c90d5..0bb339857985 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -40,6 +40,7 @@ static const struct cpuid_bit cpuid_bits[] = {
>  	{ X86_FEATURE_PER_THREAD_MBA,	CPUID_ECX,  0, 0x00000010, 3 },
>  	{ X86_FEATURE_SGX1,		CPUID_EAX,  0, 0x00000012, 0 },
>  	{ X86_FEATURE_SGX2,		CPUID_EAX,  1, 0x00000012, 0 },
> +	{ X86_FEATURE_SGX_EDECCSSA,	CPUID_EAX, 11, 0x00000012, 0 },
>  	{ X86_FEATURE_HW_PSTATE,	CPUID_EDX,  7, 0x80000007, 0 },
>  	{ X86_FEATURE_CPB,		CPUID_EDX,  9, 0x80000007, 0 },
>  	{ X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 75dcf7a72605..c21b4a5dc8fa 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -644,7 +644,7 @@ void kvm_set_cpu_caps(void)
>  	);
>  
>  	kvm_cpu_cap_init_scattered(CPUID_12_EAX,
> -		SF(SGX1) | SF(SGX2)
> +		SF(SGX1) | SF(SGX2) | SF(SGX_EDECCSSA)
>  	);
>  
>  	kvm_cpu_cap_mask(CPUID_8000_0001_ECX,
> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
> index a19d473d0184..4e5b8444f161 100644
> --- a/arch/x86/kvm/reverse_cpuid.h
> +++ b/arch/x86/kvm/reverse_cpuid.h
> @@ -23,6 +23,7 @@ enum kvm_only_cpuid_leafs {
>  /* Intel-defined SGX sub-features, CPUID level 0x12 (EAX). */
>  #define KVM_X86_FEATURE_SGX1		KVM_X86_FEATURE(CPUID_12_EAX, 0)
>  #define KVM_X86_FEATURE_SGX2		KVM_X86_FEATURE(CPUID_12_EAX, 1)
> +#define KVM_X86_FEATURE_SGX_EDECCSSA	KVM_X86_FEATURE(CPUID_12_EAX, 11)
>  
>  struct cpuid_reg {
>  	u32 function;
> @@ -78,6 +79,8 @@ static __always_inline u32 __feature_translate(int x86_feature)
>  		return KVM_X86_FEATURE_SGX1;
>  	else if (x86_feature == X86_FEATURE_SGX2)
>  		return KVM_X86_FEATURE_SGX2;
> +	else if (x86_feature == X86_FEATURE_SGX_EDECCSSA)
> +		return KVM_X86_FEATURE_SGX_EDECCSSA;
>  
>  	return x86_feature;
>  }
> 
> base-commit: ee56a283988d739c25d2d00ffb22707cb487ab47
> -- 
> 2.37.1
> 


BR, Jarkko

Re: [PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function to KVM guest

[Jarkko Sakkinen]

On Thu, Aug 25, 2022 at 06:26:06AM +0300, Jarkko Sakkinen wrote:
> Nit: shouldn't be this be x86/kvm?

Also, why don't you make one patch set with Dave's patch
included.

BR, Jarkko

Re: [PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function to KVM guest

[Sean Christopherson]

On Thu, Aug 25, 2022, Jarkko Sakkinen wrote:
> Nit: shouldn't be this be x86/kvm?

Heh, no, because x86/kvm is the scope for Linux running as a KVM guest, i.e. for
changes to arch/x86/kernel/kvm.c.

But yeah, "KVM: x86:" or maybe even "KVM: VMX:" would be preferable given that all
of the meaningful changes are KVM specific.

> On Thu, Aug 18, 2022 at 02:38:29PM +1200, Kai Huang wrote:
> > The new Asynchronous Exit (AEX) notification mechanism (AEX-notify)
> > allows one enclave to receive a notification in the ERESUME after the
> > enclave exit due to an AEX.  EDECCSSA is a new SGX user leaf function
> > (ENCLU[EDECCSSA]) to facilitate the AEX notification handling.  The new
> > EDECCSSA is enumerated via CPUID(EAX=0x12,ECX=0x0):EAX[11].
> > 
> > Besides Allowing reporting the new AEX-notify attribute to KVM guests,
> > also allow reporting the new EDECCSSA user leaf function to KVM guests
> > so the guest can fully utilize the AEX-notify mechanism.
> > 
> > Similar to existing X86_FEATURE_SGX1 and X86_FEATURE_SGX2, introduce a
> > new scattered X86_FEATURE_SGX_EDECCSSA bit for the new EDECCSSA, and
> > report it in KVM's supported CPUIDs so the userspace hypervisor (i.e.
> > Qemu) can enable it for the guest.

Silly nit, but I'd prefer to leave off the "so the userspace hypervisor ... can
enable it for the guest".  Userspace doesn't actually need to wait for KVM enabling.
As noted below, KVM doesn't need to do anything extra, and KVM _can't_ prevent the
guest from using EDECCSSA.

> > Note there's no additional enabling work required to allow guest to use
> > the new EDECCSSA.  KVM is not able to trap ENCLU anyway.

And maybe call out that the KVM "enabling" is not strictly necessary?  And note
that there's a virtualization hole?  E.g.

  Note, no additional KVM enabling is required to allow the guest to use
  EDECCSSA, it's impossible to trap ENCLU (without completely preventing the
  guest from using SGX).  Advertise EDECCSSA as supported purely so that
  userspace doesn't need to special case EDECCSSA, i.e. doesn't need to
  manually check host CPUID.

  The inability to trap ENCLU also means that KVM can't prevent the guest
  from using EDECCSSA, but that virtualization hole is benign as far as KVM
  is concerned.  EDECCSSA is simply a fancy way to modify internal enclave
  state.

> > More background about how do AEX-notify and EDECCSSA work:
> > 
> > SGX maintains a Current State Save Area Frame (CSSA) for each enclave
> > thread.  When AEX happens, the enclave thread context is saved to the
> > CSSA and the CSSA is increased by 1.  For a normal ERESUME which doesn't
> > deliver AEX notification, it restores the saved thread context from the
> > previously saved SSA and decreases the CSSA.  If AEX-notify is enabled
> > for one enclave, the ERESUME acts differently.  Instead of restoring the
> > saved thread context and decreasing the CSSA, it acts like EENTER which
> > doesn't decrease the CSSA but establishes a clean slate thread context
> > using the CSSA for the enclave to handle the notification.  After some
> > handling, the enclave must discard the "new-established" SSA and switch
> > back to the previously saved SSA (upon AEX).  Otherwise, the enclave
> > will run out of SSA space upon further AEXs and eventually fail to run.
> > 
> > To solve this problem, the new EDECCSSA essentially decreases the CSSA.
> > It can be used by the enclave notification handler to switch back to the
> > previous saved SSA when needed, i.e. after it handles the notification.
> > 
> > Signed-off-by: Kai Huang <kai.huang@intel.com>
> > ---
> > 
> > Hi Dave,
> > 
> > This patch, along with your patch to expose AEX-notify attribute bit to
> > guest, have been tested that both AEX-notify and EDECCSSA work in the VM.
> > Feel free to merge this patch.

Dave, any objection to taking this through the KVM tree?

Re: [PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function to KVM guest

[Dave Hansen]

On 8/25/22 08:19, Sean Christopherson wrote:
>>> This patch, along with your patch to expose AEX-notify attribute bit to
>>> guest, have been tested that both AEX-notify and EDECCSSA work in the VM.
>>> Feel free to merge this patch.
> Dave, any objection to taking this through the KVM tree?

This specific patch?  Or are you talking about the couple of AEX-notify
patches in their entirety?

Re: [PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function to KVM guest

[Sean Christopherson]

On Thu, Aug 25, 2022, Dave Hansen wrote:
> On 8/25/22 08:19, Sean Christopherson wrote:
> >>> This patch, along with your patch to expose AEX-notify attribute bit to
> >>> guest, have been tested that both AEX-notify and EDECCSSA work in the VM.
> >>> Feel free to merge this patch.
> > Dave, any objection to taking this through the KVM tree?
> 
> This specific patch?  Or are you talking about the couple of AEX-notify
> patches in their entirety?

I was thinking just this specific patch, but I temporarily forgot there are more
patches in flight.  It would be a bit odd to have effectively half of the AEX-notify
enabling go through KVM.

So with shortlog/changelog tweaks,

Acked-by: Sean Christopherson <seanjc@google.com>

Re: [PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function to KVM guest

[Jarkko Sakkinen]

On Thu, Aug 25, 2022 at 03:49:38PM +0000, Sean Christopherson wrote:
> On Thu, Aug 25, 2022, Dave Hansen wrote:
> > On 8/25/22 08:19, Sean Christopherson wrote:
> > >>> This patch, along with your patch to expose AEX-notify attribute bit to
> > >>> guest, have been tested that both AEX-notify and EDECCSSA work in the VM.
> > >>> Feel free to merge this patch.
> > > Dave, any objection to taking this through the KVM tree?
> > 
> > This specific patch?  Or are you talking about the couple of AEX-notify
> > patches in their entirety?
> 
> I was thinking just this specific patch, but I temporarily forgot there are more
> patches in flight.  It would be a bit odd to have effectively half of the AEX-notify
> enabling go through KVM.
> 
> So with shortlog/changelog tweaks,
> 
> Acked-by: Sean Christopherson <seanjc@google.com>

with subsystem tag change (Sean's version):

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

BR, Jarkko

Re: [PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function to KVM guest

[Huang, Kai]

On Thu, 2022-08-25 at 15:19 +0000, Sean Christopherson wrote:
> On Thu, Aug 25, 2022, Jarkko Sakkinen wrote:
> > Nit: shouldn't be this be x86/kvm?
> 
> Heh, no, because x86/kvm is the scope for Linux running as a KVM guest, i.e. for
> changes to arch/x86/kernel/kvm.c.
> 
> But yeah, "KVM: x86:" or maybe even "KVM: VMX:" would be preferable given that all
> of the meaningful changes are KVM specific.
> 
> > On Thu, Aug 18, 2022 at 02:38:29PM +1200, Kai Huang wrote:
> > > The new Asynchronous Exit (AEX) notification mechanism (AEX-notify)
> > > allows one enclave to receive a notification in the ERESUME after the
> > > enclave exit due to an AEX.  EDECCSSA is a new SGX user leaf function
> > > (ENCLU[EDECCSSA]) to facilitate the AEX notification handling.  The new
> > > EDECCSSA is enumerated via CPUID(EAX=0x12,ECX=0x0):EAX[11].
> > > 
> > > Besides Allowing reporting the new AEX-notify attribute to KVM guests,
> > > also allow reporting the new EDECCSSA user leaf function to KVM guests
> > > so the guest can fully utilize the AEX-notify mechanism.
> > > 
> > > Similar to existing X86_FEATURE_SGX1 and X86_FEATURE_SGX2, introduce a
> > > new scattered X86_FEATURE_SGX_EDECCSSA bit for the new EDECCSSA, and
> > > report it in KVM's supported CPUIDs so the userspace hypervisor (i.e.
> > > Qemu) can enable it for the guest.
> 
> Silly nit, but I'd prefer to leave off the "so the userspace hypervisor ... can
> enable it for the guest".  Userspace doesn't actually need to wait for KVM enabling.
> As noted below, KVM doesn't need to do anything extra, and KVM _can't_ prevent the
> guest from using EDECCSSA.

Indeed KVM cannot prevent.

> 
> > > Note there's no additional enabling work required to allow guest to use
> > > the new EDECCSSA.  KVM is not able to trap ENCLU anyway.
> 
> And maybe call out that the KVM "enabling" is not strictly necessary?  And note
> that there's a virtualization hole?  E.g.
> 
>   Note, no additional KVM enabling is required to allow the guest to use
>   EDECCSSA, it's impossible to trap ENCLU (without completely preventing the
>   guest from using SGX).  Advertise EDECCSSA as supported purely so that
>   userspace doesn't need to special case EDECCSSA, i.e. doesn't need to
>   manually check host CPUID.
> 
>   The inability to trap ENCLU also means that KVM can't prevent the guest
>   from using EDECCSSA, but that virtualization hole is benign as far as KVM
>   is concerned.  EDECCSSA is simply a fancy way to modify internal enclave
>   state.

Thanks.  Will use above.


-- 
Thanks,
-Kai

Re: [PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function to KVM guest

[Huang, Kai]

On Thu, 2022-08-25 at 22:27 +0300, Jarkko Sakkinen wrote:
> On Thu, Aug 25, 2022 at 03:49:38PM +0000, Sean Christopherson wrote:
> > On Thu, Aug 25, 2022, Dave Hansen wrote:
> > > On 8/25/22 08:19, Sean Christopherson wrote:
> > > > > > This patch, along with your patch to expose AEX-notify attribute bit to
> > > > > > guest, have been tested that both AEX-notify and EDECCSSA work in the VM.
> > > > > > Feel free to merge this patch.
> > > > Dave, any objection to taking this through the KVM tree?
> > > 
> > > This specific patch?  Or are you talking about the couple of AEX-notify
> > > patches in their entirety?
> > 
> > I was thinking just this specific patch, but I temporarily forgot there are more
> > patches in flight.  It would be a bit odd to have effectively half of the AEX-notify
> > enabling go through KVM.
> > 
> > So with shortlog/changelog tweaks,
> > 
> > Acked-by: Sean Christopherson <seanjc@google.com>
> 
> with subsystem tag change (Sean's version):
> 
> Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> BR, Jarkko

Thanks.  Will send out a new version with updated title and changelog.

-- 
Thanks,
-Kai