Re: [PATCH] x86/io-apic: fix directed EOI when using AMd-Vi interrupt remapping

["Woodhouse, David" <dwmw@xxxxxxxxxxxx>]

On Sat, 2024-10-19 at 05:23 +0200, Marek Marczykowski-Górecki wrote:
> On Fri, Oct 18, 2024 at 10:08:13AM +0200, Roger Pau Monne wrote:
> > When using AMD-VI interrupt remapping the vector field in the IO-APIC RTE is
> > repurposed to contain part of the offset into the remapping table.  
> > Previous to
> > 2ca9fbd739b8 Xen had logic so that the offset into the interrupt remapping
> > table would match the vector.  Such logic was mandatory for end of 
> > interrupt to
> > work, since the vector field (even when not containing a vector) is used by 
> > the
> > IO-APIC to find for which pin the EOI must be performed.
> > 
> > Introduce a table to store the EOI handlers when using interrupt remapping, 
> > so
> > that the IO-APIC driver can translate pins into EOI handlers without having 
> > to
> > read the IO-APIC RTE entry.  Note that to simplify the logic such table is 
> > used
> > unconditionally when interrupt remapping is enabled, even if strictly it 
> > would
> > only be required for AMD-Vi.
> > 
> > Reported-by: Willi Junga <xenproject@xxxxxx>
> > Suggested-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
> > Fixes: 2ca9fbd739b8 ('AMD IOMMU: allocate IRTE entries instead of using a 
> > static mapping')
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> I can confirm it fixes touchpad issue on Framework 13 AMD,
> it works without ioapic_ack=new now, thanks!
> Tested-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>

Thanks for testing. But... why did this work with the auto-EOI? That
*should* have had exactly the same problem, surely? 

The problem fixed by this patch is that the directed EOI used the
actual vector# and *not* the bits that the I/O APIC *thinks* are the
vector#, which are actually the IRTE index#.

But if you let the CPU do its broadcast EOI then surely *that* is going
to use the actual vector# too, and have precisely the same problem?

If you use the code prior to this patch, *without* ioapic_ack=new (i.e.
the mode that was failing), what happens if you do this:

--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -595,7 +595,7 @@ void setup_local_APIC(void)
     /*
      * Enable directed EOI
      */
-    if ( directed_eoi_enabled )
+    if ( 0 && directed_eoi_enabled )
     {
         value |= APIC_SPIV_DIRECTED_EOI;
         apic_printk(APIC_VERBOSE, "Suppress EOI broadcast on CPU#%d\n",


I'm guessing that 'fixes' it too? In which case, it looks like AMD has
some undocumented hack in between its APIC and I/O APIC to let it
magically auto-EOI the correct pin somehow?

Adding Boris and x86@xxxxxxxxxx to Cc... do we know anything about such
an AMD hack?

Attachment: smime.p7s
Description: S/MIME cryptographic signature

Amazon Development Centre (London) Ltd.Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.