public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Arch cleanup v3
@ 2007-07-26 14:51 Gregory Haskins
       [not found] ` <20070726144602.4847.64724.stgit-sLgBBP33vUGnsjUZhwzVf9HuzzzSOjJt@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Gregory Haskins @ 2007-07-26 14:51 UTC (permalink / raw)
  To: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
  Cc: ghaskins-Et1tbQHTxzrQT0dZR+AlfA

I have rebased the patch series on top of kvm.git HEAD origin/master (before I
was on the preempt-hooks branch) and am now including a patch to cleanup a
race condition on VMX w.r.t. VMCS management.  I have a third patch that
changes vcpu->_priv over to container_of as discussed, but its dependent on
Rusty's vcpu array cleanup so its not ready for prime-time yet.  Once we have
his patch in its final form, I will send out the third patch as well.

Until then, this patch series is self-sufficient and can be applied if
desired.  It builds fine, and has been boot tested on VMX with windows and
linux. 

Signed-off-by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] KVM: Protect race-condition between VMCS and current_vmcs on VMX hardware
@ 2007-07-26 15:15 Gregory Haskins
       [not found] ` <46A882480200005A00028358-Igcdv/6uVdMHoYOw/+koYqIwWpluYiW7@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Gregory Haskins @ 2007-07-26 15:15 UTC (permalink / raw)
  To: avi-atKUWr5tajBWk0Htik3J/w; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Thu, 2007-07-26 at 18:03 +0300, Avi Kivity wrote:
> Gregory Haskins wrote:
> > We need to provide locking around the current_vmcs/VMCS interactions to
> > protect against race conditions.
> >
> >   
> 
> Can you explain the race?

Sure.  It can happen with two VMs are running simultaneously.  Lets call
them VM-a and VM-b.  Assume the scenario: VM-a is on CPU-x, gets
migrated to CPU-y, and VM-b gets scheduled in on CPU-x.  There is a race
on CPU-x with the VMCS handling logic between the VM-b process context,
and the IPI to execute the __vcpu_clear for VM-a. 

Disabling interrupts was chosen as the sync-primitive, because the code
will always be on the CPU in question.



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] KVM: Protect race-condition between VMCS and current_vmcs on VMX hardware
@ 2007-07-26 15:40 Gregory Haskins
  0 siblings, 0 replies; 13+ messages in thread
From: Gregory Haskins @ 2007-07-26 15:40 UTC (permalink / raw)
  To: avi-atKUWr5tajBWk0Htik3J/w; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Thu, 2007-07-26 at 18:35 +0300, Avi Kivity wrote:

> A race indeed, good catch.
> 
> I think the race is only on the per_cpu(current_vmcs) variable, no?  The 
> actual vmcs ptr (as loaded by vmptrld) is handled by the processor.

Correct.

> 
> > Disabling interrupts was chosen as the sync-primitive, because the code
> > will always be on the CPU in question.
> >
> >   
> 
> Looks a bit heavy handed.  How about replacing (in __vcpu_clear())
> 
>     if (per_cpu(current_vmcs, cpu) == vcpu->vmcs)
>         per_cpu(current_vmcs, cpu) = NULL;
> 
> by
> 
>     cmpxchg_local(&per_cpu(current_vmcs, cpu), vcpu->vmcs, NULL);
> 
> ?

Hmm...possibly.  I've never worked with the cmpxchg subsystem so let me
look into it a little bit and get back to you.



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] KVM: Protect race-condition between VMCS and current_vmcs on VMX hardware
@ 2007-07-26 16:40 Gregory Haskins
  0 siblings, 0 replies; 13+ messages in thread
From: Gregory Haskins @ 2007-07-26 16:40 UTC (permalink / raw)
  To: avi-atKUWr5tajBWk0Htik3J/w; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Thu, 2007-07-26 at 19:31 +0300, Avi Kivity wrote:
> Avi Kivity wrote:
> >>
> >> Sure.  It can happen with two VMs are running simultaneously.  Lets call
> >> them VM-a and VM-b.  Assume the scenario: VM-a is on CPU-x, gets
> >> migrated to CPU-y, and VM-b gets scheduled in on CPU-x.  There is a race
> >> on CPU-x with the VMCS handling logic between the VM-b process context,
> >> and the IPI to execute the __vcpu_clear for VM-a.
> >>   
> >
> > A race indeed, good catch.
> >
> > I think the race is only on the per_cpu(current_vmcs) variable, no?  
> > The actual vmcs ptr (as loaded by vmptrld) is handled by the processor.
> 
> btw, I think the race is benign.  if __vcpu_clear() wins, vcpu_load() 
> gets to set current_vmcs and all is well.  If vcpu_load() wins, 
> __vcpu_clear() stomps on current_vmcs, but the only effect of that the 
> next time vcpu_load() is called, it issues an unnecessary vmptrld.


Hmm.. Yes I think you are right.  When I first started thinking about
this is when I thought we needed to VMCLEAR the current before the
VMPTRLD, in which case this would be a real bug.  But in light of you
setting me straight on that issue, I think this race drops away too.  We
should probably comment the code just in case current_vmcs gets more
complex in the future so it doesn't get lost ;)



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2007-07-26 23:54 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-26 14:51 [PATCH 0/2] Arch cleanup v3 Gregory Haskins
     [not found] ` <20070726144602.4847.64724.stgit-sLgBBP33vUGnsjUZhwzVf9HuzzzSOjJt@public.gmane.org>
2007-07-26 14:52   ` [PATCH 1/2] KVM: Remove arch specific components from the general code Gregory Haskins
     [not found]     ` <20070726145204.4847.53350.stgit-sLgBBP33vUGnsjUZhwzVf9HuzzzSOjJt@public.gmane.org>
2007-07-26 15:04       ` Anthony Liguori
     [not found]         ` <46A8B816.7080303-rdkfGonbjUSkNkDKm+mE6A@public.gmane.org>
2007-07-26 15:10           ` Avi Kivity
2007-07-26 17:44           ` Paul Turner
2007-07-26 23:54           ` Rusty Russell
2007-07-26 14:52   ` [PATCH 2/2] KVM: Protect race-condition between VMCS and current_vmcs on VMX hardware Gregory Haskins
     [not found]     ` <20070726145210.4847.90637.stgit-sLgBBP33vUGnsjUZhwzVf9HuzzzSOjJt@public.gmane.org>
2007-07-26 15:03       ` Avi Kivity
  -- strict thread matches above, loose matches on Subject: below --
2007-07-26 15:15 Gregory Haskins
     [not found] ` <46A882480200005A00028358-Igcdv/6uVdMHoYOw/+koYqIwWpluYiW7@public.gmane.org>
2007-07-26 15:35   ` Avi Kivity
     [not found]     ` <46A8BF26.5030802-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2007-07-26 16:31       ` Avi Kivity
2007-07-26 15:40 Gregory Haskins
2007-07-26 16:40 Gregory Haskins

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox