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

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

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