WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] passthrough: don't use open coded IO-APIC accesses

To: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] passthrough: don't use open coded IO-APIC accesses
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Tue, 16 Aug 2011 12:57:27 +0100
Delivery-date: Tue, 16 Aug 2011 04:57:35 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
This makes the respective functions quite a bit more legible.

Since this requires fiddling with __ioapic_{read,write}_entry() anyway,
make them and their wrappers have their argument types match those of
__io_apic_{read,write}() (int -> unsigned int).

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>

--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -162,7 +162,8 @@ union entry_union {
     struct IO_APIC_route_entry entry;
 };
 
-static struct IO_APIC_route_entry __ioapic_read_entry(int apic, int pin, int 
raw)
+struct IO_APIC_route_entry __ioapic_read_entry(
+    unsigned int apic, unsigned int pin, bool_t raw)
 {
     unsigned int (*read)(unsigned int, unsigned int)
         = raw ? __io_apic_read : io_apic_read;
@@ -172,7 +173,8 @@ static struct IO_APIC_route_entry __ioap
     return eu.entry;
 }
 
-static struct IO_APIC_route_entry ioapic_read_entry(int apic, int pin, int raw)
+static struct IO_APIC_route_entry ioapic_read_entry(
+    unsigned int apic, unsigned int pin, bool_t raw)
 {
     struct IO_APIC_route_entry entry;
     unsigned long flags;
@@ -183,8 +185,9 @@ static struct IO_APIC_route_entry ioapic
     return entry;
 }
 
