Xen devel - Jul 2009 - [PATCH] x86, hvm: Allow delivery of timer interrupts to VCPUs != 0.

This patch is needed for kexec/kdump since kdump may halt VCPU#0.

Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>



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

This doesn''t look too bad, given the current vpt interface we have. My
main
comment is regarding vpit_vcpu(), vrtc_vcpu() and vhpet.vcpu. Seems all of
these could unconditionally use i8259_target. I suggest also introducing
vhpet_vcpu(), and then pointing all three *_vcpu() macros at
pt_i8259_target() only. And get rid of the vhpet.vcpu field.

Would that work okay?

 -- Keir

On 03/07/2009 05:27, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> This patch is needed for kexec/kdump since kdump may halt VCPU#0.
> 
> Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>
> 


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

Keir Fraser writes:
> This doesn''t look too bad, given the current vpt interface we
have. My main
> comment is regarding vpit_vcpu(), vrtc_vcpu() and vhpet.vcpu. Seems all of
> these could unconditionally use i8259_target. I suggest also introducing
> vhpet_vcpu(), and then pointing all three *_vcpu() macros at
> pt_i8259_target() only. And get rid of the vhpet.vcpu field.
> 
> Would that work okay?
Probably no problem, but may I modify vlapic.c as follows?

diff -r 80839a223746 xen/arch/x86/hvm/vlapic.c
--- a/xen/arch/x86/hvm/vlapic.c	Wed Jul 01 20:22:29 2009 +0100
+++ b/xen/arch/x86/hvm/vlapic.c	Fri Jul 03 15:59:56 2009 +0900
@@ -809,12 +809,11 @@ void vlapic_adjust_i8259_target(struct d
     for_each_vcpu ( d, v )
         if ( __vlapic_accept_pic_intr(v) )
-            goto found;
-
-    v = d->vcpu ? d->vcpu[0] : NULL;
-
- found:
-    d->arch.hvm_domain.i8259_target = v;
+        {
+            d->arch.hvm_domain.i8259_target = v;
+            return;
+        }
 }

The reasons are:
- I''m afraid that d->arch.hvm_domain.i8259_target == NULL
- if vcpu[0] is halted and all vlapic.LVT0 are masked,
  timer doesn''t work even when vlapic will be unmasked
  not as ExtINT mode.

So, I think that the last __vlapic_accept_pic_intr''ed vcpu
should be reserved in d->arch.hvm_domain.i8259_target.

Thanks,
Kouya

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

On 03/07/2009 08:57, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> - I''m afraid that d->arch.hvm_domain.i8259_target == NULL
If VCPUj is != NULL then VCPUi is also != NULL for all i < j. So this is not
a concern: there''s always a VCPU0 if there are any VCPUs at all.
> - if vcpu[0] is halted and all vlapic.LVT0 are masked,
>   timer doesn''t work even when vlapic will be unmasked
>   not as ExtINT mode.
Not sure what you mean? If legacy IRQs are routed through the IOAPIC then it
does not matter whether LAPIC.LVT0 is masked. And __vlapic_accept_pic_intr()
correctly handles that. If virtual wire mode is not through the IOAPIC then
of course LVT0 mask does matter, but I think we have that case correct too.
> So, I think that the last __vlapic_accept_pic_intr''ed vcpu
> should be reserved in d->arch.hvm_domain.i8259_target.
I don''t think the logic needs to change.

 -- Keir



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

Hi Keir,

You are correct. I misunderstood the issue. 
With your suggestion (always use i8259_target), i8259_target in kdump
transits as: NULL=>vcpu1=>vcpu0=>vcpu1.
In this case, timer interrupts were missed in my previous patch, but
that was my fault. (I felt weird that i8259_target points vcpu0 in
spite of the halt)

I''ll post two patches per your suggestion. one is cleanup of hpet.c
and the other is for delivery of timer interrupts.

Thanks,
Kouya

Keir Fraser writes:
> On 03/07/2009 08:57, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> 
> > - I''m afraid that d->arch.hvm_domain.i8259_target == NULL
> 
> If VCPUj is != NULL then VCPUi is also != NULL for all i < j. So this is
not
> a concern: there''s always a VCPU0 if there are any VCPUs at all.
> 
> > - if vcpu[0] is halted and all vlapic.LVT0 are masked,
> >   timer doesn''t work even when vlapic will be unmasked
> >   not as ExtINT mode.
> 
> Not sure what you mean? If legacy IRQs are routed through the IOAPIC then
it
> does not matter whether LAPIC.LVT0 is masked. And
__vlapic_accept_pic_intr()
> correctly handles that. If virtual wire mode is not through the IOAPIC then
> of course LVT0 mask does matter, but I think we have that case correct too.
> 
> > So, I think that the last __vlapic_accept_pic_intr''ed vcpu
> > should be reserved in d->arch.hvm_domain.i8259_target.
> 
> I don''t think the logic needs to change.
> 
>  -- Keir
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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

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

I applied your patches, thanks. Feel free to submit something similar for
passthrough IRQ handling.

 -- Keir



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