AMD/IOMMU: Delete iommu_{get,set,clear}_bit() helpers

These are obfuscations around simple bit operations, and the compiler really
can do a better job when it can see them normally:

  add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-189 (-189)
  Function                                     old     new   delta
  guest_iommu_add_ppr_log                      266     251     -15
  guest_iommu_add_event_log                    266     251     -15
  iommu_reset_log                              274     242     -32
  guest_iommu_process_command                 1602    1544     -58
  guest_iommu_mmio_write                      1123    1054     -69
  Total: Before=3019344, After=3019155, chg -0.01%

Drop all status register MASK/SHIFT constants, and enumerate the bits
normally.  Rename EVENT_OVERFLOW to EVENT_LOG_OVERFLOW for consistency.  (The
field name in the spec is inconsistent, despite the description referring to
an overflow of the event log.)

The only semantic change is in iommu_reset_log() where 'run_bit' changes from
being a bit position to being a single-bit mask.  Update some local variable
types to be more suitable.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/drivers/passthrough/amd/iommu-defs.h b/xen/drivers/passthrough/amd/iommu-defs.h
index f8b62cb033fee89cda12fcfad1660cc75990266a..963009de6a8aa6b167eae9ab8d31f71936f47a42 100644 (file)
--- a/xen/drivers/passthrough/amd/iommu-defs.h
+++ b/xen/drivers/passthrough/amd/iommu-defs.h
@@ -437,28 +437,18 @@ union amd_iommu_x2apic_control {
 
 /* Status Register*/
 #define IOMMU_STATUS_MMIO_OFFSET               0x2020
-#define IOMMU_STATUS_EVENT_OVERFLOW_MASK       0x00000001
-#define IOMMU_STATUS_EVENT_OVERFLOW_SHIFT      0
-#define IOMMU_STATUS_EVENT_LOG_INT_MASK                0x00000002
-#define IOMMU_STATUS_EVENT_LOG_INT_SHIFT       1
-#define IOMMU_STATUS_COMP_WAIT_INT_MASK                0x00000004
-#define IOMMU_STATUS_COMP_WAIT_INT_SHIFT       2
-#define IOMMU_STATUS_EVENT_LOG_RUN_MASK                0x00000008
-#define IOMMU_STATUS_EVENT_LOG_RUN_SHIFT       3
-#define IOMMU_STATUS_CMD_BUFFER_RUN_MASK       0x00000010
-#define IOMMU_STATUS_CMD_BUFFER_RUN_SHIFT      4
-#define IOMMU_STATUS_PPR_LOG_OVERFLOW_MASK      0x00000020
-#define IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT     5
-#define IOMMU_STATUS_PPR_LOG_INT_MASK           0x00000040
-#define IOMMU_STATUS_PPR_LOG_INT_SHIFT          6
-#define IOMMU_STATUS_PPR_LOG_RUN_MASK           0x00000080
-#define IOMMU_STATUS_PPR_LOG_RUN_SHIFT          7
-#define IOMMU_STATUS_GAPIC_LOG_OVERFLOW_MASK    0x00000100
-#define IOMMU_STATUS_GAPIC_LOG_OVERFLOW_SHIFT   8
-#define IOMMU_STATUS_GAPIC_LOG_INT_MASK         0x00000200
-#define IOMMU_STATUS_GAPIC_LOG_INT_SHIFT        9
-#define IOMMU_STATUS_GAPIC_LOG_RUN_MASK         0x00000400
-#define IOMMU_STATUS_GAPIC_LOG_RUN_SHIFT        10
+
+#define IOMMU_STATUS_EVENT_LOG_OVERFLOW   0x00000001
+#define IOMMU_STATUS_EVENT_LOG_INT        0x00000002
+#define IOMMU_STATUS_COMP_WAIT_INT        0x00000004
+#define IOMMU_STATUS_EVENT_LOG_RUN        0x00000008
+#define IOMMU_STATUS_CMD_BUFFER_RUN       0x00000010
+#define IOMMU_STATUS_PPR_LOG_OVERFLOW     0x00000020
+#define IOMMU_STATUS_PPR_LOG_INT          0x00000040
+#define IOMMU_STATUS_PPR_LOG_RUN          0x00000080
+#define IOMMU_STATUS_GAPIC_LOG_OVERFLOW   0x00000100
+#define IOMMU_STATUS_GAPIC_LOG_INT        0x00000200
+#define IOMMU_STATUS_GAPIC_LOG_RUN        0x00000400
 
 /* I/O Page Table */
 #define IOMMU_PAGE_TABLE_ENTRY_SIZE    8

diff --git a/xen/drivers/passthrough/amd/iommu.h b/xen/drivers/passthrough/amd/iommu.h
index f590de8cbf799ce0f38010cb7cd5e8703c01ed0e..81b6812d3a7711aee0201636c0572e7c011f56f8 100644 (file)
--- a/xen/drivers/passthrough/amd/iommu.h
+++ b/xen/drivers/passthrough/amd/iommu.h
@@ -374,21 +374,6 @@ static inline void __free_amd_iommu_tables(void *table, int order)
     free_xenheap_pages(table, order);
 }
 
