[PATCH] Don't free irqaction for com irq when release irq.

[PATCH] Don't free irqaction for com irq when release irq.

[Zhang, Xiantao]

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

# HG changeset patch
# User root@localhost.localdomain
# Date 1251916103 14400
# Node ID 49e847aed58dde35f8a0f909999d01d97be6f531
# Parent  3b7cbf32fee909d860daacf70b8c3b97eaf036b5
x86: com devices's irqaction shouldn't free.

Since irqs of serial devices are initialized in early Xen and
its irqaction is not allocated from heap, so doesn't need free
in release irq logic.
Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>

diff -r 3b7cbf32fee9 -r 49e847aed58d xen/arch/x86/irq.c
--- a/xen/arch/x86/irq.c	Mon Aug 31 10:54:32 2009 +0100
+++ b/xen/arch/x86/irq.c	Wed Sep 02 14:28:23 2009 -0400
@@ -564,7 +564,7 @@ void release_irq(unsigned int irq)
     /* Wait to make sure it's not being used on another CPU */
     do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
 
-    if (action)
+    if ( !COM_IRQ(irq) && action )
         xfree(action);
 }
 
diff -r 3b7cbf32fee9 -r 49e847aed58d xen/arch/x86/setup.c
--- a/xen/arch/x86/setup.c	Mon Aug 31 10:54:32 2009 +0100
+++ b/xen/arch/x86/setup.c	Wed Sep 02 14:28:23 2009 -0400
@@ -464,10 +464,10 @@ void __init __start_xen(unsigned long mb
 
     /* We initialise the serial devices very early so we can get debugging. */
     ns16550.io_base = 0x3f8;
-    ns16550.irq     = 4;
+    ns16550.irq     = COM1_IRQ;
     ns16550_init(0, &ns16550);
     ns16550.io_base = 0x2f8;
-    ns16550.irq     = 3;
+    ns16550.irq     = COM2_IRQ;
     ns16550_init(1, &ns16550);
     console_init_preirq();
 
diff -r 3b7cbf32fee9 -r 49e847aed58d xen/include/asm-x86/irq.h
--- a/xen/include/asm-x86/irq.h	Mon Aug 31 10:54:32 2009 +0100
+++ b/xen/include/asm-x86/irq.h	Wed Sep 02 14:28:23 2009 -0400
@@ -26,6 +26,11 @@
 #define MAX_NR_IRQS (2 * MAX_GSI_IRQS)
 
 #define irq_cfg(irq)        &irq_cfg[(irq)]
+
+#define COM1_IRQ 4
+#define COM2_IRQ 3
+
+#define COM_IRQ(irq) ((irq) == COM1_IRQ || (irq) == COM2_IRQ)
 
 struct irq_cfg {
         int  vector;

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

# HG changeset patch
# User root@localhost.localdomain
# Date 1251916103 14400
# Node ID 49e847aed58dde35f8a0f909999d01d97be6f531
# Parent  3b7cbf32fee909d860daacf70b8c3b97eaf036b5
x86: com devices's irqaction shouldn't free.

Since irqs of serial devices are intialized in early Xen and
its irqaction is not allocated from heap, so doesn't need free
in release irq logic.

Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>

diff -r 3b7cbf32fee9 -r 49e847aed58d xen/arch/x86/irq.c
--- a/xen/arch/x86/irq.c	Mon Aug 31 10:54:32 2009 +0100
+++ b/xen/arch/x86/irq.c	Wed Sep 02 14:28:23 2009 -0400
@@ -564,7 +564,7 @@ void release_irq(unsigned int irq)
     /* Wait to make sure it's not being used on another CPU */
     do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
 
-    if (action)
+    if ( !COM_IRQ(irq) && action )
         xfree(action);
 }
 
diff -r 3b7cbf32fee9 -r 49e847aed58d xen/arch/x86/setup.c
--- a/xen/arch/x86/setup.c	Mon Aug 31 10:54:32 2009 +0100
+++ b/xen/arch/x86/setup.c	Wed Sep 02 14:28:23 2009 -0400
@@ -464,10 +464,10 @@ void __init __start_xen(unsigned long mb
 
     /* We initialise the serial devices very early so we can get debugging. */
     ns16550.io_base = 0x3f8;
-    ns16550.irq     = 4;
+    ns16550.irq     = COM1_IRQ;
     ns16550_init(0, &ns16550);
     ns16550.io_base = 0x2f8;
-    ns16550.irq     = 3;
+    ns16550.irq     = COM2_IRQ;
     ns16550_init(1, &ns16550);
     console_init_preirq();
 
diff -r 3b7cbf32fee9 -r 49e847aed58d xen/include/asm-x86/irq.h
--- a/xen/include/asm-x86/irq.h	Mon Aug 31 10:54:32 2009 +0100
+++ b/xen/include/asm-x86/irq.h	Wed Sep 02 14:28:23 2009 -0400
@@ -26,6 +26,11 @@
 #define MAX_NR_IRQS (2 * MAX_GSI_IRQS)
 
 #define irq_cfg(irq)        &irq_cfg[(irq)]
+
+#define COM1_IRQ 4
+#define COM2_IRQ 3
+
+#define COM_IRQ(irq) ((irq) == COM1_IRQ || (irq) == COM2_IRQ)
 
 struct irq_cfg {
         int  vector;

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

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

Re: [PATCH] Don't free irqaction for com irq when release irq.

[Keir Fraser]

On 03/09/2009 05:14, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:

> # HG changeset patch
> # User root@localhost.localdomain
> # Date 1251916103 14400
> # Node ID 49e847aed58dde35f8a0f909999d01d97be6f531
> # Parent  3b7cbf32fee909d860daacf70b8c3b97eaf036b5
> x86: com devices's irqaction shouldn't free.
> 
> Since irqs of serial devices are initialized in early Xen and
> its irqaction is not allocated from heap, so doesn't need free
> in release irq logic.
> Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>

Does this fix host S3?

 -- Keir

RE: Re: [PATCH] Don't free irqaction for com irq when release irq.

[Cui, Dexuan]

Just now I found another issue about Dom0 S3 resume when VT-d is enabled.  I'll send a patch for it.

Thanks,
-- Dexuan

-----Original Message-----
From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
Sent: 2009?9?3? 14:14
To: Zhang, Xiantao; Xen
Subject: [Xen-devel] Re: [PATCH] Don't free irqaction for com irq when release irq.

On 03/09/2009 05:14, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:

> # HG changeset patch
> # User root@localhost.localdomain
> # Date 1251916103 14400
> # Node ID 49e847aed58dde35f8a0f909999d01d97be6f531
> # Parent  3b7cbf32fee909d860daacf70b8c3b97eaf036b5
> x86: com devices's irqaction shouldn't free.
> 
> Since irqs of serial devices are initialized in early Xen and
> its irqaction is not allocated from heap, so doesn't need free
> in release irq logic.
> Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>

Does this fix host S3?

 -- Keir



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

RE: [PATCH] Don't free irqaction for com irq when release irq.

[Zhang, Xiantao]

No, host S3 issue is caused by ioapic suspend/resume in dom0.  IMO, dom0 shouldn't perform S3 for ioapic , right ? After fixing that issue, domain S3 works well. 
Xiantao

-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] 
Sent: Thursday, September 03, 2009 2:14 PM
To: Zhang, Xiantao; Xen
Subject: Re: [PATCH] Don't free irqaction for com irq when release irq.

On 03/09/2009 05:14, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:

> # HG changeset patch
> # User root@localhost.localdomain
> # Date 1251916103 14400
> # Node ID 49e847aed58dde35f8a0f909999d01d97be6f531
> # Parent  3b7cbf32fee909d860daacf70b8c3b97eaf036b5
> x86: com devices's irqaction shouldn't free.
> 
> Since irqs of serial devices are initialized in early Xen and
> its irqaction is not allocated from heap, so doesn't need free
> in release irq logic.
> Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>

Does this fix host S3?

 -- Keir

Re: RE: [PATCH] Don't free irqaction for com irq when release irq.

[Christoph Egger]

On Thursday 03 September 2009 11:38:35 Zhang, Xiantao wrote:
> No, host S3 issue is caused by ioapic suspend/resume in dom0.  IMO, dom0
> shouldn't perform S3 for ioapic , right ?

dom0 initializes and programs ioapic. In short: ioapic is under control of 
dom0. Please explain why dom0 shouldn't do ioapic suspend/resume.

Christoph

> After fixing that issue, domain 
> S3 works well. Xiantao
>
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
> Sent: Thursday, September 03, 2009 2:14 PM
> To: Zhang, Xiantao; Xen
> Subject: Re: [PATCH] Don't free irqaction for com irq when release irq.
>
> On 03/09/2009 05:14, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:
> > # HG changeset patch
> > # User root@localhost.localdomain
> > # Date 1251916103 14400
> > # Node ID 49e847aed58dde35f8a0f909999d01d97be6f531
> > # Parent  3b7cbf32fee909d860daacf70b8c3b97eaf036b5
> > x86: com devices's irqaction shouldn't free.
> >
> > Since irqs of serial devices are initialized in early Xen and
> > its irqaction is not allocated from heap, so doesn't need free
> > in release irq logic.
> > Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
>
> Does this fix host S3?
>
>  -- Keir
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: RE: [PATCH] Don't free irqaction for com irq when release irq.

[Jan Beulich]

>>> Christoph Egger <Christoph.Egger@amd.com> 03.09.09 12:04 >>>
>On Thursday 03 September 2009 11:38:35 Zhang, Xiantao wrote:
>> No, host S3 issue is caused by ioapic suspend/resume in dom0.  IMO, dom0
>> shouldn't perform S3 for ioapic , right ?
>
>dom0 initializes and programs ioapic. In short: ioapic is under control of 
>dom0. Please explain why dom0 shouldn't do ioapic suspend/resume.

Because Xen itself does so (and has to for its own interrupts to work).

Jan

RE: RE: [PATCH] Don't free irqaction for com irq when release irq.

[Zhang, Xiantao]

I think Jan also answered your question.   Dom0 shouldn't touch ioapic after initialization time any more.  That is to say, maybe we can find a way to get rid of ioapic from dom0.  Actually I can't see why dom0 cares so much about ioapic.   Jeremy, do you know the reason ?  IMO, dom0 should only cares about GSI and pirq mapping, but currently GSI is always equal to pirq in dom0, so no reason to keep ioapic in dom0. 
Xiantao

-----Original Message-----
From: Christoph Egger [mailto:Christoph.Egger@amd.com] 
Sent: Thursday, September 03, 2009 6:05 PM
To: xen-devel@lists.xensource.com
Cc: Zhang, Xiantao; Keir Fraser
Subject: Re: [Xen-devel] RE: [PATCH] Don't free irqaction for com irq when release irq.

On Thursday 03 September 2009 11:38:35 Zhang, Xiantao wrote:
> No, host S3 issue is caused by ioapic suspend/resume in dom0.  IMO, dom0
> shouldn't perform S3 for ioapic , right ?

dom0 initializes and programs ioapic. In short: ioapic is under control of 
dom0. Please explain why dom0 shouldn't do ioapic suspend/resume.

Christoph

> After fixing that issue, domain 
> S3 works well. Xiantao
>
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
> Sent: Thursday, September 03, 2009 2:14 PM
> To: Zhang, Xiantao; Xen
> Subject: Re: [PATCH] Don't free irqaction for com irq when release irq.
>
> On 03/09/2009 05:14, "Zhang, Xiantao" <xiantao.zhang@intel.com> wrote:
> > # HG changeset patch
> > # User root@localhost.localdomain
> > # Date 1251916103 14400
> > # Node ID 49e847aed58dde35f8a0f909999d01d97be6f531
> > # Parent  3b7cbf32fee909d860daacf70b8c3b97eaf036b5
> > x86: com devices's irqaction shouldn't free.
> >
> > Since irqs of serial devices are initialized in early Xen and
> > its irqaction is not allocated from heap, so doesn't need free
> > in release irq logic.
> > Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
>
> Does this fix host S3?
>
>  -- Keir
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd, Thomas M. McCoy, Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

RE: [PATCH] Don't free irqaction for com irq when release irq.

[Dan Magenheimer]

Unless I missed something, it looks like Keir's modified
version of this patch (20153) neglects to set the
free_on_release=0 for the com irqs.

> -----Original Message-----
> From: Zhang, Xiantao [mailto:xiantao.zhang@intel.com]
> Sent: Wednesday, September 02, 2009 10:15 PM
> To: Xen; Keir Fraser
> Subject: [Xen-devel] [PATCH] Don't free irqaction for com irq when
> release irq.
> 
> 
> # HG changeset patch
> # User root@localhost.localdomain
> # Date 1251916103 14400
> # Node ID 49e847aed58dde35f8a0f909999d01d97be6f531
> # Parent  3b7cbf32fee909d860daacf70b8c3b97eaf036b5
> x86: com devices's irqaction shouldn't free.
> 
> Since irqs of serial devices are initialized in early Xen and
> its irqaction is not allocated from heap, so doesn't need free
> in release irq logic.
> Signed-off-by: Xiantao Zhang <xiantao.zhang@intel.com>
> 
> diff -r 3b7cbf32fee9 -r 49e847aed58d xen/arch/x86/irq.c
> --- a/xen/arch/x86/irq.c	Mon Aug 31 10:54:32 2009 +0100
> +++ b/xen/arch/x86/irq.c	Wed Sep 02 14:28:23 2009 -0400
> @@ -564,7 +564,7 @@ void release_irq(unsigned int irq)
>      /* Wait to make sure it's not being used on another CPU */
>      do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
>  
> -    if (action)
> +    if ( !COM_IRQ(irq) && action )
>          xfree(action);
>  }
>  
> diff -r 3b7cbf32fee9 -r 49e847aed58d xen/arch/x86/setup.c
> --- a/xen/arch/x86/setup.c	Mon Aug 31 10:54:32 2009 +0100
> +++ b/xen/arch/x86/setup.c	Wed Sep 02 14:28:23 2009 -0400
> @@ -464,10 +464,10 @@ void __init __start_xen(unsigned long mb
>  
>      /* We initialise the serial devices very early so we can 
> get debugging. */
>      ns16550.io_base = 0x3f8;
> -    ns16550.irq     = 4;
> +    ns16550.irq     = COM1_IRQ;
>      ns16550_init(0, &ns16550);
>      ns16550.io_base = 0x2f8;
> -    ns16550.irq     = 3;
> +    ns16550.irq     = COM2_IRQ;
>      ns16550_init(1, &ns16550);
>      console_init_preirq();
>  
> diff -r 3b7cbf32fee9 -r 49e847aed58d xen/include/asm-x86/irq.h
> --- a/xen/include/asm-x86/irq.h	Mon Aug 31 10:54:32 2009 +0100
> +++ b/xen/include/asm-x86/irq.h	Wed Sep 02 14:28:23 2009 -0400
> @@ -26,6 +26,11 @@
>  #define MAX_NR_IRQS (2 * MAX_GSI_IRQS)
>  
>  #define irq_cfg(irq)        &irq_cfg[(irq)]
> +
> +#define COM1_IRQ 4
> +#define COM2_IRQ 3
> +
> +#define COM_IRQ(irq) ((irq) == COM1_IRQ || (irq) == COM2_IRQ)
>  
>  struct irq_cfg {
>          int  vector;

RE: [PATCH] Don't free irqaction for com irq when release irq.

[Jan Beulich]

>>> Dan Magenheimer <dan.magenheimer@oracle.com> 03.09.09 17:48 >>>
>Unless I missed something, it looks like Keir's modified
>version of this patch (20153) neglects to set the
>free_on_release=0 for the com irqs.

ns16550_com[] starts out as being all zero, hence there's no need to
explicitly clear that flag.

Jan

RE: [PATCH] Don't free irqaction for com irq when release irq.

[Dan Magenheimer]

Oops yes, thanks for pointing that out.  This won't
be the first or last time I've been fooled by that
convention.

For the sake of good programming practices and
(self-)documentation, since "don't free" is
a rare exception rather than the rule, may
I suggest changing the name/sense of the variable
in the struct to "dont_free", for which zero is
a reasonable default, and just enabling
it for the com irqs?

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@novell.com]
> Sent: Thursday, September 03, 2009 10:03 AM
> To: Dan Magenheimer
> Cc: Keir Fraser; Xiantao Zhang; Xen
> Subject: RE: [Xen-devel] [PATCH] Don't free irqaction for com irq when
> release irq.
> 
> 
> >>> Dan Magenheimer <dan.magenheimer@oracle.com> 03.09.09 17:48 >>>
> >Unless I missed something, it looks like Keir's modified
> >version of this patch (20153) neglects to set the
> >free_on_release=0 for the com irqs.
> 
> ns16550_com[] starts out as being all zero, hence there's no need to
> explicitly clear that flag.
> 
> Jan
> 
>

Re: [PATCH] Don't free irqaction for com irq when release irq.

[Keir Fraser]

On 03/09/2009 17:26, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:

> Oops yes, thanks for pointing that out.  This won't
> be the first or last time I've been fooled by that
> convention.
> 
> For the sake of good programming practices and
> (self-)documentation, since "don't free" is
> a rare exception rather than the rule, may
> I suggest changing the name/sense of the variable
> in the struct to "dont_free", for which zero is
> a reasonable default, and just enabling
> it for the com irqs?

All irqactions which *should* be freed are allocated in exactly one place.
It is therefore sensible to make that the 'exception' which sets a flag to
1. The variou sother users of the setup_irq() interface then do not need to
be changed, as they get a default flag value of zero.

 -- Keir

Re: RE: [PATCH] Don't free irqaction for com irq when release irq.

[Jeremy Fitzhardinge]

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

On 09/03/09 05:53, Zhang, Xiantao wrote:
> I think Jan also answered your question.   Dom0 shouldn't touch ioapic after initialization time any more.  That is to say, maybe we can find a way to get rid of ioapic from dom0.  Actually I can't see why dom0 cares so much about ioapic.   Jeremy, do you know the reason ?  IMO, dom0 should only cares about GSI and pirq mapping, but currently GSI is always equal to pirq in dom0, so no reason to keep ioapic in dom0. 
>   

Yes, I agree.  I have a prototype branch:
git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git
#rebase/dom0/new-interrupt-routing which replaces the whole interrupt
handing subsystem to avoid any direct interactions with the IO APICs.

I add a new hypercall to directly bind a gsi to a given pirq (which has
some similarities to your patch).  I've attached it below. 
(new-interrupt-routing shouldn't need this patch to function, however.)

Unfortunately I haven't had a chance to work on this lately, but when I
last tried it, it hung shortly after initializing ACPI.  I didn't get
much further than that.  I do think, however, that this is this right
way to go for dom0, esp with regard to upstreaming.

(The second patch is to allow dom0 to get Xen's acpi interrupt model so
they can always be consistent rather than independently arriving at the
same result - not not.)

    J


[-- Attachment #2: xen-new-ioapic.patch --]
[-- Type: text/x-patch, Size: 4598 bytes --]

diff -r 02003bee3e80 -r c9df3571e4d2 xen/arch/x86/mpparse.c
--- a/xen/arch/x86/mpparse.c	Thu Jun 25 18:31:10 2009 +0100
+++ b/xen/arch/x86/mpparse.c	Tue Jul 07 16:37:03 2009 -0700
@@ -871,7 +871,7 @@
 } mp_ioapic_routing[MAX_IO_APICS];
 
 
-static int mp_find_ioapic (
+int mp_find_ioapic (
 	int			gsi)
 {
 	int			i = 0;
@@ -887,7 +887,13 @@
 
 	return -1;
 }
-	
+
+int mp_find_pin (
+	int			ioapic,
+	int			gsi)
+{
+	return gsi - mp_ioapic_routing[ioapic].gsi_base;
+}	
 
 void __init mp_register_ioapic (
 	u8			id, 
@@ -962,7 +968,7 @@
 	ioapic = mp_find_ioapic(gsi);
 	if (ioapic < 0)
 		return;
-	pin = gsi - mp_ioapic_routing[ioapic].gsi_base;
+	pin = mp_find_pin(ioapic, gsi);
 
 	/*
 	 * TBD: This check is for faulty timer entries, where the override
@@ -1084,7 +1090,7 @@
 		return gsi;
 	}
 
-	ioapic_pin = gsi - mp_ioapic_routing[ioapic].gsi_base;
+	ioapic_pin = mp_find_pin(ioapic, gsi);
 
 	if (ioapic_renumber_irq)
 		gsi = ioapic_renumber_irq(ioapic, gsi);
diff -r 02003bee3e80 -r c9df3571e4d2 xen/arch/x86/physdev.c
--- a/xen/arch/x86/physdev.c	Thu Jun 25 18:31:10 2009 +0100
+++ b/xen/arch/x86/physdev.c	Tue Jul 07 16:37:03 2009 -0700
@@ -361,6 +361,63 @@
         break;
     }
 
+    case PHYSDEVOP_route_gsi: {
+        struct physdev_route_gsi route_gsi;
+        unsigned gsi;
+        int vector;
+        int ioapic, pin;
+        int level, active_low;
+
+        ret = -EFAULT;
+        if ( copy_from_guest(&route_gsi, arg, 1) != 0 )
+            break;
+
+        ret = -EPERM;
+        if ( !IS_PRIV(v->domain) )
+            break;
+
+        ret = xsm_assign_vector(v->domain, route_gsi.pirq);
+        if ( ret )
+            break;
+
+        irq = route_gsi.pirq;
+        ret = -EINVAL;
+        if ( (irq < 0) || (irq >= nr_irqs) )
+            break;
+
+        vector = irq_to_vector(irq);
+        if ( vector < 0 ) {
+            ret = vector;
+            break;
+        }
+
+        spin_lock(&pcidevs_lock);
+        spin_lock(&dom0->event_lock);
+        ret = map_domain_pirq(dom0, irq, vector,
+                              MAP_PIRQ_TYPE_GSI, NULL);
+        spin_unlock(&dom0->event_lock);
+        spin_unlock(&pcidevs_lock);
+
+        if ( ret < 0 )
+            break;
+
+        gsi = route_gsi.gsi;
+
+        ret = -ENOENT;
+        ioapic = mp_find_ioapic(gsi);
+        if ( ioapic < 0 )
+            break;
+
+        pin = mp_find_pin(ioapic, gsi);
+
+        level = (route_gsi.flags & XENROUTEGSI_level) != 0;
+        active_low = (route_gsi.flags & XENROUTEGSI_active_low) != 0;
+
+        ret = io_apic_set_pci_routing(ioapic, pin, irq, level, active_low);
+
+        break;
+    }
+
     case PHYSDEVOP_set_iopl: {
         struct physdev_set_iopl set_iopl;
         ret = -EFAULT;
diff -r 02003bee3e80 -r c9df3571e4d2 xen/include/asm-x86/mpspec.h
--- a/xen/include/asm-x86/mpspec.h	Thu Jun 25 18:31:10 2009 +0100
+++ b/xen/include/asm-x86/mpspec.h	Tue Jul 07 16:37:03 2009 -0700
@@ -34,6 +34,10 @@
 extern void mp_override_legacy_irq (u8 bus_irq, u8 polarity, u8 trigger, u32 gsi);
 extern void mp_config_acpi_legacy_irqs (void);
 extern int mp_register_gsi (u32 gsi, int edge_level, int active_high_low);
+
+extern int mp_find_ioapic (int gsi);
+extern int mp_find_pin (int ioapic, int gsi);
+
 #endif /* CONFIG_ACPI */
 
 #define PHYSID_ARRAY_SIZE	BITS_TO_LONGS(MAX_APICS)
diff -r 02003bee3e80 -r c9df3571e4d2 xen/include/public/physdev.h
--- a/xen/include/public/physdev.h	Thu Jun 25 18:31:10 2009 +0100
+++ b/xen/include/public/physdev.h	Tue Jul 07 16:37:03 2009 -0700
@@ -210,6 +210,31 @@
 typedef struct physdev_manage_pci_ext physdev_manage_pci_ext_t;
 DEFINE_XEN_GUEST_HANDLE(physdev_manage_pci_ext_t);
 
+/* 
+ * Allocate a vector for an irq (if necessary) and set up routing for
+ * the GSI in the appropriate IO/APIC.  May be used to update the
+ * IO/APIC routing once it has been initially set.
+ *
+ * @arg = pointer to physdev_route_gsi structure.
+ */
+#define PHYSDEVOP_route_gsi		21
+struct physdev_route_gsi {
+	/* IN */
+	uint32_t pirq;
+	uint32_t gsi;
+	uint32_t flags;		/* XENROUTEGSI_* */
+};
+
+typedef struct physdev_route_gsi physdev_route_gsi_t;
+DEFINE_XEN_GUEST_HANDLE(physdev_route_gsi_t);
+
+#define _XENROUTEGSI_level      (0)
+#define  XENROUTEGSI_level      (1U<<_XENROUTEGSI_level)
+#define _XENROUTEGSI_active_low (1)
+#define  XENROUTEGSI_active_low (1U<<_XENROUTEGSI_active_low)
+#define _XENROUTEGSI_masked     (2)
+#define  XENROUTEGSI_masked     (1U<<_XENROUTEGSI_masked)
+
 /*
  * Argument to physdev_op_compat() hypercall. Superceded by new physdev_op()
  * hypercall since 0x00030202.

[-- Attachment #3: xen-acpi-irq-model.patch --]
[-- Type: text/x-patch, Size: 3041 bytes --]

Subject: xen: add hypercall to get Xen's ACPI IRQ model

Rather than having Xen and dom0 independently reach the same conclusion
about the ACPI IRQ routing model (one hopes), add a hypercall so that dom0
can query Xen's state.  This allows dom0 to call the ACPI _PIC method
appropriately so that everyone (ACPI, Xen, dom0) all have consistent
views of what the interrupt routing model is.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

diff -r c484823b49a6 -r 257864bc4286 xen/arch/x86/physdev.c
--- a/xen/arch/x86/physdev.c	Tue Jul 14 14:39:09 2009 -0700
+++ b/xen/arch/x86/physdev.c	Fri Jul 24 12:18:41 2009 -0700
@@ -8,6 +8,7 @@
 #include <xen/event.h>
 #include <xen/guest_access.h>
 #include <xen/iocap.h>
+#include <xen/acpi.h>
 #include <asm/current.h>
 #include <asm/msi.h>
 #include <asm/hypercall.h>
@@ -418,6 +419,31 @@
         break;
     }
 
+    case PHYSDEVOP_acpi_irq_model:
+        if (!acpi_lapic || !acpi_ioapic)
+            ret = -ENODEV;
+        else {
+            uint32_t m;
+            struct physdev_acpi_irq_model model;
+
+            ret = 0;
+            switch ( acpi_irq_model ) {
+            case ACPI_IRQ_MODEL_PIC:     m = XEN_ACPI_IRQ_MODEL_PIC; break;
+            case ACPI_IRQ_MODEL_IOAPIC:  m = XEN_ACPI_IRQ_MODEL_IOAPIC; break;
+            case ACPI_IRQ_MODEL_IOSAPIC: m = XEN_ACPI_IRQ_MODEL_IOSAPIC; break;
+            default:
+                ret = -EINVAL;
+                break;
+            }
+
+            if (ret == 0) {
+                model.irq_model = m;
+                if ( copy_to_guest(arg, &model, sizeof(model)) != 0 )
+                    ret = -EFAULT;
+            }
+        }
+        break;
+
     case PHYSDEVOP_set_iopl: {
         struct physdev_set_iopl set_iopl;
         ret = -EFAULT;
diff -r c484823b49a6 -r 257864bc4286 xen/include/public/physdev.h
--- a/xen/include/public/physdev.h	Tue Jul 14 14:39:09 2009 -0700
+++ b/xen/include/public/physdev.h	Fri Jul 24 12:18:41 2009 -0700
@@ -236,6 +236,30 @@
 #define  XENROUTEGSI_masked     (1U<<_XENROUTEGSI_masked)
 
 /*
+ * Return Xen's ACPI interrupt model.  When Xen probes ACPI and the
+ * host's interrupt hardware, it determines whether we're using ACPI
+ * for interrupt routing at all, and if so, whether its in PIC or
+ * IOAPIC mode.  Xen then requires dom0 to match this, and call the
+ * appropriate corresponding AML code, which the host.
+ *
+ * Fails with ENODEV if Xen is not using ACPI for interrupts at all.
+ *
+ * @arg = pointer to physdev_acpi_irq_model
+ */
+#define PHYSDEVOP_acpi_irq_model        22
+struct physdev_acpi_irq_model {
+    /* OUT */
+    uint32_t irq_model;
+};
+typedef struct physdev_acpi_irq_model physdev_acpi_irq_model_t;
+DEFINE_XEN_GUEST_HANDLE(physdev_acpi_irq_model_t);
+
+#define XEN_ACPI_IRQ_MODEL_PIC             (0)
+#define XEN_ACPI_IRQ_MODEL_IOAPIC          (1)
+#define XEN_ACPI_IRQ_MODEL_IOSAPIC         (2)
+
+
+/*
  * Argument to physdev_op_compat() hypercall. Superceded by new physdev_op()
  * hypercall since 0x00030202.
  */

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

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

RE: RE: [PATCH] Don't free irqaction for com irq when release irq.

[Zhang, Xiantao]

Jeremy Fitzhardinge wrote:
> On 09/03/09 05:53, Zhang, Xiantao wrote:
>> I think Jan also answered your question.   Dom0 shouldn't touch
>> ioapic after initialization time any more.  That is to say, maybe we
>> can find a way to get rid of ioapic from dom0.  Actually I can't see
>> why dom0 cares so much about ioapic.   Jeremy, do you know the
>> reason ?  IMO, dom0 should only cares about GSI and pirq mapping,
>> but currently GSI is always equal to pirq in dom0, so no reason to
>> keep ioapic in dom0.      
>> 
> 
> Yes, I agree.  I have a prototype branch:
> git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git
> #rebase/dom0/new-interrupt-routing which replaces the whole interrupt
> handing subsystem to avoid any direct interactions with the IO APICs.

Okay, I will read your code and give my comments later. 

> I add a new hypercall to directly bind a gsi to a given pirq (which
> has some similarities to your patch).  I've attached it below.
> (new-interrupt-routing shouldn't need this patch to function,
> however.) 
> 
> Unfortunately I haven't had a chance to work on this lately, but when
> I last tried it, it hung shortly after initializing ACPI.  I didn't
> get much further than that.  I do think, however, that this is this
> right way to go for dom0, esp with regard to upstreaming.

Seems you didn't assign vectors for GSI >= 16 ?  Currenlty, hypervisor only allocate vectors for pin0 -> pin16, so irq_to_vector may not get the vectors for the required GSI. 

> (The second patch is to allow dom0 to get Xen's acpi interrupt model
> so they can always be consistent rather than independently arriving
> at the same result - not not.)

Maybe it is not a must to introduce new hypercall and just using  PHYSDEVOP_map_pirq like MSI side to bind pirq and GSI should  work for GSI irqs also. 
For the second patch, maybe we can ignore it since machines produced in recent 10 years should always use the model XEN_ACPI_IRQ_MODEL_IOAPIC. For old machines, I don't think Xen can run well on them. For the model XEN_ACPI_IRQ_MODEL_IOSAPIC, it is only available for Intel Itanium processors, so... :-)
Xiantao