From: Marcelo Tosatti <mtosatti@redhat.com>
To: Avi Kivity <avi@redhat.com>
Cc: kvm@vger.kernel.org, Jan Kiszka <jan.kiszka@siemens.com>
Subject: Re: [PATCH v2] KVM: Fix simultaneous NMIs
Date: Tue, 20 Sep 2011 13:30:21 -0300 [thread overview]
Message-ID: <20110920163021.GA29618@amt.cnet> (raw)
In-Reply-To: <4E78BE21.4070508@redhat.com>
On Tue, Sep 20, 2011 at 07:24:01PM +0300, Avi Kivity wrote:
> On 09/20/2011 05:59 PM, Marcelo Tosatti wrote:
> >On Tue, Sep 20, 2011 at 04:56:02PM +0300, Avi Kivity wrote:
> >> On 09/20/2011 04:25 PM, Marcelo Tosatti wrote:
> >> >>
> >> >> @@ -2827,6 +2828,7 @@ static int kvm_vcpu_ioctl_x86_set_mce(struct kvm_vcpu *vcpu,
> >> >> static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
> >> >> struct kvm_vcpu_events *events)
> >> >> {
> >> >> + process_nmi(vcpu);
> >> >> events->exception.injected =
> >> >> vcpu->arch.exception.pending&&
> >> >> !kvm_exception_is_soft(vcpu->arch.exception.nr);
> >> >> @@ -2844,7 +2846,7 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
> >> >> KVM_X86_SHADOW_INT_MOV_SS | KVM_X86_SHADOW_INT_STI);
> >> >>
> >> >> events->nmi.injected = vcpu->arch.nmi_injected;
> >> >> - events->nmi.pending = vcpu->arch.nmi_pending;
> >> >> + events->nmi.pending = vcpu->arch.nmi_pending != 0;
> >> >> events->nmi.masked = kvm_x86_ops->get_nmi_mask(vcpu);
> >> >> events->nmi.pad = 0;
> >> >
> >> >nmi_queued should also be saved and restored. Not sure if its necessary
> >> >though.
> >> >
> >> >Should at least reset nmi_queued somewhere (set_vcpu_events?).
> >>
> >> Did you miss the call to process_nmi()?
> >
> >It transfers nmi_queued to nmi_pending with capping. What i mean is that
> >upon system reset, nmi_queued (which refers to pre-reset system state)
> >should be zeroed.
>
> The is get_events(); for set_events(), process_nmi() does zero
> nmi_queued() (and then we overwrite nmi_pending).
Right.
> >
> >> We do have a small issue. If we exit during NMI-blocked-by-STI and
> >> nmi_pending == 2, then we lose the second interrupt. Should rarely
> >> happen, since external interrupts never exit in that condition, but
> >> it's a wart.
> >
> >And the above system reset case, you should be able to handle it by
> >saving/restoring nmi_queued (so that QEMU can zero it in vcpu_reset).
>
> We could just add a KVM_CAP (and flag) that extends nmi_pending from
> a bool to a counter.
Or just add a new field to the pad.
next prev parent reply other threads:[~2011-09-20 16:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-20 10:43 [PATCH v2] KVM: Fix simultaneous NMIs Avi Kivity
2011-09-20 13:25 ` Marcelo Tosatti
2011-09-20 13:56 ` Avi Kivity
2011-09-20 14:59 ` Marcelo Tosatti
2011-09-20 16:24 ` Avi Kivity
2011-09-20 16:30 ` Marcelo Tosatti [this message]
2011-09-20 17:28 ` Avi Kivity
2011-09-21 8:46 ` Avi Kivity
2011-09-21 16:44 ` Marcelo Tosatti
2011-09-23 11:54 ` Marcelo Tosatti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110920163021.GA29618@amt.cnet \
--to=mtosatti@redhat.com \
--cc=avi@redhat.com \
--cc=jan.kiszka@siemens.com \
--cc=kvm@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.