2022-06-02 16:09:11

by Santosh Shukla

[permalink] [raw]
Subject: [PATCH 3/7] KVM: SVM: Add VNMI support in get/set_nmi_mask

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 <[email protected]>
---
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);
+}
+
+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;
+
if (masked) {
vcpu->arch.hflags |= HF_NMI_MASK;
if (!sev_es_guest(vcpu->kvm))
--
2.25.1



2022-06-07 18:03:35

by Maxim Levitsky

[permalink] [raw]
Subject: Re: [PATCH 3/7] KVM: SVM: Add VNMI support in get/set_nmi_mask

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 <[email protected]>
> ---
>  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.

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?

Best regards,
Maxim Levitsky


> +
>         if (masked) {
>                 vcpu->arch.hflags |= HF_NMI_MASK;
>                 if (!sev_es_guest(vcpu->kvm))


2022-06-17 15:21:53

by Santosh Shukla

[permalink] [raw]
Subject: Re: [PATCH 3/7] KVM: SVM: Add VNMI support in get/set_nmi_mask



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 <[email protected]>
>> ---
>>  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?
>
> Best regards,
> Maxim Levitsky
>
>
>> +
>>         if (masked) {
>>                 vcpu->arch.hflags |= HF_NMI_MASK;
>>                 if (!sev_es_guest(vcpu->kvm))
>
>

2022-06-17 15:23:17

by Santosh Shukla

[permalink] [raw]
Subject: Re: [PATCH 3/7] KVM: SVM: Add VNMI support in get/set_nmi_mask



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 <[email protected]>
>>> ---
>>>  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))
>>
>>

2022-07-10 16:44:37

by Maxim Levitsky

[permalink] [raw]
Subject: Re: [PATCH 3/7] KVM: SVM: Add VNMI support in get/set_nmi_mask

On Fri, 2022-06-17 at 20:18 +0530, Shukla, Santosh wrote:
>
> 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 <[email protected]>
> > > > ---
> > > > 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.

And yet, svm_set_nmi_mask is called when KVM wants to explicitly mask NMI
without injecting a NMI, it does this when entering (emulated) SMI.

So the KVM has to set V_NMI_MASK here, becaue no real NMI is injected,
and thus the CPU will not set this bit itself.

Best regards,
Maxim Levitsky
>
> > > Best regards,
> > > Maxim Levitsky
> > >
> > >
> > > > +
> > > > if (masked) {
> > > > vcpu->arch.hflags |= HF_NMI_MASK;
> > > > if (!sev_es_guest(vcpu->kvm))


2022-07-10 18:47:03

by Maxim Levitsky

[permalink] [raw]
Subject: Re: [PATCH 3/7] KVM: SVM: Add VNMI support in get/set_nmi_mask

On Fri, 2022-06-17 at 20:18 +0530, Shukla, Santosh wrote:
>
> 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 <[email protected]>
> > > > ---
> > > > 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.

And yet, svm_set_nmi_mask is called when KVM wants to explicitly mask NMI
without injecting a NMI, it does this when entering (emulated) SMI.

So the KVM has to set V_NMI_MASK here, even though no real NMI was received.

Best regards,
Maxim Levitsky
>
> > > Best regards,
> > > Maxim Levitsky
> > >
> > >
> > > > +
> > > > if (masked) {
> > > > vcpu->arch.hflags |= HF_NMI_MASK;
> > > > if (!sev_es_guest(vcpu->kvm))


2022-07-21 09:51:13

by Santosh Shukla

[permalink] [raw]
Subject: Re: [PATCH 3/7] KVM: SVM: Add VNMI support in get/set_nmi_mask



On 7/10/2022 9:39 PM, Maxim Levitsky wrote:
> On Fri, 2022-06-17 at 20:18 +0530, Shukla, Santosh wrote:
>>
>> 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 <[email protected]>
>>>>> ---
>>>>> 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.
>
> And yet, svm_set_nmi_mask is called when KVM wants to explicitly mask NMI
> without injecting a NMI, it does this when entering (emulated) SMI.
>
> So the KVM has to set V_NMI_MASK here, becaue no real NMI is injected,
> and thus the CPU will not set this bit itself.
>

Yes, we will handle smm case in v3.

Thanks,
Santosh

> Best regards,
> Maxim Levitsky
>>
>>>> Best regards,
>>>> Maxim Levitsky
>>>>
>>>>
>>>>> +
>>>>> if (masked) {
>>>>> vcpu->arch.hflags |= HF_NMI_MASK;
>>>>> if (!sev_es_guest(vcpu->kvm))
>
>