diff mbox series

[v2,04/11] KVM: SVM: drop the SVM specific H_FLAGS

Message ID 20221129193717.513824-5-mlevitsk@redhat.com (mailing list archive)
State New, archived
Headers show
Series SVM: vNMI (with my fixes) | expand

Commit Message

Maxim Levitsky Nov. 29, 2022, 7:37 p.m. UTC 
GIF and 'waiting for IRET' are used only for the SVM and thus should
not be in H_FLAGS.

NMI mask is not x86 specific but it is only used for SVM without vNMI.

The VMX have similar concept of NMI mask (soft_vnmi_blocked),
and it is used when its 'vNMI' feature is not enabled,
but because the VMX can't intercept IRET, it is more of a hack,
and thus should not use common host flags either.

No functional change is intended.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  3 ---
 arch/x86/kvm/svm/svm.c          | 22 +++++++++++++---------
 arch/x86/kvm/svm/svm.h          | 25 ++++++++++++++++++++++---
 3 files changed, 35 insertions(+), 15 deletions(-)

Comments

Shukla, Santosh Dec. 5, 2022, 3:31 p.m. UTC | #1 
On 11/30/2022 1:07 AM, Maxim Levitsky wrote:
> GIF and 'waiting for IRET' are used only for the SVM and thus should
> not be in H_FLAGS.
> 
> NMI mask is not x86 specific but it is only used for SVM without vNMI.
> 
> The VMX have similar concept of NMI mask (soft_vnmi_blocked),
> and it is used when its 'vNMI' feature is not enabled,
> but because the VMX can't intercept IRET, it is more of a hack,
> and thus should not use common host flags either.
> 
> No functional change is intended.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  3 ---
>  arch/x86/kvm/svm/svm.c          | 22 +++++++++++++---------
>  arch/x86/kvm/svm/svm.h          | 25 ++++++++++++++++++++++---
>  3 files changed, 35 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 70af7240a1d5af..9208ad7a6bd004 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -2052,9 +2052,6 @@ enum {
>  	TASK_SWITCH_GATE = 3,
>  };
>  
> -#define HF_GIF_MASK		(1 << 0)
> -#define HF_NMI_MASK		(1 << 3)
> -#define HF_IRET_MASK		(1 << 4)
>  #define HF_GUEST_MASK		(1 << 5) /* VCPU is in guest-mode */
>  
>  #ifdef CONFIG_KVM_SMM
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 91352d69284524..512b2aa21137e2 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -1326,6 +1326,9 @@ static void __svm_vcpu_reset(struct kvm_vcpu *vcpu)
>  	vcpu->arch.microcode_version = 0x01000065;
>  	svm->tsc_ratio_msr = kvm_caps.default_tsc_scaling_ratio;
>  
> +	svm->nmi_masked = false;
> +	svm->awaiting_iret_completion = false;
> +
>  	if (sev_es_guest(vcpu->kvm))
>  		sev_es_vcpu_reset(svm);
>  }
> @@ -2470,7 +2473,7 @@ static int iret_interception(struct kvm_vcpu *vcpu)
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  
>  	++vcpu->stat.nmi_window_exits;
> -	vcpu->arch.hflags |= HF_IRET_MASK;
> +	svm->awaiting_iret_completion = true;
>  	if (!sev_es_guest(vcpu->kvm)) {
>  		svm_clr_intercept(svm, INTERCEPT_IRET);
>  		svm->nmi_iret_rip = kvm_rip_read(vcpu);
> @@ -3466,7 +3469,7 @@ static void svm_inject_nmi(struct kvm_vcpu *vcpu)
>  	if (svm->nmi_l1_to_l2)
>  		return;
>  
> -	vcpu->arch.hflags |= HF_NMI_MASK;
> +	svm->nmi_masked = true;
>  	if (!sev_es_guest(vcpu->kvm))
>  		svm_set_intercept(svm, INTERCEPT_IRET);
>  	++vcpu->stat.nmi_injections;
> @@ -3580,7 +3583,7 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu)
>  		return false;
>  
>  	ret = (vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) ||
> -	      (vcpu->arch.hflags & HF_NMI_MASK);
> +	      (svm->nmi_masked);
>  
>  	return ret;
>  }
> @@ -3602,7 +3605,7 @@ static int svm_nmi_allowed(struct kvm_vcpu *vcpu, bool for_injection)
>  
>  static bool svm_get_nmi_mask(struct kvm_vcpu *vcpu)
>  {
> -	return !!(vcpu->arch.hflags & HF_NMI_MASK);
> +	return to_svm(vcpu)->nmi_masked;
>  }
>  
>  static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked)
> @@ -3610,11 +3613,11 @@ static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked)
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  
>  	if (masked) {
> -		vcpu->arch.hflags |= HF_NMI_MASK;
> +		svm->nmi_masked = true;
>  		if (!sev_es_guest(vcpu->kvm))
>  			svm_set_intercept(svm, INTERCEPT_IRET);
>  	} else {
> -		vcpu->arch.hflags &= ~HF_NMI_MASK;
> +		svm->nmi_masked = false;
>  		if (!sev_es_guest(vcpu->kvm))
>  			svm_clr_intercept(svm, INTERCEPT_IRET);
>  	}
> @@ -3700,7 +3703,7 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  
> -	if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK)
> +	if (svm->nmi_masked && !svm->awaiting_iret_completion)
>  		return; /* IRET will cause a vm exit */
>  
>  	if (!gif_set(svm)) {
> @@ -3824,10 +3827,11 @@ static void svm_complete_interrupts(struct kvm_vcpu *vcpu)
>  	 * If we've made progress since setting HF_IRET_MASK, we've
>  	 * executed an IRET and can allow NMI injection.
>  	 */
> -	if ((vcpu->arch.hflags & HF_IRET_MASK) &&
> +	if (svm->awaiting_iret_completion &&
>  	    (sev_es_guest(vcpu->kvm) ||
>  	     kvm_rip_read(vcpu) != svm->nmi_iret_rip)) {
> -		vcpu->arch.hflags &= ~(HF_NMI_MASK | HF_IRET_MASK);
> +		svm->awaiting_iret_completion = false;
> +		svm->nmi_masked = false;
>  		kvm_make_request(KVM_REQ_EVENT, vcpu);
>  	}
>  
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 4826e6cc611bf1..587ddc150f9f34 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -237,8 +237,24 @@ struct vcpu_svm {
>  
>  	struct svm_nested_state nested;
>  
> +	/* NMI mask value, used when vNMI is not enabled */
> +	bool nmi_masked;
> +
> +	/*
> +	 * True when the NMI still masked but guest IRET was just intercepted
> +	 * and KVM is waiting for RIP change which will signal that this IRET was
> +	 * retired and thus NMI can be unmasked.
> +	 */
> +	bool awaiting_iret_completion;
> +
> +	/*
> +	 * Set when KVM waits for IRET completion and needs to
> +	 * inject NMIs as soon as it completes (e.g NMI is pending injection).
> +	 * The KVM takes over EFLAGS.TF for this.
> +	 */
>  	bool nmi_singlestep;
>  	u64 nmi_singlestep_guest_rflags;
> +
^^^ blank line.

Thanks,
Santosh
>  	bool nmi_l1_to_l2;
>  
>  	unsigned long soft_int_csbase;
> @@ -280,6 +296,9 @@ struct vcpu_svm {
>  	bool guest_state_loaded;
>  
>  	bool x2avic_msrs_intercepted;
> +
> +	/* Guest GIF value which is used when vGIF is not enabled */
> +	bool gif_value;
>  };
>  
>  struct svm_cpu_data {
> @@ -497,7 +516,7 @@ static inline void enable_gif(struct vcpu_svm *svm)
>  	if (vmcb)
>  		vmcb->control.int_ctl |= V_GIF_MASK;
>  	else
> -		svm->vcpu.arch.hflags |= HF_GIF_MASK;
> +		svm->gif_value = true;
>  }
>  
>  static inline void disable_gif(struct vcpu_svm *svm)
> @@ -507,7 +526,7 @@ static inline void disable_gif(struct vcpu_svm *svm)
>  	if (vmcb)
>  		vmcb->control.int_ctl &= ~V_GIF_MASK;
>  	else
> -		svm->vcpu.arch.hflags &= ~HF_GIF_MASK;
> +		svm->gif_value = false;
>  }
>  
>  static inline bool gif_set(struct vcpu_svm *svm)
> @@ -517,7 +536,7 @@ static inline bool gif_set(struct vcpu_svm *svm)
>  	if (vmcb)
>  		return !!(vmcb->control.int_ctl & V_GIF_MASK);
>  	else
> -		return !!(svm->vcpu.arch.hflags & HF_GIF_MASK);
> +		return svm->gif_value;
>  }
>  
>  static inline bool nested_npt_enabled(struct vcpu_svm *svm)
Sean Christopherson Jan. 28, 2023, 12:56 a.m. UTC | #2 
On Tue, Nov 29, 2022, Maxim Levitsky wrote:
> @@ -3580,7 +3583,7 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu)
>  		return false;
>  
>  	ret = (vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) ||
> -	      (vcpu->arch.hflags & HF_NMI_MASK);
> +	      (svm->nmi_masked);

