On Mon, Feb 26, 2024 at 12:26:50AM -0800, isaku.yamahata@xxxxxxxxx wrote:
>From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>
>Wire up TDX PV HLT hypercall to the KVM backend function.
>
>Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>---
>v19:
>- move tdvps_state_non_arch_check() to this patch
>
>v18:
>- drop buggy_hlt_workaround and use TDH.VP.RD(TD_VCPU_STATE_DETAILS)
>
>Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>---
> arch/x86/kvm/vmx/tdx.c | 26 +++++++++++++++++++++++++-
> arch/x86/kvm/vmx/tdx.h | 4 ++++
> 2 files changed, 29 insertions(+), 1 deletion(-)
>
>diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>index eb68d6c148b6..a2caf2ae838c 100644
>--- a/arch/x86/kvm/vmx/tdx.c
>+++ b/arch/x86/kvm/vmx/tdx.c
>@@ -688,7 +688,18 @@ void tdx_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>
> bool tdx_protected_apic_has_interrupt(struct kvm_vcpu *vcpu)
> {
>- return pi_has_pending_interrupt(vcpu);
>+ bool ret = pi_has_pending_interrupt(vcpu);
Maybe
bool has_pending_interrupt = pi_has_pending_interrupt(vcpu);
"ret" isn't a good name. or even call pi_has_pending_interrupt() directly in
the if statement below.
>+ union tdx_vcpu_state_details details;
>+ struct vcpu_tdx *tdx = to_tdx(vcpu);
>+
>+ if (ret || vcpu->arch.mp_state != KVM_MP_STATE_HALTED)
>+ return true;
Question: why mp_state matters here?
>+
>+ if (tdx->interrupt_disabled_hlt)
>+ return false;
Shouldn't we move this into vt_interrupt_allowed()? VMX calls the function to
check if interrupt is disabled. KVM can clear tdx->interrupt_disabled_hlt on
every TD-enter and set it only on TD-exit due to the guest making a
TDVMCALL(hlt) w/ interrupt disabled.
>+
>+ details.full = td_state_non_arch_read64(tdx, TD_VCPU_STATE_DETAILS_NON_ARCH);
>+ return !!details.vmxip;
> }
>
> void tdx_prepare_switch_to_guest(struct kvm_vcpu *vcpu)
>@@ -1130,6 +1141,17 @@ static int tdx_emulate_cpuid(struct kvm_vcpu *vcpu)
> return 1;
> }
>
>+static int tdx_emulate_hlt(struct kvm_vcpu *vcpu)
>+{
>+ struct vcpu_tdx *tdx = to_tdx(vcpu);
>+
>+ /* See tdx_protected_apic_has_interrupt() to avoid heavy seamcall */
>+ tdx->interrupt_disabled_hlt = tdvmcall_a0_read(vcpu);
>+
>+ tdvmcall_set_return_code(vcpu, TDVMCALL_SUCCESS);
>+ return kvm_emulate_halt_noskip(vcpu);
>+}
>+
> static int handle_tdvmcall(struct kvm_vcpu *vcpu)
> {
> if (tdvmcall_exit_type(vcpu))
>@@ -1138,6 +1160,8 @@ static int handle_tdvmcall(struct kvm_vcpu *vcpu)
> switch (tdvmcall_leaf(vcpu)) {
> case EXIT_REASON_CPUID:
> return tdx_emulate_cpuid(vcpu);
>+ case EXIT_REASON_HLT:
>+ return tdx_emulate_hlt(vcpu);
> default:
> break;
> }
>diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h
>index 4399d474764f..11c74c34555f 100644
>--- a/arch/x86/kvm/vmx/tdx.h
>+++ b/arch/x86/kvm/vmx/tdx.h
>@@ -104,6 +104,8 @@ struct vcpu_tdx {
> bool host_state_need_restore;
> u64 msr_host_kernel_gs_base;
>
>+ bool interrupt_disabled_hlt;
>+
> /*
> * Dummy to make pmu_intel not corrupt memory.
> * TODO: Support PMU for TDX. Future work.
>@@ -166,6 +168,7 @@ static __always_inline void tdvps_vmcs_check(u32 field, u8 bits)
> }
>
> static __always_inline void tdvps_management_check(u64 field, u8 bits) {}
>+static __always_inline void tdvps_state_non_arch_check(u64 field, u8 bits) {}
>
> #define TDX_BUILD_TDVPS_ACCESSORS(bits, uclass, lclass) \
> static __always_inline u##bits td_##lclass##_read##bits(struct vcpu_tdx *tdx, \
>@@ -226,6 +229,7 @@ TDX_BUILD_TDVPS_ACCESSORS(32, VMCS, vmcs);
> TDX_BUILD_TDVPS_ACCESSORS(64, VMCS, vmcs);
>
> TDX_BUILD_TDVPS_ACCESSORS(8, MANAGEMENT, management);
>+TDX_BUILD_TDVPS_ACCESSORS(64, STATE_NON_ARCH, state_non_arch);
>
> static __always_inline u64 td_tdcs_exec_read64(struct kvm_tdx *kvm_tdx, u32 field)
> {
>--
>2.25.1
>
>
