| Message ID | 20220602142620.3196-5-santosh.shukla@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Virtual NMI feature | expand |
On Thu, 2022-06-02 at 19:56 +0530, 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@amd.com> > --- > 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 d67a54517d95..a405e414cae4 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -3483,6 +3483,9 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu) > struct vmcb *vmcb = svm->vmcb; > bool ret; > > + if (is_vnmi_enabled(vmcb) && is_vnmi_mask_set(vmcb)) > + return true; How does this interact with GIF? if the guest does clgi, will the CPU update the V_NMI_MASK on its own If vGIF is enabled? What happens if vGIF is disabled and vNMI is enabled? KVM then intercepts the stgi/clgi, and it should then update the V_NMI_MASK? > + > if (!gif_set(svm)) > return true; > > @@ -3618,6 +3621,9 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu) > { > struct vcpu_svm *svm = to_svm(vcpu); > > + if (is_vnmi_enabled(svm->vmcb) && is_vnmi_mask_set(svm->vmcb)) > + return; This might have hidden assumption that we will only enable NMI window when vNMI is masked. > + > if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK) > return; /* IRET will cause a vm exit */ > Best regards, Maxim Levitsky
On Tue, 2022-06-07 at 16:10 +0300, Maxim Levitsky wrote: > On Thu, 2022-06-02 at 19:56 +0530, 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@amd.com> > > --- > > 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 d67a54517d95..a405e414cae4 100644 > > --- a/arch/x86/kvm/svm/svm.c > > +++ b/arch/x86/kvm/svm/svm.c > > @@ -3483,6 +3483,9 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu) > > struct vmcb *vmcb = svm->vmcb; > > bool ret; > > > > + if (is_vnmi_enabled(vmcb) && is_vnmi_mask_set(vmcb)) > > + return true; > > How does this interact with GIF? if the guest does clgi, will the > CPU update the V_NMI_MASK on its own If vGIF is enabled? > > What happens if vGIF is disabled and vNMI is enabled? KVM then intercepts > the stgi/clgi, and it should then update the V_NMI_MASK? > > > > > > + > > if (!gif_set(svm)) > > return true; > > > > @@ -3618,6 +3621,9 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu) > > { > > struct vcpu_svm *svm = to_svm(vcpu); > > > > + if (is_vnmi_enabled(svm->vmcb) && is_vnmi_mask_set(svm->vmcb)) > > + return; > > This might have hidden assumption that we will only enable NMI window when vNMI is masked. Also what if vNMI is already pending? Best regards, Maxim Levitsky > > > > > + > > if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK) > > return; /* IRET will cause a vm exit */ > > > > > Best regards, > Maxim Levitsky
On 6/7/2022 6:42 PM, Maxim Levitsky wrote: > On Tue, 2022-06-07 at 16:10 +0300, Maxim Levitsky wrote: >> On Thu, 2022-06-02 at 19:56 +0530, 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@amd.com> >>> --- >>> 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 d67a54517d95..a405e414cae4 100644 >>> --- a/arch/x86/kvm/svm/svm.c >>> +++ b/arch/x86/kvm/svm/svm.c >>> @@ -3483,6 +3483,9 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu) >>> struct vmcb *vmcb = svm->vmcb; >>> bool ret; >>> >>> + if (is_vnmi_enabled(vmcb) && is_vnmi_mask_set(vmcb)) >>> + return true; >> >> How does this interact with GIF? if the guest does clgi, will the >> CPU update the V_NMI_MASK on its own If vGIF is enabled? >> Yes. >> What happens if vGIF is disabled and vNMI is enabled? KVM then intercepts >> the stgi/clgi, and it should then update the V_NMI_MASK? >> No. For both case - HW takes the V_NMI event at the boundary of VMRUN instruction. >> >> >> >>> + >>> if (!gif_set(svm)) >>> return true; >>> >>> @@ -3618,6 +3621,9 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu) >>> { >>> struct vcpu_svm *svm = to_svm(vcpu); >>> >>> + if (is_vnmi_enabled(svm->vmcb) && is_vnmi_mask_set(svm->vmcb)) >>> + return; >> >> This might have hidden assumption that we will only enable NMI window when vNMI is masked. > > Also what if vNMI is already pending? > If V_NMI_MASK set, that means V_NMI is pending, if so then inject another V_NMI in next VMRUN. Thanks, Santosh > Best regards, > Maxim Levitsky >> >> >> >>> + >>> if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK) >>> return; /* IRET will cause a vm exit */ >>> >> >> >> Best regards, >> Maxim Levitsky > >
On Fri, 2022-06-17 at 20:29 +0530, Shukla, Santosh wrote: > > On 6/7/2022 6:42 PM, Maxim Levitsky wrote: > > On Tue, 2022-06-07 at 16:10 +0300, Maxim Levitsky wrote: > > > On Thu, 2022-06-02 at 19:56 +0530, 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@amd.com> > > > > --- > > > > 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 d67a54517d95..a405e414cae4 100644 > > > > --- a/arch/x86/kvm/svm/svm.c > > > > +++ b/arch/x86/kvm/svm/svm.c > > > > @@ -3483,6 +3483,9 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu) > > > > struct vmcb *vmcb = svm->vmcb; > > > > bool ret; > > > > > > > > + if (is_vnmi_enabled(vmcb) && is_vnmi_mask_set(vmcb)) > > > > + return true; > > > > > > How does this interact with GIF? if the guest does clgi, will the > > > CPU update the V_NMI_MASK on its own If vGIF is enabled? > > > > Yes. > > > > What happens if vGIF is disabled and vNMI is enabled? KVM then intercepts > > > the stgi/clgi, and it should then update the V_NMI_MASK? > > > > No. > > For both case - HW takes the V_NMI event at the boundary of VMRUN instruction. How that is possible? if vGIF is disabled in L1, then L1 can't execute STGI/CLGI - that means that the CPU can't update the V_NMI, as it never sees the STGI/CLGI beeing executed. Best regards, Maxim Levitsky > > > > > > > > > > > + > > > > if (!gif_set(svm)) > > > > return true; > > > > > > > > @@ -3618,6 +3621,9 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu) > > > > { > > > > struct vcpu_svm *svm = to_svm(vcpu); > > > > > > > > + if (is_vnmi_enabled(svm->vmcb) && is_vnmi_mask_set(svm->vmcb)) > > > > + return; > > > > > > This might have hidden assumption that we will only enable NMI window when vNMI is masked. > > > > Also what if vNMI is already pending? > > > If V_NMI_MASK set, that means V_NMI is pending, if so then inject another V_NMI in next VMRUN. > > Thanks, > Santosh > > > Best regards, > > Maxim Levitsky > > > > > > > > > > + > > > > if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK) > > > > return; /* IRET will cause a vm exit */ > > > > > > > > > > Best regards, > > > Maxim Levitsky
On 7/10/2022 9:38 PM, Maxim Levitsky wrote: > On Fri, 2022-06-17 at 20:29 +0530, Shukla, Santosh wrote: >> >> On 6/7/2022 6:42 PM, Maxim Levitsky wrote: >>> On Tue, 2022-06-07 at 16:10 +0300, Maxim Levitsky wrote: >>>> On Thu, 2022-06-02 at 19:56 +0530, 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@amd.com> >>>>> --- >>>>> 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 d67a54517d95..a405e414cae4 100644 >>>>> --- a/arch/x86/kvm/svm/svm.c >>>>> +++ b/arch/x86/kvm/svm/svm.c >>>>> @@ -3483,6 +3483,9 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu) >>>>> struct vmcb *vmcb = svm->vmcb; >>>>> bool ret; >>>>> >>>>> + if (is_vnmi_enabled(vmcb) && is_vnmi_mask_set(vmcb)) >>>>> + return true; >>>> >>>> How does this interact with GIF? if the guest does clgi, will the >>>> CPU update the V_NMI_MASK on its own If vGIF is enabled? >>>> >> Yes. >> >>>> What happens if vGIF is disabled and vNMI is enabled? KVM then intercepts >>>> the stgi/clgi, and it should then update the V_NMI_MASK? >>>> >> No. >> >> For both case - HW takes the V_NMI event at the boundary of VMRUN instruction. > > How that is possible? if vGIF is disabled in L1, then L1 can't execute STGI/CLGI - > that means that the CPU can't update the V_NMI, as it never sees the STGI/CLGI > beeing executed. > If vGIF is disabled then HW will take the vnmi event at the boundary of vmrun instruction. Thanks, Santosh > Best regards, > Maxim Levitsky > >> >>>> >>>> >>>>> + >>>>> if (!gif_set(svm)) >>>>> return true; >>>>> >>>>> @@ -3618,6 +3621,9 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu) >>>>> { >>>>> struct vcpu_svm *svm = to_svm(vcpu); >>>>> >>>>> + if (is_vnmi_enabled(svm->vmcb) && is_vnmi_mask_set(svm->vmcb)) >>>>> + return; >>>> >>>> This might have hidden assumption that we will only enable NMI window when vNMI is masked. >>> >>> Also what if vNMI is already pending? >>> >> If V_NMI_MASK set, that means V_NMI is pending, if so then inject another V_NMI in next VMRUN. >> >> Thanks, >> Santosh >> >>> Best regards, >>> Maxim Levitsky >>>> >>>> >>>>> + >>>>> if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK) >>>>> return; /* IRET will cause a vm exit */ >>>>> >>>> >>>> Best regards, >>>> Maxim Levitsky > >
On Thu, 2022-07-21 at 15:01 +0530, Shukla, Santosh wrote: > > On 7/10/2022 9:38 PM, Maxim Levitsky wrote: > > On Fri, 2022-06-17 at 20:29 +0530, Shukla, Santosh wrote: > > > On 6/7/2022 6:42 PM, Maxim Levitsky wrote: > > > > On Tue, 2022-06-07 at 16:10 +0300, Maxim Levitsky wrote: > > > > > On Thu, 2022-06-02 at 19:56 +0530, 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@amd.com> > > > > > > --- > > > > > > 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 d67a54517d95..a405e414cae4 100644 > > > > > > --- a/arch/x86/kvm/svm/svm.c > > > > > > +++ b/arch/x86/kvm/svm/svm.c > > > > > > @@ -3483,6 +3483,9 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu) > > > > > > struct vmcb *vmcb = svm->vmcb; > > > > > > bool ret; > > > > > > > > > > > > + if (is_vnmi_enabled(vmcb) && is_vnmi_mask_set(vmcb)) > > > > > > + return true; > > > > > > > > > > How does this interact with GIF? if the guest does clgi, will the > > > > > CPU update the V_NMI_MASK on its own If vGIF is enabled? > > > > > > > > Yes. > > > > > > > > What happens if vGIF is disabled and vNMI is enabled? KVM then intercepts > > > > > the stgi/clgi, and it should then update the V_NMI_MASK? > > > > > > > > No. > > > > > > For both case - HW takes the V_NMI event at the boundary of VMRUN instruction. > > > > How that is possible? if vGIF is disabled in L1, then L1 can't execute STGI/CLGI - > > that means that the CPU can't update the V_NMI, as it never sees the STGI/CLGI > > beeing executed. > > > > If vGIF is disabled then HW will take the vnmi event at the boundary of vmrun instruction. I think I understand now, if vGIF is enabled, and V_NMI_MASK is set, and the guest does STGI, then nothing new should be injected. If V_NMI_MASK is not set, then svm_nmi_blocked will respect the HF_GIF_MASK, and on STGI interception, the new NMI will be injected on VM entry by setting the V_NMI_PENDING. So looks like it should work. Thanks, Best regards, Maxim Levitsky > > Thanks, > Santosh > > > Best regards, > > Maxim Levitsky > > > > > > > > > > > > > + > > > > > > if (!gif_set(svm)) > > > > > > return true; > > > > > > > > > > > > @@ -3618,6 +3621,9 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu) > > > > > > { > > > > > > struct vcpu_svm *svm = to_svm(vcpu); > > > > > > > > > > > > + if (is_vnmi_enabled(svm->vmcb) && is_vnmi_mask_set(svm->vmcb)) > > > > > > + return; > > > > > > > > > > This might have hidden assumption that we will only enable NMI window when vNMI is masked. > > > > > > > > Also what if vNMI is already pending? > > > > > > > If V_NMI_MASK set, that means V_NMI is pending, if so then inject another V_NMI in next VMRUN. > > > > > > Thanks, > > > Santosh > > > > > > > Best regards, > > > > Maxim Levitsky > > > > > > > > > > > + > > > > > > if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK) > > > > > > return; /* IRET will cause a vm exit */ > > > > > > > > > > > > > > > > Best regards, > > > > > Maxim Levitsky
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index d67a54517d95..a405e414cae4 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -3483,6 +3483,9 @@ bool svm_nmi_blocked(struct kvm_vcpu *vcpu) struct vmcb *vmcb = svm->vmcb; bool ret; + if (is_vnmi_enabled(vmcb) && is_vnmi_mask_set(vmcb)) + return true; + if (!gif_set(svm)) return true; @@ -3618,6 +3621,9 @@ static void svm_enable_nmi_window(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); + if (is_vnmi_enabled(svm->vmcb) && is_vnmi_mask_set(svm->vmcb)) + return; + if ((vcpu->arch.hflags & (HF_NMI_MASK | HF_IRET_MASK)) == HF_NMI_MASK) return; /* IRET will cause a vm exit */
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@amd.com> --- arch/x86/kvm/svm/svm.c | 6 ++++++ 1 file changed, 6 insertions(+)