Re: [PATCH 1/7] KVM: TDX: Add a place holder to handle TDX VM exit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/16/2024 8:54 AM, Binbin Wu wrote:



On 12/13/2024 4:57 PM, Xiaoyao Li wrote:
On 12/1/2024 11:53 AM, Binbin Wu wrote:
...

  }
    void tdx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, int pgd_level) @@ -1135,6 +1215,88 @@ int tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
      return tdx_sept_drop_private_spte(kvm, gfn, level, pfn);
  }
  +int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
+{
+    struct vcpu_tdx *tdx = to_tdx(vcpu);
+    u64 vp_enter_ret = tdx->vp_enter_ret;
+    union vmx_exit_reason exit_reason;
+
+    if (fastpath != EXIT_FASTPATH_NONE)
+        return 1;
+
+    /*
+     * Handle TDX SW errors, including TDX_SEAMCALL_UD, TDX_SEAMCALL_GP and
+     * TDX_SEAMCALL_VMFAILINVALID.
+     */
+    if (unlikely((vp_enter_ret & TDX_SW_ERROR) == TDX_SW_ERROR)) {
+        KVM_BUG_ON(!kvm_rebooting, vcpu->kvm);
+        goto unhandled_exit;
+    }
+
+    /*
+     * Without off-TD debug enabled, failed_vmentry case must have
+     * TDX_NON_RECOVERABLE set.
+     */

This comment is confusing. I'm not sure why it is put here. Below code does nothing with exit_reason.failed_vmentry.

Because when failed_vmentry occurs, vp_enter_ret will have
TDX_NON_RECOVERABLE set, so it will be handled below.

The words somehow is confusing, which to me is implying something like:

	WARN_ON(!debug_td() && exit_reason.failed_vmentry &&
                !(vp_enter_ret & TDX_NON_RECOVERABLE))

Besides, VMX returns KVM_EXIT_FAIL_ENTRY for vm-entry failure. So the question is why TDX cannot do it same way?


+    if (unlikely(vp_enter_ret & (TDX_ERROR | TDX_NON_RECOVERABLE))) {
+        /* Triple fault is non-recoverable. */
+        if (unlikely(tdx_check_exit_reason(vcpu, EXIT_REASON_TRIPLE_FAULT)))
+            return tdx_handle_triple_fault(vcpu);
+
+        kvm_pr_unimpl("TD vp_enter_ret 0x%llx, hkid 0x%x hkid pa 0x%llx\n",
+                  vp_enter_ret, to_kvm_tdx(vcpu->kvm)->hkid,
+                  set_hkid_to_hpa(0, to_kvm_tdx(vcpu->kvm)->hkid));

It indeed needs clarification for the need of "hkid" and "hkid pa". Especially the "hkdi pa", which is the result of applying HKID of the current TD to a physical address 0. I cannot think of any reason why we need such info.
Yes, set_hkid_to_hpa(0, to_kvm_tdx(vcpu->kvm)->hkid) should be removed.
I didn't notice it.

don't forget to justify why HKID is useful here. To me, HKID can be dropped as well.

Thanks!



+        goto unhandled_exit;
+    }
+
+    /* From now, the seamcall status should be TDX_SUCCESS. */
+    WARN_ON_ONCE((vp_enter_ret & TDX_SEAMCALL_STATUS_MASK) != TDX_SUCCESS);

Is there any case that TDX_SUCCESS with additional non-zero information in the lower 32-bits? I thought TDX_SUCCESS is a whole 64- bit status code.
TDX status code uses the upper 32-bits.

When the status code is TDX_SUCCESS and has a valid VMX exit reason, the lower
32-bit is the VMX exit reason.

You can refer to the TDX module ABI spec or interface_function_completion_status.json
from the intel-tdx-module-1.5-abi-table for details.


I see. (I asked a silly question that I even missed the normal Exit case)





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux