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

[Jeremy Fitzhardinge <jeremy@goop.org>]

[-- 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