-static inline void iommu_set_bit(uint32_t *reg, uint32_t bit)
-{
-    set_field_in_reg_u32(IOMMU_CONTROL_ENABLED, *reg, 1U << bit, bit, reg);
-}
-
-static inline void iommu_clear_bit(uint32_t *reg, uint32_t bit)
-{
-    set_field_in_reg_u32(IOMMU_CONTROL_DISABLED, *reg, 1U << bit, bit, reg);
-}
-
-static inline uint32_t iommu_get_bit(uint32_t reg, uint32_t bit)
-{
-    return get_field_from_reg_u32(reg, 1U << bit, bit);
-}
-
 static inline int iommu_has_cap(struct amd_iommu *iommu, uint32_t bit)
 {
     return !!(iommu->cap.header & (1u << bit));

diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c b/xen/drivers/passthrough/amd/iommu_cmd.c
index 92eaab407bcb65bb8a14943a7a57cb32369a2d47..1eae339692894f56b6ac5db2aff5c140b5f1cbfe 100644 (file)
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
@@ -64,11 +64,11 @@ int send_iommu_command(struct amd_iommu *iommu, u32 cmd[])
 
 static void flush_command_buffer(struct amd_iommu *iommu)
 {
-    u32 cmd[4], status;
-    int loop_count, comp_wait;
+    unsigned int cmd[4], status, loop_count;
+    bool comp_wait;
 
     /* RW1C 'ComWaitInt' in status register */
-    writel(IOMMU_STATUS_COMP_WAIT_INT_MASK,
+    writel(IOMMU_STATUS_COMP_WAIT_INT,
            iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
 
     /* send an empty COMPLETION_WAIT command to flush command buffer */
@@ -85,16 +85,14 @@ static void flush_command_buffer(struct amd_iommu *iommu)
     loop_count = 1000;
     do {
         status = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
-        comp_wait = get_field_from_reg_u32(status,
-                                           IOMMU_STATUS_COMP_WAIT_INT_MASK,
-                                           IOMMU_STATUS_COMP_WAIT_INT_SHIFT);
+        comp_wait = status & IOMMU_STATUS_COMP_WAIT_INT;
         --loop_count;
     } while ( !comp_wait && loop_count );
 
     if ( comp_wait )
     {
         /* RW1C 'ComWaitInt' in status register */
-        writel(IOMMU_STATUS_COMP_WAIT_INT_MASK,
+        writel(IOMMU_STATUS_COMP_WAIT_INT,
                iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
         return;
     }

diff --git a/xen/drivers/passthrough/amd/iommu_guest.c b/xen/drivers/passthrough/amd/iommu_guest.c
index aaf12fe1cbad219f947213f4376f7951400ddd5a..d05901d3489f6ade04d7cec5b2c06bfed4b3ee74 100644 (file)
--- a/xen/drivers/passthrough/amd/iommu_guest.c
+++ b/xen/drivers/passthrough/amd/iommu_guest.c
@@ -30,12 +30,6 @@
 #define GUEST_ADDRESS_SIZE_6_LEVEL              0x2
 #define HOST_ADDRESS_SIZE_6_LEVEL               0x2
 
-#define guest_iommu_set_status(iommu, bit) \
-        iommu_set_bit(&((iommu)->reg_status.lo), bit)
-
-#define guest_iommu_clear_status(iommu, bit) \
-        iommu_clear_bit(&((iommu)->reg_status.lo), bit)
-
 #define reg_to_u64(reg) (((uint64_t)reg.hi << 32) | reg.lo )
 #define u64_to_reg(reg, val) \
     do \
@@ -183,7 +177,7 @@ void guest_iommu_add_ppr_log(struct domain *d, u32 entry[])
     if ( ++tail >= iommu->ppr_log.entries )
     {
         tail = 0;
-        guest_iommu_set_status(iommu, IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT);
+        iommu->reg_status.lo |= IOMMU_STATUS_PPR_LOG_OVERFLOW;
     }
     iommu_set_rb_pointer(&iommu->ppr_log.reg_tail.lo, tail);
     unmap_domain_page(log_base);
@@ -231,7 +225,7 @@ void guest_iommu_add_event_log(struct domain *d, u32 entry[])
     if ( ++tail >= iommu->event_log.entries )
     {
         tail = 0;
-        guest_iommu_set_status(iommu, IOMMU_STATUS_EVENT_OVERFLOW_SHIFT);
+        iommu->reg_status.lo |= IOMMU_STATUS_EVENT_LOG_OVERFLOW;
     }
 
     iommu_set_rb_pointer(&iommu->event_log.reg_tail.lo, tail);
@@ -322,11 +316,11 @@ static int do_completion_wait(struct domain *d, cmd_entry_t *cmd)
 
     iommu = domain_iommu(d);
 
-    i = iommu_get_bit(cmd->data[0], IOMMU_COMP_WAIT_I_FLAG_SHIFT);
-    s = iommu_get_bit(cmd->data[0], IOMMU_COMP_WAIT_S_FLAG_SHIFT);
+    i = cmd->data[0] & IOMMU_COMP_WAIT_I_FLAG_MASK;
+    s = cmd->data[0] & IOMMU_COMP_WAIT_S_FLAG_MASK;
 
     if ( i )
-        guest_iommu_set_status(iommu, IOMMU_STATUS_COMP_WAIT_INT_SHIFT);
+        iommu->reg_status.lo |= IOMMU_STATUS_COMP_WAIT_INT;
 
     if ( s )
     {
@@ -352,8 +346,7 @@ static int do_completion_wait(struct domain *d, cmd_entry_t *cmd)
         unmap_domain_page(vaddr);
     }
 
-    com_wait_int = iommu_get_bit(iommu->reg_status.lo,
-                                 IOMMU_STATUS_COMP_WAIT_INT_SHIFT);
+    com_wait_int = iommu->reg_status.lo & IOMMU_STATUS_COMP_WAIT_INT;
 
     if ( iommu->reg_ctrl.com_wait_int_en && com_wait_int )
         guest_iommu_deliver_msi(d);
@@ -539,16 +532,16 @@ static int guest_iommu_write_ctrl(struct guest_iommu *iommu, uint64_t val)
     {
         guest_iommu_enable_ring_buffer(iommu, &iommu->event_log,
                                        sizeof(event_entry_t));
-        guest_iommu_set_status(iommu, IOMMU_STATUS_EVENT_LOG_RUN_SHIFT);
-        guest_iommu_clear_status(iommu, IOMMU_STATUS_EVENT_OVERFLOW_SHIFT);
+        iommu->reg_status.lo |=  IOMMU_STATUS_EVENT_LOG_RUN;
+        iommu->reg_status.lo &= ~IOMMU_STATUS_EVENT_LOG_OVERFLOW;
     }
 
     if ( newctrl.iommu_en && newctrl.ppr_en && newctrl.ppr_log_en )
     {
         guest_iommu_enable_ring_buffer(iommu, &iommu->ppr_log,
                                        sizeof(ppr_entry_t));
-        guest_iommu_set_status(iommu, IOMMU_STATUS_PPR_LOG_RUN_SHIFT);
-        guest_iommu_clear_status(iommu, IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT);
+        iommu->reg_status.lo |=  IOMMU_STATUS_PPR_LOG_RUN;
+        iommu->reg_status.lo &= ~IOMMU_STATUS_PPR_LOG_OVERFLOW;
     }
 
     if ( newctrl.iommu_en && iommu->reg_ctrl.cmd_buf_en &&
@@ -559,7 +552,7 @@ static int guest_iommu_write_ctrl(struct guest_iommu *iommu, uint64_t val)
     }
 
     if ( iommu->reg_ctrl.event_log_en && !newctrl.event_log_en )
-        guest_iommu_clear_status(iommu, IOMMU_STATUS_EVENT_LOG_RUN_SHIFT);
+        iommu->reg_status.lo &= ~IOMMU_STATUS_EVENT_LOG_RUN;
 
     if ( iommu->reg_ctrl.iommu_en && !newctrl.iommu_en )
         guest_iommu_disable(iommu);
@@ -698,13 +691,13 @@ static void guest_iommu_mmio_write64(struct guest_iommu *iommu,
         u64_to_reg(&iommu->ppr_log.reg_tail, val);
         break;
     case IOMMU_STATUS_MMIO_OFFSET:
-        val &= IOMMU_STATUS_EVENT_OVERFLOW_MASK |
-               IOMMU_STATUS_EVENT_LOG_INT_MASK |
-               IOMMU_STATUS_COMP_WAIT_INT_MASK |
-               IOMMU_STATUS_PPR_LOG_OVERFLOW_MASK |
-               IOMMU_STATUS_PPR_LOG_INT_MASK |
-               IOMMU_STATUS_GAPIC_LOG_OVERFLOW_MASK |
-               IOMMU_STATUS_GAPIC_LOG_INT_MASK;
+        val &= IOMMU_STATUS_EVENT_LOG_OVERFLOW |
+               IOMMU_STATUS_EVENT_LOG_INT |
+               IOMMU_STATUS_COMP_WAIT_INT |
+               IOMMU_STATUS_PPR_LOG_OVERFLOW |
+               IOMMU_STATUS_PPR_LOG_INT |
+               IOMMU_STATUS_GAPIC_LOG_OVERFLOW |
+               IOMMU_STATUS_GAPIC_LOG_INT;
         u64_to_reg(&iommu->reg_status, reg_to_u64(iommu->reg_status) & ~val);
         break;
 

diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index 0ffc83a843a72138224ade3ca084ac1a37345970..5544dd95056cd326864b1d2117467a6ab7068332 100644 (file)
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -344,20 +344,18 @@ static void iommu_reset_log(struct amd_iommu *iommu,
                             struct ring_buffer *log,
                             void (*ctrl_func)(struct amd_iommu *iommu, bool))
 {
-    u32 entry;
-    int log_run, run_bit;
-    int loop_count = 1000;
+    unsigned int entry, run_bit, loop_count = 1000;
+    bool log_run;
 
     BUG_ON(!iommu || ((log != &iommu->event_log) && (log != &iommu->ppr_log)));
 
     run_bit = ( log == &iommu->event_log ) ?
-        IOMMU_STATUS_EVENT_LOG_RUN_SHIFT :
-        IOMMU_STATUS_PPR_LOG_RUN_SHIFT;
+        IOMMU_STATUS_EVENT_LOG_RUN : IOMMU_STATUS_PPR_LOG_RUN;
 
     /* wait until EventLogRun bit = 0 */
     do {
         entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
-        log_run = iommu_get_bit(entry, run_bit);
+        log_run = entry & run_bit;
         loop_count--;
     } while ( log_run && loop_count );
 
@@ -371,8 +369,8 @@ static void iommu_reset_log(struct amd_iommu *iommu,
     ctrl_func(iommu, IOMMU_CONTROL_DISABLED);
 
     /* RW1C overflow bit */
-    writel(log == &iommu->event_log ? IOMMU_STATUS_EVENT_OVERFLOW_MASK
-                                    : IOMMU_STATUS_PPR_LOG_OVERFLOW_MASK,
+    writel(log == &iommu->event_log ? IOMMU_STATUS_EVENT_LOG_OVERFLOW
+                                    : IOMMU_STATUS_PPR_LOG_OVERFLOW,
            iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
 
     /*reset event log base address */
@@ -589,7 +587,7 @@ static void iommu_check_event_log(struct amd_iommu *iommu)
     unsigned long flags;
 
     /* RW1C interrupt status bit */
-    writel(IOMMU_STATUS_EVENT_LOG_INT_MASK,
+    writel(IOMMU_STATUS_EVENT_LOG_INT,
            iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
 
     iommu_read_log(iommu, &iommu->event_log,
@@ -599,7 +597,7 @@ static void iommu_check_event_log(struct amd_iommu *iommu)
     
     /* Check event overflow. */
     entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
-    if ( iommu_get_bit(entry, IOMMU_STATUS_EVENT_OVERFLOW_SHIFT) )
+    if ( entry & IOMMU_STATUS_EVENT_LOG_OVERFLOW )
         iommu_reset_log(iommu, &iommu->event_log, set_iommu_event_log_control);
     else
     {
@@ -621,7 +619,7 @@ static void iommu_check_event_log(struct amd_iommu *iommu)
      * Re-check to make sure the bit has been cleared.
      */
     entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
-    if ( entry & IOMMU_STATUS_EVENT_LOG_INT_MASK )
+    if ( entry & IOMMU_STATUS_EVENT_LOG_INT )
         tasklet_schedule(&amd_iommu_irq_tasklet);
 
     spin_unlock_irqrestore(&iommu->lock, flags);
@@ -678,7 +676,7 @@ static void iommu_check_ppr_log(struct amd_iommu *iommu)
     unsigned long flags;
 
     /* RW1C interrupt status bit */
-    writel(IOMMU_STATUS_PPR_LOG_INT_MASK,
+    writel(IOMMU_STATUS_PPR_LOG_INT,
            iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
 
     iommu_read_log(iommu, &iommu->ppr_log,
@@ -688,7 +686,7 @@ static void iommu_check_ppr_log(struct amd_iommu *iommu)
 
     /* Check event overflow. */
     entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
-    if ( iommu_get_bit(entry, IOMMU_STATUS_PPR_LOG_OVERFLOW_SHIFT) )
+    if ( entry & IOMMU_STATUS_PPR_LOG_OVERFLOW )
         iommu_reset_log(iommu, &iommu->ppr_log, set_iommu_ppr_log_control);
     else
     {
@@ -710,7 +708,7 @@ static void iommu_check_ppr_log(struct amd_iommu *iommu)
      * Re-check to make sure the bit has been cleared.
      */
     entry = readl(iommu->mmio_base + IOMMU_STATUS_MMIO_OFFSET);
-    if ( entry & IOMMU_STATUS_PPR_LOG_INT_MASK )
+    if ( entry & IOMMU_STATUS_PPR_LOG_INT )
         tasklet_schedule(&amd_iommu_irq_tasklet);
 
     spin_unlock_irqrestore(&iommu->lock, flags);