diff mbox series

[v2,3/8] KVM: TDX: Add a place holder for handler of TDX hypercalls (TDG.VP.VMCALL)

Message ID 20250211025442.3071607-4-binbin.wu@linux.intel.com (mailing list archive)
State New
Headers show
Series KVM: TDX: TDX hypercalls may exit to userspace | expand

Commit Message

Binbin Wu Feb. 11, 2025, 2:54 a.m. UTC 
From: Isaku Yamahata <isaku.yamahata@intel.com>

Add a place holder and related helper functions for preparation of
TDG.VP.VMCALL handling.

The TDX module specification defines TDG.VP.VMCALL API (TDVMCALL for short)
for the guest TD to call hypercall to VMM.  When the guest TD issues a
TDVMCALL, the guest TD exits to VMM with a new exit reason.  The arguments
from the guest TD and returned values from the VMM are passed in the guest
registers.  The guest RCX register indicates which registers are used.
Define helper functions to access those registers.

A new VMX exit reason TDCALL is added to indicate the exit is due to
TDVMCALL from the guest TD.  Define the TDCALL exit reason and add a place
holder to handle such exit.

Suggested-by: Sean Christopherson <seanjc@google.com>
Co-developed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Co-developed-by: Binbin Wu <binbin.wu@linux.intel.com>
Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
---
Hypercalls exit to userspace v2:
- Get/set tdvmcall inputs/outputs from/to vp_enter_args.
- Morph the guest requested exit reason (via TDVMCALL) to KVM's tracked
  exit reason when it could, i.e. when the TDVMCALL leaf number is less
  than 0x10000. (Sean)
- Drop helpers for read/write a0~a3.

Hypercalls exit to userspace v1:
- Update changelog.
- Drop the unused tdx->tdvmcall. (Chao)
- Use TDVMCALL_STATUS prefix for TDX call status codes (Binbin)
---
 arch/x86/include/uapi/asm/vmx.h |  4 ++-
 arch/x86/kvm/vmx/tdx.c          | 49 ++++++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 2 deletions(-)

Comments

Chao Gao Feb. 11, 2025, 8:41 a.m. UTC | #1 
>+static __always_inline unsigned long tdvmcall_exit_type(struct kvm_vcpu *vcpu)
>+{
>+	return to_tdx(vcpu)->vp_enter_args.r10;
>+}

please add a newline here.

>+static __always_inline unsigned long tdvmcall_leaf(struct kvm_vcpu *vcpu)
>+{
>+	return to_tdx(vcpu)->vp_enter_args.r11;
>+}

..

>+static __always_inline void tdvmcall_set_return_code(struct kvm_vcpu *vcpu,
>+						     long val)
>+{
>+	to_tdx(vcpu)->vp_enter_args.r10 = val;
>+}

ditto.

