Xen devel - Feb 2006 - [PATCH][discuss] evtchn race condition

Keir, 
below/attached patch is necessary to allow SVM partitions to boot
unmodified guests with xen-unstable.hg c/s 8961.  c/s 8822 and 8828
(some necessary evtchn modifications) cause SVM partitions to fail with
"lost interrupt" hda error during boot. 
We currently do not understand why these modifications are necessary and
in fact, a race occurs with one part of the patch (added
evtch_pending_sel if statement). Clearly this patch is not a final
solution, and we are continuing to debug this issue.
Any comments, insight are appreciated,
Tom

Signed-off-by: Tom Woller <thomas.woller@amd.com>

diff -r 4f838d2ac31a xen/arch/x86/hvm/io.c
--- a/xen/arch/x86/hvm/io.c	Fri Feb 24 14:41:45 2006
+++ b/xen/arch/x86/hvm/io.c	Fri Feb 24 13:38:39 2006
@@ -720,10 +720,8 @@
      * Re-set the selector and master flags in case any other
notifications
      * are pending.
      */
-    if ( d->shared_info->evtchn_pending[port/BITS_PER_LONG] )
-        set_bit(port/BITS_PER_LONG,
&v->vcpu_info->evtchn_pending_sel);
-    if ( v->vcpu_info->evtchn_pending_sel )
-        v->vcpu_info->evtchn_upcall_pending = 1;
+    set_bit(port/BITS_PER_LONG, &v->vcpu_info->evtchn_pending_sel);
+    v->vcpu_info->evtchn_upcall_pending = 1;
 }
 
 void hvm_safe_block(void)
@@ -735,12 +733,15 @@
     for ( ; ; )
     {
         /* Clear master flag & selector flag so we will wake from
block. */
-        v->vcpu_info->evtchn_upcall_pending = 0;
+        if (!v->vcpu_info->evtchn_pending_sel)
+            v->vcpu_info->evtchn_upcall_pending = 0;
         clear_bit(port/BITS_PER_LONG,
&v->vcpu_info->evtchn_pending_sel);
         smp_mb__after_clear_bit();
 
         /* Event pending already? */
         if ( test_bit(port, &d->shared_info->evtchn_pending[0]) )
+            break;
+        if( v->vcpu_info->evtchn_upcall_pending )
             break;
 
         do_sched_op(SCHEDOP_block, 0);




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 24 Feb 2006, at 20:14, Woller, Thomas wrote:
> below/attached patch is necessary to allow SVM partitions to boot
> unmodified guests with xen-unstable.hg c/s 8961.  c/s 8822 and 8828
> (some necessary evtchn modifications) cause SVM partitions to fail with
> "lost interrupt" hda error during boot.
> We currently do not understand why these modifications are necessary 
> and
> in fact, a race occurs with one part of the patch (added
> evtch_pending_sel if statement). Clearly this patch is not a final
> solution, and we are continuing to debug this issue.
> Any comments, insight are appreciated,
It looks as though the patch probably disables blocking on halt 
entirely, since you unconditionally set evtchn_pending_sel and 
evtchn_upcall_pedning on exit from hvm_wait_io. This then causes you to 
immediately exit from hvm_safe_block (since the pending_sel and 
upcall_pending will always both be != 0). Replacing your patch with one 
that simply removes the call to hvm_safe_block() from 
svm_vmexit_do_hlt() would I expect have the same effect (may be worth 
confirming?).

The fact that SVM has this problem, and VMX doesn''t, maybe indicates a 
difference in HLT virtualization? For example, what happens if an 
ExtInt becomes pending for a SVM guest while the guest has interrupts 
disabled? What happens when that guest executes ''STI; HLT''?
Will the
processor inject the pending ExtInt rather than intercept with reason 
HLT?

My suspicion is that VMX and SVM maybe have subtly different behaviour 
in the above situation. I notice that neither VMX nor SVM do_hlt code 
actually checks for pending ExtInts.

By the way, we''re working on a patch to move the
''iopacket_port'' out of
the standard event-channel map. This will obviate the need to screw 
around with event selectors and so on, and I believe make the code 
rather simpler. I suspect that these changes will be orthogonal to your 
current bug though (i.e., neither help nor hinder you).

  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser wrote:
> It looks as though the patch probably disables blocking on
> halt entirely, since you unconditionally set
> evtchn_pending_sel and evtchn_upcall_pedning on exit from
> hvm_wait_io. This then causes you to immediately exit from
> hvm_safe_block (since the pending_sel and upcall_pending will
> always both be != 0). Replacing your patch with one that
> simply removes the call to hvm_safe_block() from
> svm_vmexit_do_hlt() would I expect have the same effect (may be worth
> confirming?). 
Yes - you are correct. 
removing hvm_safe_block() from the hlt function alleviates the issue
also.

Attached is simple patch to remove hvm_safe_block() as a tmp solution.

Since this is only an SVM file modification, should not affect VT code
path.

Can you apply until final solution is found?
Applies cleanly to 8991.

Tom

Signed-off-by: Tom Woller <thomas.woller@amd.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel