On 6/17/2022 8:15 PM, Shukla, Santosh wrote: > > > On 6/7/2022 6:37 PM, Maxim Levitsky wrote: >> On Thu, 2022-06-02 at 19:56 +0530, Santosh Shukla wrote: >>> VMCB intr_ctrl bit12 (V_NMI_MASK) is set by the processor when handling >>> NMI in guest and is cleared after the NMI is handled. Treat V_NMI_MASK as >>> read-only in the hypervisor and do not populate set accessors. >>> >>> Signed-off-by: Santosh Shukla <santosh.shukla@xxxxxxx> >>> --- >>> arch/x86/kvm/svm/svm.c | 20 +++++++++++++++++++- >>> 1 file changed, 19 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c >>> index 860f28c668bd..d67a54517d95 100644 >>> --- a/arch/x86/kvm/svm/svm.c >>> +++ b/arch/x86/kvm/svm/svm.c >>> @@ -323,6 +323,16 @@ static int is_external_interrupt(u32 info) >>> return info == (SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_INTR); >>> } >>> >>> +static bool is_vnmi_enabled(struct vmcb *vmcb) >>> +{ >>> + return vnmi && (vmcb->control.int_ctl & V_NMI_ENABLE); >>> +} >> >> Following Paolo's suggestion I recently removed vgif_enabled(), >> based on the logic that vgif_enabled == vgif, because >> we always enable vGIF for L1 as long as 'vgif' module param is set, >> which is set unless either hardware or user cleared it. >> > Yes. In v2, Thanks!. > >> Note that here vmcb is the current vmcb, which can be vmcb02, >> and it might be wrong >> >>> + >>> +static bool is_vnmi_mask_set(struct vmcb *vmcb) >>> +{ >>> + return !!(vmcb->control.int_ctl & V_NMI_MASK); >>> +} >>> + >>> static u32 svm_get_interrupt_shadow(struct kvm_vcpu *vcpu) >>> { >>> struct vcpu_svm *svm = to_svm(vcpu); >>> @@ -3502,13 +3512,21 @@ 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); >>> + struct vcpu_svm *svm = to_svm(vcpu); >>> + >>> + if (is_vnmi_enabled(svm->vmcb)) >>> + return is_vnmi_mask_set(svm->vmcb); >>> + else >>> + return !!(vcpu->arch.hflags & HF_NMI_MASK); >>> } >>> >>> static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked) >>> { >>> struct vcpu_svm *svm = to_svm(vcpu); >>> >>> + if (is_vnmi_enabled(svm->vmcb)) >>> + return; >> >> What if the KVM wants to mask NMI, shoudn't we update the >> V_NMI_MASK value in int_ctl instead of doing nothing? >> V_NMI_MASK is cpu controlled meaning HW sets the mask while processing event and clears right after processing, so in away its Read-only for hypervisor. >> Best regards, >> Maxim Levitsky >> >> >>> + >>> if (masked) { >>> vcpu->arch.hflags |= HF_NMI_MASK; >>> if (!sev_es_guest(vcpu->kvm)) >> >>