>+static __always_inline void tdvmcall_set_return_val(struct kvm_vcpu *vcpu,
>+						    unsigned long val)
>+{
>+	to_tdx(vcpu)->vp_enter_args.r11 = val;
>+}
>+
> static inline void tdx_hkid_free(struct kvm_tdx *kvm_tdx)
> {
> 	tdx_guest_keyid_free(kvm_tdx->hkid);
>@@ -810,6 +829,7 @@ static bool tdx_guest_state_is_invalid(struct kvm_vcpu *vcpu)
> static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
> {
> 	struct vcpu_tdx *tdx = to_tdx(vcpu);
>+	u32 exit_reason;
> 
> 	switch (tdx->vp_enter_ret & TDX_SEAMCALL_STATUS_MASK) {
> 	case TDX_SUCCESS:
>@@ -822,7 +842,21 @@ static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
> 		return -1u;
> 	}
> 
>-	return tdx->vp_enter_ret;
>+	exit_reason = tdx->vp_enter_ret;
>+
>+	switch (exit_reason) {
>+	case EXIT_REASON_TDCALL:
>+		if (tdvmcall_exit_type(vcpu))
>+			return EXIT_REASON_VMCALL;
>+
>+		if (tdvmcall_leaf(vcpu) < 0x10000)

Can you add a comment for the hard-coded 0x10000?

I am wondering what would happen if the guest tries to make a tdvmcall with
leaf=0 or leaf=1 to mislead KVM into calling the NMI/interrupt handling
routine. Would it trigger the unknown NMI warning or effectively inject an
interrupt into the host?

I think we should do the conversion for leafs that are defined in the current
GHCI spec.

>+			return tdvmcall_leaf(vcpu);
>+		break;
>+	default:
>+		break;
>+	}
>+
>+	return exit_reason;
> }
Binbin Wu Feb. 11, 2025, 9:08 a.m. UTC | #2 
On 2/11/2025 4:41 PM, Chao Gao wrote:
>> +static __always_inline unsigned long tdvmcall_exit_type(struct kvm_vcpu *vcpu)
>> +{
>> +	return to_tdx(vcpu)->vp_enter_args.r10;
>> +}
> please add a newline here.
>
>> +static __always_inline unsigned long tdvmcall_leaf(struct kvm_vcpu *vcpu)
>> +{
>> +	return to_tdx(vcpu)->vp_enter_args.r11;
>> +}
> ..
>
>> +static __always_inline void tdvmcall_set_return_code(struct kvm_vcpu *vcpu,
>> +						     long val)
>> +{
>> +	to_tdx(vcpu)->vp_enter_args.r10 = val;
>> +}
> ditto.
>
>> +static __always_inline void tdvmcall_set_return_val(struct kvm_vcpu *vcpu,
>> +						    unsigned long val)
>> +{
>> +	to_tdx(vcpu)->vp_enter_args.r11 = val;
>> +}
>> +
>> static inline void tdx_hkid_free(struct kvm_tdx *kvm_tdx)
>> {
>> 	tdx_guest_keyid_free(kvm_tdx->hkid);
>> @@ -810,6 +829,7 @@ static bool tdx_guest_state_is_invalid(struct kvm_vcpu *vcpu)
>> static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
>> {
>> 	struct vcpu_tdx *tdx = to_tdx(vcpu);
>> +	u32 exit_reason;
>>
>> 	switch (tdx->vp_enter_ret & TDX_SEAMCALL_STATUS_MASK) {
>> 	case TDX_SUCCESS:
>> @@ -822,7 +842,21 @@ static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
>> 		return -1u;
>> 	}
>>
>> -	return tdx->vp_enter_ret;
>> +	exit_reason = tdx->vp_enter_ret;
>> +
>> +	switch (exit_reason) {
>> +	case EXIT_REASON_TDCALL:
>> +		if (tdvmcall_exit_type(vcpu))
>> +			return EXIT_REASON_VMCALL;
>> +
>> +		if (tdvmcall_leaf(vcpu) < 0x10000)
> Can you add a comment for the hard-coded 0x10000?
>
> I am wondering what would happen if the guest tries to make a tdvmcall with
> leaf=0 or leaf=1 to mislead KVM into calling the NMI/interrupt handling
> routine. Would it trigger the unknown NMI warning or effectively inject an
> interrupt into the host?
Oh, yes, it's possible.

>
> I think we should do the conversion for leafs that are defined in the current
> GHCI spec.
Yes, it should be limited to the supported leaves defined in the GHCI.
Thanks for pointing it out!

>
>> +			return tdvmcall_leaf(vcpu);
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return exit_reason;
>> }
Sean Christopherson Feb. 11, 2025, 11:46 p.m. UTC | #3 
On Tue, Feb 11, 2025, Chao Gao wrote:
> >@@ -810,6 +829,7 @@ static bool tdx_guest_state_is_invalid(struct kvm_vcpu *vcpu)
> > static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
> > {
> > 	struct vcpu_tdx *tdx = to_tdx(vcpu);
> >+	u32 exit_reason;
> > 
> > 	switch (tdx->vp_enter_ret & TDX_SEAMCALL_STATUS_MASK) {
> > 	case TDX_SUCCESS:
> >@@ -822,7 +842,21 @@ static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
> > 		return -1u;
> > 	}
> > 
> >-	return tdx->vp_enter_ret;
> >+	exit_reason = tdx->vp_enter_ret;
> >+
> >+	switch (exit_reason) {
> >+	case EXIT_REASON_TDCALL:
> >+		if (tdvmcall_exit_type(vcpu))
> >+			return EXIT_REASON_VMCALL;
> >+
> >+		if (tdvmcall_leaf(vcpu) < 0x10000)
> 
> Can you add a comment for the hard-coded 0x10000?

Or better yet, a #define of some kind (with a comment ;-) ).
Binbin Wu Feb. 12, 2025, 2:21 a.m. UTC | #4 
On 2/12/2025 7:46 AM, Sean Christopherson wrote:
> On Tue, Feb 11, 2025, Chao Gao wrote:
>>> @@ -810,6 +829,7 @@ static bool tdx_guest_state_is_invalid(struct kvm_vcpu *vcpu)
>>> static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
>>> {
>>> 	struct vcpu_tdx *tdx = to_tdx(vcpu);
>>> +	u32 exit_reason;
>>>
>>> 	switch (tdx->vp_enter_ret & TDX_SEAMCALL_STATUS_MASK) {
>>> 	case TDX_SUCCESS:
>>> @@ -822,7 +842,21 @@ static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
>>> 		return -1u;
>>> 	}
>>>
>>> -	return tdx->vp_enter_ret;
>>> +	exit_reason = tdx->vp_enter_ret;
>>> +
>>> +	switch (exit_reason) {
>>> +	case EXIT_REASON_TDCALL:
>>> +		if (tdvmcall_exit_type(vcpu))
>>> +			return EXIT_REASON_VMCALL;
>>> +
>>> +		if (tdvmcall_leaf(vcpu) < 0x10000)
>> Can you add a comment for the hard-coded 0x10000?
> Or better yet, a #define of some kind (with a comment ;-) ).

As Chao pointed out, we should convert the leaves defined in the GHCI spec
and supported in KVM only.  Specific leaf numbers will be used instead of
comparing to 0x10000.

I plan to change it to:

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 2b24f50ad0ee..af8276402212 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -920,11 +920,17 @@ static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
                 if (tdvmcall_exit_type(vcpu))
                         return EXIT_REASON_VMCALL;

-               if (tdvmcall_leaf(vcpu) < 0x10000) {
-                       if (tdvmcall_leaf(vcpu) == EXIT_REASON_EPT_VIOLATION)
+               switch(tdvmcall_leaf(vcpu)) {
+                       case EXIT_REASON_EPT_VIOLATION:
                                 return EXIT_REASON_EPT_MISCONFIG;
-
-                       return tdvmcall_leaf(vcpu);
+                       case EXIT_REASON_CPUID:
+                       case EXIT_REASON_HLT:
+                       case EXIT_REASON_IO_INSTRUCTION:
+                       case EXIT_REASON_MSR_READ:
+                       case EXIT_REASON_MSR_WRITE:
+                               return tdvmcall_leaf(vcpu);
+                       default:
+                               break;
                 }
                 break;
         case EXIT_REASON_EPT_MISCONFIG:
diff mbox series

Patch

diff --git a/arch/x86/include/uapi/asm/vmx.h b/arch/x86/include/uapi/asm/vmx.h
index a5faf6d88f1b..6a9f268a2d2c 100644
--- a/arch/x86/include/uapi/asm/vmx.h
+++ b/arch/x86/include/uapi/asm/vmx.h
@@ -92,6 +92,7 @@ 
 #define EXIT_REASON_TPAUSE              68
 #define EXIT_REASON_BUS_LOCK            74
 #define EXIT_REASON_NOTIFY              75
+#define EXIT_REASON_TDCALL              77
 
 #define VMX_EXIT_REASONS \
 	{ EXIT_REASON_EXCEPTION_NMI,         "EXCEPTION_NMI" }, \
@@ -155,7 +156,8 @@ 
 	{ EXIT_REASON_UMWAIT,                "UMWAIT" }, \
 	{ EXIT_REASON_TPAUSE,                "TPAUSE" }, \
 	{ EXIT_REASON_BUS_LOCK,              "BUS_LOCK" }, \
-	{ EXIT_REASON_NOTIFY,                "NOTIFY" }
+	{ EXIT_REASON_NOTIFY,                "NOTIFY" }, \
+	{ EXIT_REASON_TDCALL,                "TDCALL" }
 
 #define VMX_EXIT_REASON_FLAGS \
 	{ VMX_EXIT_REASONS_FAILED_VMENTRY,	"FAILED_VMENTRY" }
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index cb64675e6ad9..420ee492e919 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -235,6 +235,25 @@  static bool tdx_operand_busy(u64 err)
  */
 static DEFINE_PER_CPU(struct list_head, associated_tdvcpus);
 
+static __always_inline unsigned long tdvmcall_exit_type(struct kvm_vcpu *vcpu)
+{
+	return to_tdx(vcpu)->vp_enter_args.r10;
+}
+static __always_inline unsigned long tdvmcall_leaf(struct kvm_vcpu *vcpu)
+{
+	return to_tdx(vcpu)->vp_enter_args.r11;
+}
+static __always_inline void tdvmcall_set_return_code(struct kvm_vcpu *vcpu,
+						     long val)
+{
+	to_tdx(vcpu)->vp_enter_args.r10 = val;
+}
+static __always_inline void tdvmcall_set_return_val(struct kvm_vcpu *vcpu,
+						    unsigned long val)
+{
+	to_tdx(vcpu)->vp_enter_args.r11 = val;
+}
+
 static inline void tdx_hkid_free(struct kvm_tdx *kvm_tdx)
 {
 	tdx_guest_keyid_free(kvm_tdx->hkid);
@@ -810,6 +829,7 @@  static bool tdx_guest_state_is_invalid(struct kvm_vcpu *vcpu)
 static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_tdx *tdx = to_tdx(vcpu);
+	u32 exit_reason;
 
 	switch (tdx->vp_enter_ret & TDX_SEAMCALL_STATUS_MASK) {
 	case TDX_SUCCESS:
@@ -822,7 +842,21 @@  static __always_inline u32 tdx_to_vmx_exit_reason(struct kvm_vcpu *vcpu)
 		return -1u;
 	}
 
-	return tdx->vp_enter_ret;
+	exit_reason = tdx->vp_enter_ret;
+
+	switch (exit_reason) {
+	case EXIT_REASON_TDCALL:
+		if (tdvmcall_exit_type(vcpu))
+			return EXIT_REASON_VMCALL;
+
+		if (tdvmcall_leaf(vcpu) < 0x10000)
+			return tdvmcall_leaf(vcpu);
+		break;
+	default:
+		break;
+	}
+
+	return exit_reason;
 }
 
 static noinstr void tdx_vcpu_enter_exit(struct kvm_vcpu *vcpu)
@@ -930,6 +964,17 @@  fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu, bool force_immediate_exit)
 	return tdx_exit_handlers_fastpath(vcpu);
 }
 
+static int handle_tdvmcall(struct kvm_vcpu *vcpu)
+{
+	switch (tdvmcall_leaf(vcpu)) {
+	default:
+		break;
+	}
+
+	tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_INVALID_OPERAND);
+	return 1;
+}
+
 void tdx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, int pgd_level)
 {
 	u64 shared_bit = (pgd_level == 5) ? TDX_SHARED_BIT_PWL_5 :
@@ -1262,6 +1307,8 @@  int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
 		vcpu->run->exit_reason = KVM_EXIT_SHUTDOWN;
 		vcpu->mmio_needed = 0;
 		return 0;
+	case EXIT_REASON_TDCALL:
+		return handle_tdvmcall(vcpu);
 	default:
 		break;
 	}