Unnecessary parantheses.

>  
>  	return ret;
>  }
diff mbox series

Patch

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 70af7240a1d5af..9208ad7a6bd004 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2052,9 +2052,6 @@  enum {
 	TASK_SWITCH_GATE = 3,
 };
 
-#define HF_GIF_MASK		(1 << 0)
-#define HF_NMI_MASK		(1 << 3)
-#define HF_IRET_MASK		(1 << 4)
 #define HF_GUEST_MASK		(1 << 5) /* VCPU is in guest-mode */
 
 #ifdef CONFIG_KVM_SMM
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 91352d69284524..512b2aa21137e2 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1326,6 +1326,9 @@  static void __svm_vcpu_reset(struct kvm_vcpu *vcpu)
 	vcpu->arch.microcode_version = 0x01000065;
 	svm->tsc_ratio_msr = kvm_caps.default_tsc_scaling_ratio;
 
+	svm->nmi_masked = false;
+	svm->awaiting_iret_completion = false;
+
 	if (sev_es_guest(vcpu->kvm))
 		sev_es_vcpu_reset(svm);
 }
@@ -2470,7 +2473,7 @@  static int iret_interception(struct kvm_vcpu *vcpu)
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 	++vcpu->stat.nmi_window_exits;
-	vcpu->arch.hflags |= HF_IRET_MASK;
+	svm->awaiting_iret_completion = true;
 	if (!sev_es_guest(vcpu->kvm)) {
 		svm_clr_intercept(svm, INTERCEPT_IRET);
 		svm->nmi_iret_rip = kvm_rip_read(vcpu);
@@ -3466,7 +3469,7 @@  static void svm_inject_nmi(struct kvm_vcpu *vcpu)
 	if (svm->nmi_l1_to_l2)
 		return;
 
-	vcpu->arch.hflags |= HF_NMI_MASK;
+	svm->nmi_masked = true;
 	if (!sev_es_guest(vcpu->kvm))
 		svm_set_intercept(svm, INTERCEPT_IRET);
 	++vcpu->stat.nmi_injections;
@@ -3580,7 +3583,7 @@  bool svm_nmi_blocked(struct kvm_vcpu *vcpu)
 		return false;
 
 	ret = (vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) ||
-	      (vcpu->arch.hflags & HF_NMI_MASK);
+	      (svm->nmi_masked);
 
 	return ret;
 }
