Re: [Xen-ia64-devel] [PATCH]Make VTIdomain boot again

Re: [Xen-ia64-devel] [PATCH]Make VTIdomain boot again

[Alex Williamson]

On Thu, 2006-02-09 at 10:31 +0800, Xu, Anthony wrote:
> Keir,
> 
> This patch is intended to make VTIdomain boot again, and will not impact
> domU. Actually VTIdomain and domU can boot simultaneously after applying
> this patch, Could you please check this patch into xen-unstable.hg? This
> is a must in 3.0.1.

Hi Anthony,

   Xen 3.0.1 went out on Feb. 1st.  A couple comments

--- a/xen/arch/ia64/linux-xen/irq_ia64.c        Thu Jan 26 10:31:28 2006
+++ b/xen/arch/ia64/linux-xen/irq_ia64.c        Wed Feb  8 16:28:11 2006
@@ -169,7 +169,7 @@
         * handler needs to be able to wait for further keyboard
interrupts, which can't
         * come through until ia64_eoi() has been done.
         */
-       irq_exit();
+       xen_irq_exit(regs);
 }

>>> This needs to be in an #ifdef XEN.

-void irq_exit(void)
+void xen_irq_exit(struct pt_regs *regs)
 {
        //account_system_vtime(current);
        sub_preempt_count(IRQ_EXIT_OFFSET);
-       if (!in_interrupt() && local_softirq_pending()) {
+    if((((char *)regs) -(char *) current) !=
(IA64_STK_OFFSET-IA64_PT_REGS_SIZE))
+        return;
+
+       if (!in_interrupt()&&local_softirq_pending()) {

>>> Formatting here is inconsistent with the rest of the file and
itself.  Could we be setting some flag and adding the test to the
existing in_interrupt/local_softirq_pending check?  This looks rather
precarious.  Thanks,

	Alex  

RE: [Xen-ia64-devel] [PATCH]Make VTIdomain boot again

[Dong, Eddie]

Alex Williamson wrote:
> itself.  Could we be setting some flag and adding the test to the
> existing in_interrupt/local_softirq_pending check?  This looks rather
> precarious.  Thanks,
> 
Alex:
	Will that be funny if we embed return_to_guest check in no
matter in_interrupt() or local_softirq_pending() check? How about to add
a new MACRO return_to_guest() and check in parallel with in_interrupt()
and local_softirq_pending()?
Thx,eddie

RE: [Xen-ia64-devel] [PATCH]Make VTIdomain boot again

[Xu, Anthony]

Williamson,

This patch is just a workaround, the essential cause is, the soft_IRQ should be done at the path of ia64_leave_kernel which aligned with native linux and XEN/X86, instead of being invoking in irq_exit function.

What's your opinion?

Thanks,
-Anthony 

>-----Original Message-----
>From: Alex Williamson [mailto:alex.williamson@hp.com]
>Sent: 2006年2月9日 11:32
>To: Xu, Anthony
>Cc: xen-devel@lists.xensource.com; xen-ia64-devel@lists.xensource.com
>Subject: Re: [Xen-ia64-devel] [Xen-devel] [PATCH]Make VTIdomain boot again
>
>On Thu, 2006-02-09 at 10:31 +0800, Xu, Anthony wrote:
>> Keir,
>>
>> This patch is intended to make VTIdomain boot again, and will not impact
>> domU. Actually VTIdomain and domU can boot simultaneously after applying
>> this patch, Could you please check this patch into xen-unstable.hg? This
>> is a must in 3.0.1.
>
>Hi Anthony,
>
>   Xen 3.0.1 went out on Feb. 1st.  A couple comments
>
>--- a/xen/arch/ia64/linux-xen/irq_ia64.c        Thu Jan 26 10:31:28 2006
>+++ b/xen/arch/ia64/linux-xen/irq_ia64.c        Wed Feb  8 16:28:11 2006
>@@ -169,7 +169,7 @@
>         * handler needs to be able to wait for further keyboard
>interrupts, which can't
>         * come through until ia64_eoi() has been done.
>         */
>-       irq_exit();
>+       xen_irq_exit(regs);
> }
>
>>>> This needs to be in an #ifdef XEN.
>
>-void irq_exit(void)
>+void xen_irq_exit(struct pt_regs *regs)
> {
>        //account_system_vtime(current);
>        sub_preempt_count(IRQ_EXIT_OFFSET);
>-       if (!in_interrupt() && local_softirq_pending()) {
>+    if((((char *)regs) -(char *) current) !=
>(IA64_STK_OFFSET-IA64_PT_REGS_SIZE))
>+        return;
>+
>+       if (!in_interrupt()&&local_softirq_pending()) {
>
>>>> Formatting here is inconsistent with the rest of the file and
>itself.  Could we be setting some flag and adding the test to the
>existing in_interrupt/local_softirq_pending check?  This looks rather
>precarious.  Thanks,
>
>	Alex

RE: [Xen-ia64-devel] [PATCH]Make VTIdomain boot again

[Xu, Anthony]

[-- Attachment #1: Type: text/plain, Size: 2384 bytes --]

Hi all,

Do_softirq is invoked at the path of ia64_leave_kernel instead of being invoking in function irq_exit, which is aligned with XEN/X86 and avoids __enter_schedule being reentered. VTIdomain can boot again after applying this patch.

BTW, a small bug is fixed.
- (p6)	br.call.sptk.many b0=deliver_pending_interrupt
+(pUStk)	br.call.sptk.many b0=deliver_pending_interrupt
Since P6 is scratch register, after returning from deliver_pending_interrupt, p6 may be changed. But p6 is also used by subsequent code and should not be changed. 

Signed-off-by Anthony Xu <anthony.xu@intel.com>

Thanks,
-Anthony 

>-----Original Message-----
>From: Alex Williamson [mailto:alex.williamson@hp.com]
>Sent: 2006年2月9日 11:32
>To: Xu, Anthony
>Cc: xen-devel@lists.xensource.com; xen-ia64-devel@lists.xensource.com
>Subject: Re: [Xen-ia64-devel] [Xen-devel] [PATCH]Make VTIdomain boot again
>
>On Thu, 2006-02-09 at 10:31 +0800, Xu, Anthony wrote:
>> Keir,
>>
>> This patch is intended to make VTIdomain boot again, and will not impact
>> domU. Actually VTIdomain and domU can boot simultaneously after applying
>> this patch, Could you please check this patch into xen-unstable.hg? This
>> is a must in 3.0.1.
>
>Hi Anthony,
>
>   Xen 3.0.1 went out on Feb. 1st.  A couple comments
>
>--- a/xen/arch/ia64/linux-xen/irq_ia64.c        Thu Jan 26 10:31:28 2006
>+++ b/xen/arch/ia64/linux-xen/irq_ia64.c        Wed Feb  8 16:28:11 2006
>@@ -169,7 +169,7 @@
>         * handler needs to be able to wait for further keyboard
>interrupts, which can't
>         * come through until ia64_eoi() has been done.
>         */
>-       irq_exit();
>+       xen_irq_exit(regs);
> }
>
>>>> This needs to be in an #ifdef XEN.
>
>-void irq_exit(void)
>+void xen_irq_exit(struct pt_regs *regs)
> {
>        //account_system_vtime(current);
>        sub_preempt_count(IRQ_EXIT_OFFSET);
>-       if (!in_interrupt() && local_softirq_pending()) {
>+    if((((char *)regs) -(char *) current) !=
>(IA64_STK_OFFSET-IA64_PT_REGS_SIZE))
>+        return;
>+
>+       if (!in_interrupt()&&local_softirq_pending()) {
>
>>>> Formatting here is inconsistent with the rest of the file and
>itself.  Could we be setting some flag and adding the test to the
>existing in_interrupt/local_softirq_pending check?  This looks rather
>precarious.  Thanks,
>
>	Alex

[-- Attachment #2: dosoftirq.patch --]
[-- Type: application/octet-stream, Size: 1858 bytes --]

# HG changeset patch
# User xxu10@localhost.localdomain
# Node ID 89cb1daa3f31e2984e3f734a701108bdba13fa8b
# Parent  2710f2fe624a5103d198e6685e250c6678f34ec1
soft_IRQ is done at the path of ia64_leave_kernel instead of being invoking in function irq_exit, which is aligned with XEN/X86 and make VTIdomain boot again.

Signed-off-by Anthony Xu <anthony.xu@intel.com>

diff -r 2710f2fe624a -r 89cb1daa3f31 xen/arch/ia64/linux-xen/entry.S
--- a/xen/arch/ia64/linux-xen/entry.S	Tue Feb  7 11:59:27 2006
+++ b/xen/arch/ia64/linux-xen/entry.S	Thu Feb  9 08:46:05 2006
@@ -895,6 +895,10 @@
 #endif
 .work_processed_kernel:
 #ifdef XEN
+(pUStk) ssm psr.i
+(pUStk)    br.call.sptk.many b0=process_soft_irq
+(pUStk) rsm psr.i
+    ;;
 	alloc loc0=ar.pfs,0,1,1,0
 	adds out0=16,r12
     adds r7 = PT(EML_UNAT)+16,r12
@@ -907,7 +911,7 @@
 (p8) br.sptk.few leave_kernel_self
 	;; 
 #endif
-(p6)	br.call.sptk.many b0=deliver_pending_interrupt
+(pUStk)	br.call.sptk.many b0=deliver_pending_interrupt
     ;;
 	mov ar.pfs=loc0
 	mov ar.unat=r7  /* load eml_unat  */
diff -r 2710f2fe624a -r 89cb1daa3f31 xen/arch/ia64/xen/xenirq.c
--- a/xen/arch/ia64/xen/xenirq.c	Tue Feb  7 11:59:27 2006
+++ b/xen/arch/ia64/xen/xenirq.c	Thu Feb  9 08:46:05 2006
@@ -64,11 +64,23 @@
 {
 	//account_system_vtime(current);
 	sub_preempt_count(IRQ_EXIT_OFFSET);
+//	if (!in_interrupt() && local_softirq_pending()) {
+//		add_preempt_count(SOFTIRQ_OFFSET);
+//		do_softirq();
+//		sub_preempt_count(SOFTIRQ_OFFSET);
+//	}
+	//preempt_enable_no_resched();
+}
+
+// ONLY gets called from ia64_leave_kernel
+// ONLY call with interrupts enable
+void process_soft_irq()
+{
 	if (!in_interrupt() && local_softirq_pending()) {
 		add_preempt_count(SOFTIRQ_OFFSET);
 		do_softirq();
 		sub_preempt_count(SOFTIRQ_OFFSET);
 	}
-	//preempt_enable_no_resched();
 }
+
 /* end from linux/kernel/softirq.c */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [Xen-ia64-devel] [PATCH]Make VTIdomain boot again

[Keir Fraser]

On 9 Feb 2006, at 09:32, Xu, Anthony wrote:

> Do_softirq is invoked at the path of ia64_leave_kernel instead of 
> being invoking in function irq_exit, which is aligned with XEN/X86 and 
> avoids __enter_schedule being reentered. VTIdomain can boot again 
> after applying this patch.
>
> BTW, a small bug is fixed.
> - (p6)	br.call.sptk.many b0=deliver_pending_interrupt
> +(pUStk)	br.call.sptk.many b0=deliver_pending_interrupt
> Since P6 is scratch register, after returning from 
> deliver_pending_interrupt, p6 may be changed. But p6 is also used by 
> subsequent code and should not be changed.
>
> Signed-off-by Anthony Xu <anthony.xu@intel.com>

Should I check in this patch instead of your earlier one (which used 
xen_irq_exit instead or irq_exit?).

  -- Keir

RE: [Xen-ia64-devel] [PATCH]Make VTIdomain boot again

[Alex Williamson]

On Thu, 2006-02-09 at 15:07 +0800, Dong, Eddie wrote:
> Alex Williamson wrote:
> > itself.  Could we be setting some flag and adding the test to the
> > existing in_interrupt/local_softirq_pending check?  This looks rather
> > precarious.  Thanks,
> > 
> Alex:
> 	Will that be funny if we embed return_to_guest check in no
> matter in_interrupt() or local_softirq_pending() check? How about to add
> a new MACRO return_to_guest() and check in parallel with in_interrupt()
> and local_softirq_pending()?

   Yeah, that's what I was suggesting.  Thanks,

	Alex
-- 
Alex Williamson                             HP Linux & Open Source Lab

RE: [Xen-ia64-devel] [PATCH]Make VTIdomain boot again

[Alex Williamson]

On Thu, 2006-02-09 at 15:14 +0800, Xu, Anthony wrote:
> Williamson,
> 
> This patch is just a workaround, the essential cause is, the soft_IRQ
> should be done at the path of ia64_leave_kernel which aligned with
> native linux and XEN/X86, instead of being invoking in irq_exit
> function.
> 
> What's your opinion?

   I'm afraid workarounds always last longer than expected, so we should
still try to do this cleanly.  Xen 3.0.1 is already out so the time
pressure to get this fixed is lessened.  Being ia64 specific Xen code, I
think we should try to do it right, get it into xen-ia64-unstable, then
push it to xen-unstable.  Thanks,

	Alex

-- 
Alex Williamson                             HP Linux & Open Source Lab

Re: [Xen-ia64-devel] [PATCH]Make VTIdomain boot again

[Keir Fraser]

On 9 Feb 2006, at 15:17, Alex Williamson wrote:

>> 	This are 2 seperate one, so please check in both if possible.
>
>    I'm confused, these patches are completely overlapping, aren't they?
> We can't do both.  Thanks,

As I noticed when I just checked them both in. Looks to me as though 
the second is the preferable cleaner one, so I checked it in on top of 
the first one.

As you say, it's probably best for you to do any further cleanup in the 
ia64-unstable tree. :-)

  -- Keir

RE: [Xen-ia64-devel] [PATCH]Make VTIdomain boot again

[Xu, Anthony]

Keir,

Sorry for the confusion, Pls delete the earlier one, the second one is an update per alex's request. So Pls only check in the second patch.

Thanks,
-Anthony 


>-----Original Message-----
>From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk]
>Sent: 2006年2月9日 23:25
>To: Alex Williamson
>Cc: xen-devel@lists.xensource.com; Dong, Eddie; Xu, Anthony;
>xen-ia64-devel@lists.xensource.com
>Subject: Re: [Xen-ia64-devel] [Xen-devel] [PATCH]Make VTIdomain boot again
>
>
>On 9 Feb 2006, at 15:17, Alex Williamson wrote:
>
>>> 	This are 2 seperate one, so please check in both if possible.
>>
>>    I'm confused, these patches are completely overlapping, aren't they?
>> We can't do both.  Thanks,
>
>As I noticed when I just checked them both in. Looks to me as though
>the second is the preferable cleaner one, so I checked it in on top of
>the first one.
>
>As you say, it's probably best for you to do any further cleanup in the
>ia64-unstable tree. :-)
>
>  -- Keir