Re: [PATCHv2 4/7] KVM: SVM: Report NMI not allowed when Guest busy handling VNMI

[Shukla, Santosh]

Hello Sean,

On 7/21/2022 3:24 AM, Sean Christopherson wrote:
> On Sat, Jul 09, 2022, Santosh Shukla wrote:
>> In the VNMI case, Report NMI is not allowed when the processor set the
>> V_NMI_MASK to 1 which means the Guest is busy handling VNMI.
>>
>> Signed-off-by: Santosh Shukla <santosh.shukla@xxxxxxx>
>> ---
>> v2:
>> - Moved vnmi check after is_guest_mode() in func _nmi_blocked().
>> - Removed is_vnmi_mask_set check from _enable_nmi_window().
>> as it was a redundent check.
>>
>>  arch/x86/kvm/svm/svm.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index 3574e804d757..44c1f2317b45 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -3480,6 +3480,9 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu)
>>  	if (is_guest_mode(vcpu) && nested_exit_on_nmi(svm))
>>  		return false;
>>  
>> +	if (is_vnmi_enabled(svm) && is_vnmi_mask_set(svm))
>> +		return true;
>> +
>>  	ret = (vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) ||
>>  	      (vcpu->arch.hflags & HF_NMI_MASK);
>>  
>> @@ -3609,6 +3612,9 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu)
>>  {
>>  	struct vcpu_svm *svm = to_svm(vcpu);
>>  
>> +	if (is_vnmi_enabled(svm))
>> +		return;
> 
> Ugh, is there really no way to trigger an exit when NMIs become unmasked?  Because
> if there isn't, this is broken for KVM.
> 

Yes. there is.

NMI_INTERCEPT will trigger VMEXIT when second NMI arrives while guest is busy handling first NMI.
And in that scenario, Guest will exit with V_NMI_MASK set to 1, KVM can inject pending(Second)
NMI(V_NMI=1). Guest will resume handling the first NMI, then HW will
clear the V_NMI_MASK and later HW will take the pending V_NMI in side the guest. 

I'll handle above case in v3.

Thanks,
Santosh

> On bare metal, if two NMIs arrive "simultaneously", so long as NMIs aren't blocked,
> the first NMI will be delivered and the second will be pended, i.e. software will
> see both NMIs.  And if that doesn't hold true, the window for a true collision is
> really, really tiny.
> 
> But in KVM, because a vCPU may not be run a long duration, that window becomes
> very large.  To not drop NMIs and more faithfully emulate hardware, KVM allows two
> NMIs to be _pending_.  And when that happens, KVM needs to trigger an exit when
> NMIs become unmasked _after_ the first NMI is injected.
> 
>> +
>>  	if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK)
>>  		return; /* IRET will cause a vm exit */
>>  
>> -- 
>> 2.25.1
>>