@@ -3602,7 +3605,7 @@  static int svm_nmi_allowed(struct kvm_vcpu *vcpu, bool for_injection)
 
 static bool svm_get_nmi_mask(struct kvm_vcpu *vcpu)
 {
-	return !!(vcpu->arch.hflags & HF_NMI_MASK);
+	return to_svm(vcpu)->nmi_masked;
 }
 
 static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked)
@@ -3610,11 +3613,11 @@  static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked)
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 	if (masked) {
-		vcpu->arch.hflags |= HF_NMI_MASK;
+		svm->nmi_masked = true;
 		if (!sev_es_guest(vcpu->kvm))
 			svm_set_intercept(svm, INTERCEPT_IRET);
 	} else {
-		vcpu->arch.hflags &= ~HF_NMI_MASK;
+		svm->nmi_masked = false;
 		if (!sev_es_guest(vcpu->kvm))
 			svm_clr_intercept(svm, INTERCEPT_IRET);
 	}
@@ -3700,7 +3703,7 @@  static void svm_enable_nmi_window(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
-	if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK)
+	if (svm->nmi_masked && !svm->awaiting_iret_completion)
 		return; /* IRET will cause a vm exit */
 
 	if (!gif_set(svm)) {
@@ -3824,10 +3827,11 @@  static void svm_complete_interrupts(struct kvm_vcpu *vcpu)
 	 * If we've made progress since setting HF_IRET_MASK, we've
 	 * executed an IRET and can allow NMI injection.
 	 */
-	if ((vcpu->arch.hflags & HF_IRET_MASK) &&
+	if (svm->awaiting_iret_completion &&
 	    (sev_es_guest(vcpu->kvm) ||
 	     kvm_rip_read(vcpu) != svm->nmi_iret_rip)) {
-		vcpu->arch.hflags &= ~(HF_NMI_MASK | HF_IRET_MASK);
+		svm->awaiting_iret_completion = false;
+		svm->nmi_masked = false;
 		kvm_make_request(KVM_REQ_EVENT, vcpu);
 	}
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 4826e6cc611bf1..587ddc150f9f34 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -237,8 +237,24 @@  struct vcpu_svm {
 
 	struct svm_nested_state nested;
 
+	/* NMI mask value, used when vNMI is not enabled */
+	bool nmi_masked;
+
+	/*
+	 * True when the NMI still masked but guest IRET was just intercepted
+	 * and KVM is waiting for RIP change which will signal that this IRET was
+	 * retired and thus NMI can be unmasked.
+	 */
+	bool awaiting_iret_completion;
+
+	/*
+	 * Set when KVM waits for IRET completion and needs to
+	 * inject NMIs as soon as it completes (e.g NMI is pending injection).
+	 * The KVM takes over EFLAGS.TF for this.
+	 */
 	bool nmi_singlestep;
 	u64 nmi_singlestep_guest_rflags;
+
 	bool nmi_l1_to_l2;
 
 	unsigned long soft_int_csbase;
@@ -280,6 +296,9 @@  struct vcpu_svm {
 	bool guest_state_loaded;
 
 	bool x2avic_msrs_intercepted;
+
+	/* Guest GIF value which is used when vGIF is not enabled */
+	bool gif_value;
 };
 
 struct svm_cpu_data {
@@ -497,7 +516,7 @@  static inline void enable_gif(struct vcpu_svm *svm)
 	if (vmcb)
 		vmcb->control.int_ctl |= V_GIF_MASK;
 	else
-		svm->vcpu.arch.hflags |= HF_GIF_MASK;
+		svm->gif_value = true;
 }
 
 static inline void disable_gif(struct vcpu_svm *svm)
@@ -507,7 +526,7 @@  static inline void disable_gif(struct vcpu_svm *svm)
 	if (vmcb)
 		vmcb->control.int_ctl &= ~V_GIF_MASK;
 	else
-		svm->vcpu.arch.hflags &= ~HF_GIF_MASK;
+		svm->gif_value = false;
 }
 
 static inline bool gif_set(struct vcpu_svm *svm)
@@ -517,7 +536,7 @@  static inline bool gif_set(struct vcpu_svm *svm)
 	if (vmcb)
 		return !!(vmcb->control.int_ctl & V_GIF_MASK);
 	else
-		return !!(svm->vcpu.arch.hflags & HF_GIF_MASK);
+		return svm->gif_value;
 }
 
 static inline bool nested_npt_enabled(struct vcpu_svm *svm)