Re: [PATCH] kvm/x86: clear hlt for intel cpu when resetting vcpu

[Chao Gao <chao.gao@intel.com>]

On Fri, Jun 30, 2023 at 03:26:12PM +0800, Qi Ai wrote:
>--- a/arch/x86/include/asm/kvm_host.h
>+++ b/arch/x86/include/asm/kvm_host.h
>@@ -1731,6 +1731,8 @@ struct kvm_x86_ops {
> 	 * Returns vCPU specific APICv inhibit reasons
> 	 */
> 	unsigned long (*vcpu_get_apicv_inhibit_reasons)(struct kvm_vcpu *vcpu);
>+
>+	void (*clear_hlt)(struct kvm_vcpu *vcpu);

add this new ops to arch/x86/include/asm/kvm-x86-ops.h, and declare it
optional, i.e.,

KVM_X86_OP_OPTIONAL(...)


> };
> 
> struct kvm_x86_nested_ops {
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index 44fb619803b8..11c2fde1ad98 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -8266,6 +8266,8 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = {
> 	.complete_emulated_msr = kvm_complete_insn_gp,
> 
> 	.vcpu_deliver_sipi_vector = kvm_vcpu_deliver_sipi_vector,
>+
>+	.clear_hlt = vmx_clear_hlt,
> };
> 
> static unsigned int vmx_handle_intel_pt_intr(void)
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 7f70207e8689..21360f5ed006 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -11258,6 +11258,12 @@ static void __set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
> 	vcpu->arch.exception_vmexit.pending = false;
> 
> 	kvm_make_request(KVM_REQ_EVENT, vcpu);
>+
>+	if (kvm_x86_ops.clear_hlt) {
>+		if (kvm_vcpu_is_bsp(vcpu) && regs->rip == 0xfff0 &&

it is weird this applies to BSP only.

>+				!is_protmode(vcpu))
>+			kvm_x86_ops.clear_hlt(vcpu);

Use static_call_cond(kvm_x86_clear_hlt)(vcpu) instead.

It looks incorrect that we add this side-effect heuristically here. I
am wondering if we can link vcpu->arch.mp_state to VMCS activity state,
i.e., when mp_state is set to RUNNABLE in KVM_SET_MP_STATE ioctl, KVM
sets VMCS activity state to active.

>+	}
> }
> 
> int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
>-- 
>2.20.1
>