[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] x86/IO-APIC: Improve APIC_TMR accesses



XenServer's instance of coverity complains of OVERFLOW_BEFORE_WIDEN in
mask_and_ack_level_ioapic_irq(), which is ultimately because of v being
unsigned long, and (1U << ...) being 32 bits.

Introduce a apic_tmr_read() helper like we already have for ISR and IRR, and
use it to remove the opencoded access logic.  Introduce an is_level boolean to
improve the legibility of the surrounding logic.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/arch/x86/include/asm/apic.h |  5 +++++
 xen/arch/x86/io_apic.c          | 15 +++++++--------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/include/asm/apic.h b/xen/arch/x86/include/asm/apic.h
index 7bd66dc6e151..a254e49dd13b 100644
--- a/xen/arch/x86/include/asm/apic.h
+++ b/xen/arch/x86/include/asm/apic.h
@@ -132,6 +132,11 @@ static inline bool apic_isr_read(uint8_t vector)
             (vector & 0x1f)) & 1;
 }
 
+static inline bool apic_tmr_read(unsigned int vector)
+{
+    return apic_read(APIC_TMR + (vector / 32 * 0x10)) & (1U << (vector % 32));
+}
+
 static inline bool apic_irr_read(unsigned int vector)
 {
     return apic_read(APIC_IRR + (vector / 32 * 0x10)) & (1U << (vector % 32));
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index ef076bfaf3f5..0fc1aa05fe3e 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -1652,8 +1652,7 @@ static bool io_apic_level_ack_pending(unsigned int irq)
 
 static void cf_check mask_and_ack_level_ioapic_irq(struct irq_desc *desc)
 {
-    unsigned long v;
-    int i;
+    bool is_level;
 
     irq_complete_move(desc);
 
@@ -1679,9 +1678,8 @@ static void cf_check mask_and_ack_level_ioapic_irq(struct 
irq_desc *desc)
  * operation to prevent an edge-triggered interrupt escaping meanwhile.
  * The idea is from Manfred Spraul.  --macro
  */
-    i = desc->arch.vector;
 
-    v = apic_read(APIC_TMR + ((i & ~0x1f) >> 1));
+    is_level = apic_tmr_read(desc->arch.vector);
 
     ack_APIC_irq();
     
@@ -1692,7 +1690,7 @@ static void cf_check mask_and_ack_level_ioapic_irq(struct 
irq_desc *desc)
        !io_apic_level_ack_pending(desc->irq))
         move_masked_irq(desc);
 
-    if ( !(v & (1U << (i & 0x1f))) )
+    if ( !is_level )
     {
         spin_lock(&ioapic_lock);
         __edge_IO_APIC_irq(desc->irq);
@@ -1743,13 +1741,14 @@ static void cf_check end_level_ioapic_irq_new(struct 
irq_desc *desc, u8 vector)
  * operation to prevent an edge-triggered interrupt escaping meanwhile.
  * The idea is from Manfred Spraul.  --macro
  */
-    unsigned int v, i = desc->arch.vector;
+    unsigned int i = desc->arch.vector;
+    bool is_level;
 
     /* Manually EOI the old vector if we are moving to the new */
     if ( vector && i != vector )
         eoi_IO_APIC_irq(desc);
 
-    v = apic_read(APIC_TMR + ((i & ~0x1f) >> 1));
+    is_level = apic_tmr_read(i);
 
     end_nonmaskable_irq(desc, vector);
 
@@ -1757,7 +1756,7 @@ static void cf_check end_level_ioapic_irq_new(struct 
irq_desc *desc, u8 vector)
          !io_apic_level_ack_pending(desc->irq) )
         move_native_irq(desc);
 
-    if ( !(v & (1U << (i & 0x1f))) )
+    if ( !is_level )
     {
         spin_lock(&ioapic_lock);
         __mask_IO_APIC_irq(desc->irq);
-- 
2.39.2




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.