[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y3uzAM3/XrUPRpgH@google.com>
Date: Mon, 21 Nov 2022 17:18:56 +0000
From: Sean Christopherson <seanjc@...gle.com>
To: Maxim Levitsky <mlevitsk@...hat.com>
Cc: kvm@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
linux-kernel@...r.kernel.org,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Sandipan Das <sandipan.das@....com>,
Daniel Sneddon <daniel.sneddon@...ux.intel.com>,
Jing Liu <jing2.liu@...el.com>,
Josh Poimboeuf <jpoimboe@...nel.org>,
Wyes Karny <wyes.karny@....com>,
Borislav Petkov <bp@...en8.de>,
Babu Moger <babu.moger@....com>,
Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
Jim Mattson <jmattson@...gle.com>, x86@...nel.org,
Santosh Shukla <santosh.shukla@....com>
Subject: Re: [PATCH 07/13] KVM: SVM: Add VNMI support in get/set_nmi_mask
On Mon, Nov 21, 2022, Maxim Levitsky wrote:
> On Thu, 2022-11-17 at 18:54 +0000, Sean Christopherson wrote:
> > E.g. with HF_NMI_MASK => svm->nmi_masked, the end result can be something like:
> >
> > static bool __is_vnmi_enabled(struct *vmcb)
> > {
> > return !!(vmcb->control.int_ctl & V_NMI_ENABLE);
> > }
> >
> > static bool is_vnmi_enabled(struct vcpu_svm *svm)
> > {
> > struct vmcb *vmcb = get_vnmi_vmcb(svm);
> >
> > return vmcb && __is_vnmi_enabled(vmcb);
> > }
> >
> > static bool svm_get_nmi_mask(struct kvm_vcpu *vcpu)
> > {
> > struct vcpu_svm *svm = to_svm(vcpu);
> > struct vmcb *vmcb = get_vnmi_vmcb(svm);
> >
> > if (vmcb && __is_vnmi_enabled(vmcb))
> > return !!(vmcb->control.int_ctl & V_NMI_MASK);
> > else
> > return !!(vcpu->arch.hflags & HF_NMI_MASK);
> > }
> >
> > static void svm_set_or_clear_vnmi_mask(struct vmcb *vmcb, bool set)
> > {
> > if (set)
> > vmcb->control.int_ctl |= V_NMI_MASK;
> > else
> > vmcb->control.int_ctl &= ~V_NMI_MASK;
> > }
> >
> > static void svm_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked)
> > {
> > struct vcpu_svm *svm = to_svm(vcpu);
> > struct vmcb *vmcb = get_vnmi_vmcb(svm);
> >
> > if (vmcb && __is_vnmi_enabled(vmcb)) {
> > if (masked)
> > vmcb->control.int_ctl |= V_NMI_MASK;
> > else
> > vmcb->control.int_ctl &= ~V_NMI_MASK;
> > } else {
> > svm->nmi_masked = masked;
> > }
> >
> > if (!masked)
> > svm_disable_iret_interception(svm);
> > }
>
> OK, this is one of the ways to do it, makes sense overall.
> I actualy wanted to do something like that but opted to not touch
> the original code too much, but only what I needed. I can do this
> in a next version.
After looking at more of this code, I think having get_vnmi_vmcb() is a mistake.
It just ends up being a funky wrapper to the current svm->vmcb. And the manual
check on the "vnmi" global is pointless. If KVM sets V_NMI_ENABLE in any VMCB
when vnmi=false, then that's a KVM bug.
Dropping the wrapper eliminates the possibility of a NULL VMCB pointer, and IMO
yields far more readable code.
static bool is_vnmi_enabled(struct vcpu_svm *svm)
{
return !!(svm->vmcb->control.int_ctl & V_NMI_ENABLE);
}
static bool svm_get_nmi_mask(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
if (is_vnmi_enabled(svm))
return !!(svm->vmcb->control.int_ctl & V_NMI_MASK);
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)) {
if (masked)
svm->vmcb->control.int_ctl |= V_NMI_MASK;
else
svm->vmcb->control.int_ctl &= ~V_NMI_MASK;
} else {
svm->nmi_masked = masked;
}
if (!masked)
svm_disable_iret_interception(svm);
}
Powered by blists - more mailing lists