-static void
-__ioapic_write_entry(int apic, int pin, int raw, struct IO_APIC_route_entry e)
+void __ioapic_write_entry(
+    unsigned int apic, unsigned int pin, bool_t raw,
+    struct IO_APIC_route_entry e)
 {
     void (*write)(unsigned int, unsigned int, unsigned int)
         = raw ? __io_apic_write : io_apic_write;
@@ -195,7 +198,9 @@ __ioapic_write_entry(int apic, int pin, 
     (*write)(apic, 0x10 + 2*pin, eu.w1);
 }
 
-static void ioapic_write_entry(int apic, int pin, int raw, struct 
IO_APIC_route_entry e)
+static void ioapic_write_entry(
+    unsigned int apic, unsigned int pin, bool_t raw,
+    struct IO_APIC_route_entry e)
 {
     unsigned long flags;
     spin_lock_irqsave(&ioapic_lock, flags);
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -336,13 +336,6 @@ void amd_iommu_msi_msg_update_ire(
     update_intremap_entry_from_msi_msg(iommu, pdev, msi_desc, msg);
 }
 
-unsigned int amd_iommu_read_ioapic_from_ire(
-    unsigned int apic, unsigned int reg)
-{
-    *IO_APIC_BASE(apic) = reg;
-    return *(IO_APIC_BASE(apic)+4);
-}
-
 void amd_iommu_read_msi_from_ire(
     struct msi_desc *msi_desc, struct msi_msg *msg)
 {
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -438,6 +438,8 @@ static int amd_iommu_group_id(u16 seg, u
     return rt;
 }
 
+#include <asm/io_apic.h>
+
 const struct iommu_ops amd_iommu_ops = {
     .init = amd_iommu_domain_init,
     .dom0_init = amd_iommu_dom0_init,
@@ -451,7 +453,7 @@ const struct iommu_ops amd_iommu_ops = {
     .get_device_group_id = amd_iommu_group_id,
     .update_ire_from_apic = amd_iommu_ioapic_update_ire,
     .update_ire_from_msi = amd_iommu_msi_msg_update_ire,
-    .read_apic_from_ire = amd_iommu_read_ioapic_from_ire,
+    .read_apic_from_ire = __io_apic_read,
     .read_msi_from_ire = amd_iommu_read_msi_from_ire,
     .suspend = amd_iommu_suspend,
     .resume = amd_iommu_resume,
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -34,6 +34,22 @@
 #ifdef __ia64__
 #define nr_ioapics              iosapic_get_nr_iosapics()
 #define nr_ioapic_registers(i)  iosapic_get_nr_pins(i)
+#define __io_apic_read(apic, reg) \
+    (*IO_APIC_BASE(apic) = reg, *(IO_APIC_BASE(apic)+4))
+#define __io_apic_write(apic, reg, val) \
+    (*IO_APIC_BASE(apic) = reg, *(IO_APIC_BASE(apic)+4) = (val))
+#define __ioapic_read_entry(apic, pin, raw) ({ \
+    struct IO_xAPIC_route_entry _e_; \
+    ASSERT(raw); \
+    ((u32 *)&_e_)[0] = __io_apic_read(apic, 0x10 + 2 * (pin)); \
+    ((u32 *)&_e_)[1] = __io_apic_read(apic, 0x11 + 2 * (pin)); \
+    _e_; \
+})
+#define __ioapic_write_entry(apic, pin, raw, ent) ({ \
+    ASSERT(raw); \
+    __io_apic_write(apic, 0x10 + 2 * (pin), ((u32 *)&_e_)[0]); \
+    __io_apic_write(apic, 0x11 + 2 * (pin), ((u32 *)&_e_)[1]); \
+})
 #else
 #include <asm/apic.h>
 #include <asm/io_apic.h>
@@ -374,25 +390,12 @@ unsigned int io_apic_read_remap_rte(
     if ( !iommu || !ir_ctrl || ir_ctrl->iremap_maddr == 0 ||
         (ir_ctrl->iremap_num == 0) ||
         ( (index = apic_pin_2_ir_idx[apic][ioapic_pin]) < 0 ) )
-    {
-        *IO_APIC_BASE(apic) = reg;
-        return *(IO_APIC_BASE(apic)+4);
-    }
-
-    if ( rte_upper )
-        reg--;
+        return __io_apic_read(apic, reg);
 
-    /* read lower and upper 32-bits of rte entry */
-    *IO_APIC_BASE(apic) = reg;
-    *(((u32 *)&old_rte) + 0) = *(IO_APIC_BASE(apic)+4);
-    *IO_APIC_BASE(apic) = reg + 1;
-    *(((u32 *)&old_rte) + 1) = *(IO_APIC_BASE(apic)+4);
+    old_rte = __ioapic_read_entry(apic, ioapic_pin, TRUE);
 
     if ( remap_entry_to_ioapic_rte(iommu, index, &old_rte) )
-    {
-        *IO_APIC_BASE(apic) = rte_upper ? (reg + 1) : reg;
-        return *(IO_APIC_BASE(apic)+4);
-    }
+        return __io_apic_read(apic, reg);
 
     if ( rte_upper )
         return (*(((u32 *)&old_rte) + 1));
@@ -413,49 +416,31 @@ void io_apic_write_remap_rte(
 
     if ( !iommu || !ir_ctrl || ir_ctrl->iremap_maddr == 0 )
     {
-        *IO_APIC_BASE(apic) = reg;
-        *(IO_APIC_BASE(apic)+4) = value;
+        __io_apic_write(apic, reg, value);
         return;
     }
 
-    if ( rte_upper )
-        reg--;
-
-    /* read both lower and upper 32-bits of rte entry */
-    *IO_APIC_BASE(apic) = reg;
-    *(((u32 *)&old_rte) + 0) = *(IO_APIC_BASE(apic)+4);
-    *IO_APIC_BASE(apic) = reg + 1;
-    *(((u32 *)&old_rte) + 1) = *(IO_APIC_BASE(apic)+4);
+    old_rte = __ioapic_read_entry(apic, ioapic_pin, TRUE);
 
     remap_rte = (struct IO_APIC_route_remap_entry *) &old_rte;
 
     /* mask the interrupt while we change the intremap table */
     saved_mask = remap_rte->mask;
     remap_rte->mask = 1;
-    *IO_APIC_BASE(apic) = reg;
-    *(IO_APIC_BASE(apic)+4) = *(((int *)&old_rte)+0);
+    __io_apic_write(apic, reg & ~1, *(u32 *)&old_rte);
     remap_rte->mask = saved_mask;
 
     if ( ioapic_rte_to_remap_entry(iommu, apic, ioapic_pin,
                                    &old_rte, rte_upper, value) )
     {
-        *IO_APIC_BASE(apic) = rte_upper ? (reg + 1) : reg;
-        *(IO_APIC_BASE(apic)+4) = value;
+        __io_apic_write(apic, reg, value);
 
         /* Recover the original value of 'mask' bit */
         if ( rte_upper )
-        {
-            *IO_APIC_BASE(apic) = reg;
-            *(IO_APIC_BASE(apic)+4) = *(((u32 *)&old_rte)+0);
-        }
-        return;
+            __io_apic_write(apic, reg & ~1, *(u32 *)&old_rte);
     }
-
-    /* write new entry to ioapic */
-    *IO_APIC_BASE(apic) = reg + 1;
-    *(IO_APIC_BASE(apic)+4) = *(((u32 *)&old_rte)+1);
-    *IO_APIC_BASE(apic) = reg;
-    *(IO_APIC_BASE(apic)+4) = *(((u32 *)&old_rte)+0);
+    else
+        __ioapic_write_entry(apic, ioapic_pin, TRUE, old_rte);
 }
 
 #if defined(__i386__) || defined(__x86_64__)
--- a/xen/drivers/passthrough/vtd/utils.c
+++ b/xen/drivers/passthrough/vtd/utils.c
@@ -261,9 +261,8 @@ static void dump_iommu_info(unsigned cha
     /* Dump the I/O xAPIC redirection table(s). */
     if ( iommu_enabled )
     {
-        int apic, reg;
+        int apic;
         union IO_APIC_reg_01 reg_01;
-        struct IO_APIC_route_entry rte = { 0 };
         struct IO_APIC_route_remap_entry *remap;
         struct ir_ctrl *ir_ctrl;
 
@@ -277,19 +276,14 @@ static void dump_iommu_info(unsigned cha
 
             printk( "\nRedirection table of IOAPIC %x:\n", apic);
 
-            reg = 1; /* IO xAPIC Version Register. */
-            *IO_APIC_BASE(apic) = reg;
-            reg_01.raw = *(IO_APIC_BASE(apic)+4);
+            /* IO xAPIC Version Register. */
+            reg_01.raw = __io_apic_read(apic, 1);
 
             printk("  #entry IDX FMT MASK TRIG IRR POL STAT DELI  VECTOR\n");
             for ( i = 0; i <= reg_01.bits.entries; i++ )
             {
-                reg = 0x10 + i*2;
-                *IO_APIC_BASE(apic) = reg;
-                *(((u32 *)&rte) + 0) = *(IO_APIC_BASE(apic)+4);
-
-                *IO_APIC_BASE(apic) = reg + 1;
-                *(((u32 *)&rte) + 1) = *(IO_APIC_BASE(apic)+4);
+                struct IO_APIC_route_entry rte =
+                    __ioapic_read_entry(apic, i, TRUE);
 
                 remap = (struct IO_APIC_route_remap_entry *) &rte;
                 if ( !remap->format )
--- a/xen/drivers/passthrough/vtd/vtd.h
+++ b/xen/drivers/passthrough/vtd/vtd.h
@@ -27,6 +27,9 @@
 #define UNMAP_ME_PHANTOM_FUNC    0
 
 /* Accomodate both IOAPIC and IOSAPIC. */
+#ifndef __ia64__
+#define IO_xAPIC_route_entry IO_APIC_route_entry
+#else
 struct IO_xAPIC_route_entry {
     __u32   vector      :  8,
         delivery_mode   :  3,   /* 000: FIXED
@@ -53,15 +56,14 @@ struct IO_xAPIC_route_entry {
             logical_dest    :  8;
         } logical;
 
-#ifdef __ia64__
         struct { __u32
             __reserved_1    : 16,
             dest_id         : 16;
         };
-#endif
     } dest;
 
 } __attribute__ ((packed));
+#endif
 
 struct IO_APIC_route_remap_entry {
     union {
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -93,8 +93,6 @@ void amd_iommu_msi_msg_update_ire(
     struct msi_desc *msi_desc, struct msi_msg *msg);
 void amd_iommu_read_msi_from_ire(
     struct msi_desc *msi_desc, struct msi_msg *msg);
-unsigned int amd_iommu_read_ioapic_from_ire(
-    unsigned int apic, unsigned int reg);
 
 extern int ioapic_bdf[MAX_IO_APICS];
 
--- a/xen/include/asm-x86/io_apic.h
+++ b/xen/include/asm-x86/io_apic.h
@@ -200,6 +200,12 @@ extern void ioapic_resume(void);
 
 extern void dump_ioapic_irq_info(void);
 
+extern struct IO_APIC_route_entry __ioapic_read_entry(
+    unsigned int apic, unsigned int pin, bool_t raw);
+void __ioapic_write_entry(
+    unsigned int apic, unsigned int pin, bool_t raw,
+    struct IO_APIC_route_entry);
+
 extern struct IO_APIC_route_entry **alloc_ioapic_entries(void);
 extern void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries);
 extern int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);


Attachment: pt-ioapic-access.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] passthrough: don't use open coded IO-APIC accesses, Jan Beulich <=