* KVM: x86: better fix for race between nmi injection and enabling nmi window
@ 2011-03-30 16:30 Marcelo Tosatti
2011-03-30 16:33 ` Gleb Natapov
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Marcelo Tosatti @ 2011-03-30 16:30 UTC (permalink / raw)
To: kvm; +Cc: Avi Kivity, Gleb Natapov
Based on Gleb's idea, fix race between nmi injection and enabling
nmi window in a simpler way.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a6a129f..9a7cc1be 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5152,6 +5152,7 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
{
int r;
+ int nmi_pending;
bool req_int_win = !irqchip_in_kernel(vcpu->kvm) &&
vcpu->run->request_interrupt_window;
@@ -5195,11 +5196,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
if (unlikely(r))
goto out;
+ nmi_pending = ACCESS_ONCE(vcpu->arch.nmi_pending);
+
if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
inject_pending_event(vcpu);
/* enable NMI/IRQ window open exits if needed */
- if (vcpu->arch.nmi_pending)
+ if (nmi_pending)
kvm_x86_ops->enable_nmi_window(vcpu);
else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
kvm_x86_ops->enable_irq_window(vcpu);
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: KVM: x86: better fix for race between nmi injection and enabling nmi window
2011-03-30 16:30 KVM: x86: better fix for race between nmi injection and enabling nmi window Marcelo Tosatti
@ 2011-03-30 16:33 ` Gleb Natapov
2011-03-30 16:43 ` Marcelo Tosatti
2011-03-30 17:16 ` Avi Kivity
2011-04-01 14:26 ` KVM: x86: better fix for race between nmi injection and enabling nmi window (v2) Marcelo Tosatti
2 siblings, 1 reply; 12+ messages in thread
From: Gleb Natapov @ 2011-03-30 16:33 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm, Avi Kivity
On Wed, Mar 30, 2011 at 01:30:28PM -0300, Marcelo Tosatti wrote:
>
> Based on Gleb's idea, fix race between nmi injection and enabling
> nmi window in a simpler way.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
But we need to revert the patch that introduced use of request for NMI
first. Otherwise NMI will not be delivered, no?
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index a6a129f..9a7cc1be 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5152,6 +5152,7 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> {
> int r;
> + int nmi_pending;
> bool req_int_win = !irqchip_in_kernel(vcpu->kvm) &&
> vcpu->run->request_interrupt_window;
>
> @@ -5195,11 +5196,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> if (unlikely(r))
> goto out;
>
> + nmi_pending = ACCESS_ONCE(vcpu->arch.nmi_pending);
> +
> if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> inject_pending_event(vcpu);
>
> /* enable NMI/IRQ window open exits if needed */
> - if (vcpu->arch.nmi_pending)
> + if (nmi_pending)
> kvm_x86_ops->enable_nmi_window(vcpu);
> else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
> kvm_x86_ops->enable_irq_window(vcpu);
--
Gleb.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: KVM: x86: better fix for race between nmi injection and enabling nmi window
2011-03-30 16:33 ` Gleb Natapov
@ 2011-03-30 16:43 ` Marcelo Tosatti
0 siblings, 0 replies; 12+ messages in thread
From: Marcelo Tosatti @ 2011-03-30 16:43 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, Avi Kivity
On Wed, Mar 30, 2011 at 06:33:22PM +0200, Gleb Natapov wrote:
> On Wed, Mar 30, 2011 at 01:30:28PM -0300, Marcelo Tosatti wrote:
> >
> > Based on Gleb's idea, fix race between nmi injection and enabling
> > nmi window in a simpler way.
> >
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> >
> But we need to revert the patch that introduced use of request for NMI
> first. Otherwise NMI will not be delivered, no?
Yes, pushed, can you verify please?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: KVM: x86: better fix for race between nmi injection and enabling nmi window
2011-03-30 16:30 KVM: x86: better fix for race between nmi injection and enabling nmi window Marcelo Tosatti
2011-03-30 16:33 ` Gleb Natapov
@ 2011-03-30 17:16 ` Avi Kivity
2011-03-30 18:47 ` Gleb Natapov
2011-04-01 14:26 ` KVM: x86: better fix for race between nmi injection and enabling nmi window (v2) Marcelo Tosatti
2 siblings, 1 reply; 12+ messages in thread
From: Avi Kivity @ 2011-03-30 17:16 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm, Gleb Natapov
On 03/30/2011 06:30 PM, Marcelo Tosatti wrote:
> Based on Gleb's idea, fix race between nmi injection and enabling
> nmi window in a simpler way.
>
> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index a6a129f..9a7cc1be 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5152,6 +5152,7 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> {
> int r;
> + int nmi_pending;
> bool req_int_win = !irqchip_in_kernel(vcpu->kvm)&&
> vcpu->run->request_interrupt_window;
>
> @@ -5195,11 +5196,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> if (unlikely(r))
> goto out;
>
> + nmi_pending = ACCESS_ONCE(vcpu->arch.nmi_pending);
> +
> if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> inject_pending_event(vcpu);
>
> /* enable NMI/IRQ window open exits if needed */
> - if (vcpu->arch.nmi_pending)
> + if (nmi_pending)
> kvm_x86_ops->enable_nmi_window(vcpu);
> else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
> kvm_x86_ops->enable_irq_window(vcpu);
>
What about the check in inject_pending_events()?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: KVM: x86: better fix for race between nmi injection and enabling nmi window
2011-03-30 17:16 ` Avi Kivity
@ 2011-03-30 18:47 ` Gleb Natapov
2011-03-31 9:23 ` Avi Kivity
2011-03-31 9:30 ` Marcelo Tosatti
0 siblings, 2 replies; 12+ messages in thread
From: Gleb Natapov @ 2011-03-30 18:47 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm
On Wed, Mar 30, 2011 at 07:16:34PM +0200, Avi Kivity wrote:
> On 03/30/2011 06:30 PM, Marcelo Tosatti wrote:
> >Based on Gleb's idea, fix race between nmi injection and enabling
> >nmi window in a simpler way.
> >
> >Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> >
> >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >index a6a129f..9a7cc1be 100644
> >--- a/arch/x86/kvm/x86.c
> >+++ b/arch/x86/kvm/x86.c
> >@@ -5152,6 +5152,7 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> > static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> > {
> > int r;
> >+ int nmi_pending;
> > bool req_int_win = !irqchip_in_kernel(vcpu->kvm)&&
> > vcpu->run->request_interrupt_window;
> >
> >@@ -5195,11 +5196,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> > if (unlikely(r))
> > goto out;
> >
> >+ nmi_pending = ACCESS_ONCE(vcpu->arch.nmi_pending);
> >+
> > if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> > inject_pending_event(vcpu);
> >
> > /* enable NMI/IRQ window open exits if needed */
> >- if (vcpu->arch.nmi_pending)
> >+ if (nmi_pending)
> > kvm_x86_ops->enable_nmi_window(vcpu);
> > else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
> > kvm_x86_ops->enable_irq_window(vcpu);
> >
>
> What about the check in inject_pending_events()?
>
Didn't we decide that this check is not a problem? Worst that can happen
is NMI injection will be delayed till next exit.
--
Gleb.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: KVM: x86: better fix for race between nmi injection and enabling nmi window
2011-03-30 18:47 ` Gleb Natapov
@ 2011-03-31 9:23 ` Avi Kivity
2011-03-31 9:24 ` Gleb Natapov
2011-03-31 9:30 ` Marcelo Tosatti
1 sibling, 1 reply; 12+ messages in thread
From: Avi Kivity @ 2011-03-31 9:23 UTC (permalink / raw)
To: Gleb Natapov; +Cc: Marcelo Tosatti, kvm
On 03/30/2011 08:47 PM, Gleb Natapov wrote:
> On Wed, Mar 30, 2011 at 07:16:34PM +0200, Avi Kivity wrote:
> > On 03/30/2011 06:30 PM, Marcelo Tosatti wrote:
> > >Based on Gleb's idea, fix race between nmi injection and enabling
> > >nmi window in a simpler way.
> > >
> > >Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> > >
> > >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > >index a6a129f..9a7cc1be 100644
> > >--- a/arch/x86/kvm/x86.c
> > >+++ b/arch/x86/kvm/x86.c
> > >@@ -5152,6 +5152,7 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> > > static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> > > {
> > > int r;
> > >+ int nmi_pending;
> > > bool req_int_win = !irqchip_in_kernel(vcpu->kvm)&&
> > > vcpu->run->request_interrupt_window;
> > >
> > >@@ -5195,11 +5196,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> > > if (unlikely(r))
> > > goto out;
> > >
> > >+ nmi_pending = ACCESS_ONCE(vcpu->arch.nmi_pending);
> > >+
> > > if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> > > inject_pending_event(vcpu);
> > >
> > > /* enable NMI/IRQ window open exits if needed */
> > >- if (vcpu->arch.nmi_pending)
> > >+ if (nmi_pending)
> > > kvm_x86_ops->enable_nmi_window(vcpu);
> > > else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
> > > kvm_x86_ops->enable_irq_window(vcpu);
> > >
> >
> > What about the check in inject_pending_events()?
> >
> Didn't we decide that this check is not a problem? Worst that can happen
> is NMI injection will be delayed till next exit.
Could be very far in the future.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: KVM: x86: better fix for race between nmi injection and enabling nmi window
2011-03-31 9:23 ` Avi Kivity
@ 2011-03-31 9:24 ` Gleb Natapov
2011-03-31 9:25 ` Avi Kivity
0 siblings, 1 reply; 12+ messages in thread
From: Gleb Natapov @ 2011-03-31 9:24 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm
On Thu, Mar 31, 2011 at 11:23:28AM +0200, Avi Kivity wrote:
> On 03/30/2011 08:47 PM, Gleb Natapov wrote:
> >On Wed, Mar 30, 2011 at 07:16:34PM +0200, Avi Kivity wrote:
> >> On 03/30/2011 06:30 PM, Marcelo Tosatti wrote:
> >> >Based on Gleb's idea, fix race between nmi injection and enabling
> >> >nmi window in a simpler way.
> >> >
> >> >Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> >> >
> >> >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> >index a6a129f..9a7cc1be 100644
> >> >--- a/arch/x86/kvm/x86.c
> >> >+++ b/arch/x86/kvm/x86.c
> >> >@@ -5152,6 +5152,7 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> >> > static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >> > {
> >> > int r;
> >> >+ int nmi_pending;
> >> > bool req_int_win = !irqchip_in_kernel(vcpu->kvm)&&
> >> > vcpu->run->request_interrupt_window;
> >> >
> >> >@@ -5195,11 +5196,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >> > if (unlikely(r))
> >> > goto out;
> >> >
> >> >+ nmi_pending = ACCESS_ONCE(vcpu->arch.nmi_pending);
> >> >+
> >> > if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> >> > inject_pending_event(vcpu);
> >> >
> >> > /* enable NMI/IRQ window open exits if needed */
> >> >- if (vcpu->arch.nmi_pending)
> >> >+ if (nmi_pending)
> >> > kvm_x86_ops->enable_nmi_window(vcpu);
> >> > else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
> >> > kvm_x86_ops->enable_irq_window(vcpu);
> >> >
> >>
> >> What about the check in inject_pending_events()?
> >>
> >Didn't we decide that this check is not a problem? Worst that can happen
> >is NMI injection will be delayed till next exit.
>
> Could be very far in the future.
>
Next host interrupt. But with tickles host and guest yeah.
--
Gleb.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: KVM: x86: better fix for race between nmi injection and enabling nmi window
2011-03-31 9:24 ` Gleb Natapov
@ 2011-03-31 9:25 ` Avi Kivity
2011-03-31 9:40 ` Marcelo Tosatti
2011-03-31 9:47 ` Gleb Natapov
0 siblings, 2 replies; 12+ messages in thread
From: Avi Kivity @ 2011-03-31 9:25 UTC (permalink / raw)
To: Gleb Natapov; +Cc: Marcelo Tosatti, kvm
On 03/31/2011 11:24 AM, Gleb Natapov wrote:
> On Thu, Mar 31, 2011 at 11:23:28AM +0200, Avi Kivity wrote:
> > On 03/30/2011 08:47 PM, Gleb Natapov wrote:
> > >On Wed, Mar 30, 2011 at 07:16:34PM +0200, Avi Kivity wrote:
> > >> On 03/30/2011 06:30 PM, Marcelo Tosatti wrote:
> > >> >Based on Gleb's idea, fix race between nmi injection and enabling
> > >> >nmi window in a simpler way.
> > >> >
> > >> >Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> > >> >
> > >> >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > >> >index a6a129f..9a7cc1be 100644
> > >> >--- a/arch/x86/kvm/x86.c
> > >> >+++ b/arch/x86/kvm/x86.c
> > >> >@@ -5152,6 +5152,7 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> > >> > static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> > >> > {
> > >> > int r;
> > >> >+ int nmi_pending;
> > >> > bool req_int_win = !irqchip_in_kernel(vcpu->kvm)&&
> > >> > vcpu->run->request_interrupt_window;
> > >> >
> > >> >@@ -5195,11 +5196,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> > >> > if (unlikely(r))
> > >> > goto out;
> > >> >
> > >> >+ nmi_pending = ACCESS_ONCE(vcpu->arch.nmi_pending);
> > >> >+
> > >> > if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> > >> > inject_pending_event(vcpu);
> > >> >
> > >> > /* enable NMI/IRQ window open exits if needed */
> > >> >- if (vcpu->arch.nmi_pending)
> > >> >+ if (nmi_pending)
> > >> > kvm_x86_ops->enable_nmi_window(vcpu);
> > >> > else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
> > >> > kvm_x86_ops->enable_irq_window(vcpu);
> > >> >
> > >>
> > >> What about the check in inject_pending_events()?
> > >>
> > >Didn't we decide that this check is not a problem? Worst that can happen
> > >is NMI injection will be delayed till next exit.
> >
> > Could be very far in the future.
> >
> Next host interrupt. But with tickles host and guest yeah.
>
esp. important with NMI, which may be used in a situation where your
tick (and everything else) are dead.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: KVM: x86: better fix for race between nmi injection and enabling nmi window
2011-03-31 9:25 ` Avi Kivity
@ 2011-03-31 9:40 ` Marcelo Tosatti
2011-03-31 9:47 ` Gleb Natapov
1 sibling, 0 replies; 12+ messages in thread
From: Marcelo Tosatti @ 2011-03-31 9:40 UTC (permalink / raw)
To: Avi Kivity; +Cc: Gleb Natapov, kvm
On Thu, Mar 31, 2011 at 11:25:46AM +0200, Avi Kivity wrote:
<snip>
> >> >> > else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
> >> >> > kvm_x86_ops->enable_irq_window(vcpu);
> >> >> >
> >> >>
> >> >> What about the check in inject_pending_events()?
> >> >>
> >> >Didn't we decide that this check is not a problem? Worst that can happen
> >> >is NMI injection will be delayed till next exit.
> >>
> >> Could be very far in the future.
> >>
> >Next host interrupt. But with tickles host and guest yeah.
> >
>
> esp. important with NMI, which may be used in a situation where your
> tick (and everything else) are dead.
Well the host must be alive. So eventually NMI will be delivered to the
guest, but not closely matching guest visible clocks.
What is the problem with that? The race should be rare enough to not
interfere with NMI watchdog-like things?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: KVM: x86: better fix for race between nmi injection and enabling nmi window
2011-03-31 9:25 ` Avi Kivity
2011-03-31 9:40 ` Marcelo Tosatti
@ 2011-03-31 9:47 ` Gleb Natapov
1 sibling, 0 replies; 12+ messages in thread
From: Gleb Natapov @ 2011-03-31 9:47 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm
On Thu, Mar 31, 2011 at 11:25:46AM +0200, Avi Kivity wrote:
> On 03/31/2011 11:24 AM, Gleb Natapov wrote:
> >On Thu, Mar 31, 2011 at 11:23:28AM +0200, Avi Kivity wrote:
> >> On 03/30/2011 08:47 PM, Gleb Natapov wrote:
> >> >On Wed, Mar 30, 2011 at 07:16:34PM +0200, Avi Kivity wrote:
> >> >> On 03/30/2011 06:30 PM, Marcelo Tosatti wrote:
> >> >> >Based on Gleb's idea, fix race between nmi injection and enabling
> >> >> >nmi window in a simpler way.
> >> >> >
> >> >> >Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> >> >> >
> >> >> >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> >> >index a6a129f..9a7cc1be 100644
> >> >> >--- a/arch/x86/kvm/x86.c
> >> >> >+++ b/arch/x86/kvm/x86.c
> >> >> >@@ -5152,6 +5152,7 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> >> >> > static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >> >> > {
> >> >> > int r;
> >> >> >+ int nmi_pending;
> >> >> > bool req_int_win = !irqchip_in_kernel(vcpu->kvm)&&
> >> >> > vcpu->run->request_interrupt_window;
> >> >> >
> >> >> >@@ -5195,11 +5196,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >> >> > if (unlikely(r))
> >> >> > goto out;
> >> >> >
> >> >> >+ nmi_pending = ACCESS_ONCE(vcpu->arch.nmi_pending);
> >> >> >+
> >> >> > if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> >> >> > inject_pending_event(vcpu);
> >> >> >
> >> >> > /* enable NMI/IRQ window open exits if needed */
> >> >> >- if (vcpu->arch.nmi_pending)
> >> >> >+ if (nmi_pending)
> >> >> > kvm_x86_ops->enable_nmi_window(vcpu);
> >> >> > else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
> >> >> > kvm_x86_ops->enable_irq_window(vcpu);
> >> >> >
> >> >>
> >> >> What about the check in inject_pending_events()?
> >> >>
> >> >Didn't we decide that this check is not a problem? Worst that can happen
> >> >is NMI injection will be delayed till next exit.
> >>
> >> Could be very far in the future.
> >>
> >Next host interrupt. But with tickles host and guest yeah.
> >
>
> esp. important with NMI, which may be used in a situation where your
> tick (and everything else) are dead.
>
If host is alive eventually cpu should receive reschedule IPI.
--
Gleb.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: KVM: x86: better fix for race between nmi injection and enabling nmi window
2011-03-30 18:47 ` Gleb Natapov
2011-03-31 9:23 ` Avi Kivity
@ 2011-03-31 9:30 ` Marcelo Tosatti
1 sibling, 0 replies; 12+ messages in thread
From: Marcelo Tosatti @ 2011-03-31 9:30 UTC (permalink / raw)
To: Gleb Natapov; +Cc: Avi Kivity, kvm
On Wed, Mar 30, 2011 at 08:47:03PM +0200, Gleb Natapov wrote:
> On Wed, Mar 30, 2011 at 07:16:34PM +0200, Avi Kivity wrote:
> > On 03/30/2011 06:30 PM, Marcelo Tosatti wrote:
> > >Based on Gleb's idea, fix race between nmi injection and enabling
> > >nmi window in a simpler way.
> > >
> > >Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> > >
> > >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > >index a6a129f..9a7cc1be 100644
> > >--- a/arch/x86/kvm/x86.c
> > >+++ b/arch/x86/kvm/x86.c
> > >@@ -5152,6 +5152,7 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> > > static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> > > {
> > > int r;
> > >+ int nmi_pending;
> > > bool req_int_win = !irqchip_in_kernel(vcpu->kvm)&&
> > > vcpu->run->request_interrupt_window;
> > >
> > >@@ -5195,11 +5196,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> > > if (unlikely(r))
> > > goto out;
> > >
> > >+ nmi_pending = ACCESS_ONCE(vcpu->arch.nmi_pending);
> > >+
> > > if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> > > inject_pending_event(vcpu);
> > >
> > > /* enable NMI/IRQ window open exits if needed */
> > >- if (vcpu->arch.nmi_pending)
> > >+ if (nmi_pending)
> > > kvm_x86_ops->enable_nmi_window(vcpu);
> > > else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
> > > kvm_x86_ops->enable_irq_window(vcpu);
> > >
> >
> > What about the check in inject_pending_events()?
> >
> Didn't we decide that this check is not a problem? Worst that can happen
> is NMI injection will be delayed till next exit.
Yes.
^ permalink raw reply [flat|nested] 12+ messages in thread
* KVM: x86: better fix for race between nmi injection and enabling nmi window (v2)
2011-03-30 16:30 KVM: x86: better fix for race between nmi injection and enabling nmi window Marcelo Tosatti
2011-03-30 16:33 ` Gleb Natapov
2011-03-30 17:16 ` Avi Kivity
@ 2011-04-01 14:26 ` Marcelo Tosatti
2 siblings, 0 replies; 12+ messages in thread
From: Marcelo Tosatti @ 2011-04-01 14:26 UTC (permalink / raw)
To: kvm; +Cc: Avi Kivity, Gleb Natapov
From: Gleb Natapov <gleb@redhat.com>
Fix race between nmi injection and enabling nmi window in a simpler way.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
v2: document benign race.
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index bfd7763..e6bdc14 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5152,6 +5152,7 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
{
int r;
+ bool nmi_pending;
bool req_int_win = !irqchip_in_kernel(vcpu->kvm) &&
vcpu->run->request_interrupt_window;
@@ -5197,11 +5198,19 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
if (unlikely(r))
goto out;
+ /*
+ * An NMI can be injected between local nmi_pending read and
+ * vcpu->arch.nmi_pending read inside inject_pending_event().
+ * But in that case, KVM_REQ_EVENT will be set, which makes
+ * the race described above benign.
+ */
+ nmi_pending = ACCESS_ONCE(vcpu->arch.nmi_pending);
+
if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
inject_pending_event(vcpu);
/* enable NMI/IRQ window open exits if needed */
- if (vcpu->arch.nmi_pending)
+ if (nmi_pending)
kvm_x86_ops->enable_nmi_window(vcpu);
else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
kvm_x86_ops->enable_irq_window(vcpu);
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-04-01 14:27 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-30 16:30 KVM: x86: better fix for race between nmi injection and enabling nmi window Marcelo Tosatti
2011-03-30 16:33 ` Gleb Natapov
2011-03-30 16:43 ` Marcelo Tosatti
2011-03-30 17:16 ` Avi Kivity
2011-03-30 18:47 ` Gleb Natapov
2011-03-31 9:23 ` Avi Kivity
2011-03-31 9:24 ` Gleb Natapov
2011-03-31 9:25 ` Avi Kivity
2011-03-31 9:40 ` Marcelo Tosatti
2011-03-31 9:47 ` Gleb Natapov
2011-03-31 9:30 ` Marcelo Tosatti
2011-04-01 14:26 ` KVM: x86: better fix for race between nmi injection and enabling nmi window (v2) Marcelo Tosatti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox