Re: [PATCH 2/7] KVM: SVM: Add VNMI bit definition

From: "Shukla, Santosh" <santosh.shukla@amd.com>
To: Maxim Levitsky <mlevitsk@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/7] KVM: SVM: Add VNMI bit definition
Date: Fri, 17 Jun 2022 20:12:49 +0530	[thread overview]
Message-ID: <737d3bda-0f97-0689-53e9-21bd11ae647e@amd.com> (raw)
In-Reply-To: <bd37180680b3e3ecec85b5151742092b9f1ce9ff.camel@redhat.com>

On 6/7/2022 6:25 PM, Maxim Levitsky wrote:
> On Thu, 2022-06-02 at 19:56 +0530, Santosh Shukla wrote:
>> VNMI exposes 3 capability bits (V_NMI, V_NMI_MASK, and V_NMI_ENABLE) to
>> virtualize NMI and NMI_MASK, Those capability bits are part of
>> VMCB::intr_ctrl -
>> V_NMI(11) - Indicates whether a virtual NMI is pending in the guest.
> So this is like bit in IRR
> 
>> V_NMI_MASK(12) - Indicates whether virtual NMI is masked in the guest.
> And that is like bit in ISR.
> 
> Question: what are the interactions with GIF/vGIF and this feature?
> 

Hi Maxim,

The quick answer is, that V_NMI will respect the VGIF enable controls.
More info about interaction in subsequent patch reply.

Thanks,
Santosh

>> V_NMI_ENABLE(26) - Enables the NMI virtualization feature for the guest.
>>
>> When Hypervisor wants to inject NMI, it will set V_NMI bit, Processor
>> will clear the V_NMI bit and Set the V_NMI_MASK which means the Guest is
>> handling NMI, After the guest handled the NMI, The processor will clear
>> the V_NMI_MASK on the successful completion of IRET instruction Or if
>> VMEXIT occurs while delivering the virtual NMI.
>>
>> To enable the VNMI capability, Hypervisor need to program
>> V_NMI_ENABLE bit 1.
>>
>> Signed-off-by: Santosh Shukla <santosh.shukla@amd.com>
>> ---
>>  arch/x86/include/asm/svm.h | 7 +++++++
>>  arch/x86/kvm/svm/svm.c     | 6 ++++++
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
>> index 1b07fba11704..22d918555df0 100644
>> --- a/arch/x86/include/asm/svm.h
>> +++ b/arch/x86/include/asm/svm.h
>> @@ -195,6 +195,13 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
>>  #define AVIC_ENABLE_SHIFT 31
>>  #define AVIC_ENABLE_MASK (1 << AVIC_ENABLE_SHIFT)
>>  
>> +#define V_NMI_PENDING_SHIFT 11
>> +#define V_NMI_PENDING (1 << V_NMI_PENDING_SHIFT)
>> +#define V_NMI_MASK_SHIFT 12
>> +#define V_NMI_MASK (1 << V_NMI_MASK_SHIFT)
>> +#define V_NMI_ENABLE_SHIFT 26
>> +#define V_NMI_ENABLE (1 << V_NMI_ENABLE_SHIFT)
>> +
>>  #define LBR_CTL_ENABLE_MASK BIT_ULL(0)
>>  #define VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK BIT_ULL(1)
>>  
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index 200045f71df0..860f28c668bd 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -198,6 +198,8 @@ module_param(dump_invalid_vmcb, bool, 0644);
>>  bool intercept_smi = true;
>>  module_param(intercept_smi, bool, 0444);
>>  
>> +static bool vnmi;
>> +module_param(vnmi, bool, 0444);
>>  
>>  static bool svm_gp_erratum_intercept = true;
>>  
>> @@ -4930,6 +4932,10 @@ static __init int svm_hardware_setup(void)
>>                 svm_x86_ops.vcpu_get_apicv_inhibit_reasons = NULL;
>>         }
>>  
>> +       vnmi = vnmi && boot_cpu_has(X86_FEATURE_V_NMI);
>> +       if (vnmi)
>> +               pr_info("V_NMI enabled\n");
>> +
>>         if (vls) {
>>                 if (!npt_enabled ||
>>                     !boot_cpu_has(X86_FEATURE_V_VMSAVE_VMLOAD) ||
> 
> 
> Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
> 
> Best regards,
> 	Maxim Levitsky
> 