[xen master] x86/vlapic: Drop vlapic->esr_lock

[patchbot@xxxxxxx]

commit 1ecc4effa7c56cc0072437e69bc68fea844345fe
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Thu Nov 28 00:47:37 2024 +0000
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Fri Mar 7 14:34:08 2025 +0000

    x86/vlapic: Drop vlapic->esr_lock
    
    The exact behaviour of LVTERR interrupt generation is implementation
    specific.
    
     * Newer Intel CPUs generate an interrupt when pending_esr becomes
       nonzero.
    
     * Older Intel and all AMD CPUs generate an interrupt when any
       individual bit in pending_esr becomes nonzero.
    
    Neither vendor documents their behaviour very well.  Xen implements
    the per-bit behaviour and has done since support was added.
    
    Importantly, the per-bit behaviour can be expressed using the atomic
    operations available in the x86 architecture, whereas the
    former (interrupt only on pending_esr becoming nonzero) cannot.
    
    With vlapic->hw.pending_esr held outside of the main LAPIC register page,
    it's much easier to use atomic operations.
    
    Use xchg() in vlapic_reg_write(), and *set_bit() in vlapic_error().
    
    The only interesting change is that vlapic_error() now needs to take a
    single bit only, rather than a mask, but this fine for all current
    callers and forseable changes.
    
    No change from a guests perspective.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/hvm/vlapic.c             | 41 +++++++++++++----------------------
 xen/arch/x86/include/asm/hvm/vlapic.h |  1 -
 2 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index d7dd430bd1..065b2aab5b 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -102,14 +102,16 @@ static int vlapic_find_highest_irr(struct vlapic *vlapic)
     return vlapic_find_highest_vector(&vlapic->regs->data[APIC_IRR]);
 }
 
-static void vlapic_error(struct vlapic *vlapic, unsigned int errmask)
+static void vlapic_error(struct vlapic *vlapic, unsigned int err_bit)
 {
-    unsigned long flags;
-    uint32_t esr;
-
-    spin_lock_irqsave(&vlapic->esr_lock, flags);
-    esr = vlapic->hw.pending_esr;
-    if ( (esr & errmask) != errmask )
+    /*
+     * Whether LVTERR is delivered on a per-bit basis, or only on
+     * pending_esr becoming nonzero is implementation specific.
+     *
+     * Xen implements the per-bit behaviour as it can be expressed
+     * locklessly.
+     */
+    if ( !test_and_set_bit(err_bit, &vlapic->hw.pending_esr) )
     {
         uint32_t lvterr = vlapic_get_reg(vlapic, APIC_LVTERR);
         bool inj = false;
@@ -122,17 +124,14 @@ static void vlapic_error(struct vlapic *vlapic, unsigned 
int errmask)
              * if it will succeed, and folding in RECVILL otherwise.
              */
             if ( (lvterr & APIC_VECTOR_MASK) >= 16 )
-                 inj = true;
+                inj = true;
             else
-                 errmask |= APIC_ESR_RECVILL;
+                set_bit(ilog2(APIC_ESR_RECVILL), &vlapic->hw.pending_esr);
         }
 
-        vlapic->hw.pending_esr |= errmask;
-
         if ( inj )
             vlapic_set_irq(vlapic, lvterr & APIC_VECTOR_MASK, 0);
     }
-    spin_unlock_irqrestore(&vlapic->esr_lock, flags);
 }
 
 bool vlapic_test_irq(const struct vlapic *vlapic, uint8_t vec)
@@ -153,7 +152,7 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, 
uint8_t trig)
 
     if ( unlikely(vec < 16) )
     {
-        vlapic_error(vlapic, APIC_ESR_RECVILL);
+        vlapic_error(vlapic, ilog2(APIC_ESR_RECVILL));
         return;
     }
 
@@ -525,7 +524,7 @@ void vlapic_ipi(
             vlapic_domain(vlapic), vlapic, short_hand, dest, dest_mode);
 
         if ( unlikely((icr_low & APIC_VECTOR_MASK) < 16) )
-            vlapic_error(vlapic, APIC_ESR_SENDILL);
+            vlapic_error(vlapic, ilog2(APIC_ESR_SENDILL));
         else if ( target )
             vlapic_accept_irq(vlapic_vcpu(target), icr_low);
         break;
@@ -534,7 +533,7 @@ void vlapic_ipi(
     case APIC_DM_FIXED:
         if ( unlikely((icr_low & APIC_VECTOR_MASK) < 16) )
         {
-            vlapic_error(vlapic, APIC_ESR_SENDILL);
+            vlapic_error(vlapic, ilog2(APIC_ESR_SENDILL));
             break;
         }
         /* fall through */
@@ -803,17 +802,9 @@ void vlapic_reg_write(struct vcpu *v, unsigned int reg, 
uint32_t val)
         break;
 
     case APIC_ESR:
-    {
-        unsigned long flags;
-
-        spin_lock_irqsave(&vlapic->esr_lock, flags);
-        val = vlapic->hw.pending_esr;
-        vlapic->hw.pending_esr = 0;
-        spin_unlock_irqrestore(&vlapic->esr_lock, flags);
-
+        val = xchg(&vlapic->hw.pending_esr, 0);
         vlapic_set_reg(vlapic, APIC_ESR, val);
         break;
-    }
 
     case APIC_TASKPRI:
         vlapic_set_reg(vlapic, APIC_TASKPRI, val & 0xff);
@@ -1716,8 +1707,6 @@ int vlapic_init(struct vcpu *v)
 
     vlapic_reset(vlapic);
 
-    spin_lock_init(&vlapic->esr_lock);
-
     tasklet_init(&vlapic->init_sipi.tasklet, vlapic_init_sipi_action, v);
 
     if ( v->vcpu_id == 0 )
diff --git a/xen/arch/x86/include/asm/hvm/vlapic.h 
b/xen/arch/x86/include/asm/hvm/vlapic.h
index 2c4ff94ae7..c388551198 100644
--- a/xen/arch/x86/include/asm/hvm/vlapic.h
+++ b/xen/arch/x86/include/asm/hvm/vlapic.h
@@ -69,7 +69,6 @@ struct vlapic {
         bool                 hw, regs;
         uint32_t             id, ldr;
     }                        loaded;
-    spinlock_t               esr_lock;
     struct periodic_time     pt;
     s_time_t                 timer_last_update;
     struct page_info         *regs_page;
--
generated by git-patchbot for /home/xen/git/xen.git#master