* Re: [PATCH] x86/vlapic: Don't change ICR on x2APIC SELF_IPI write
2026-10-19 14:26 [PATCH] x86/vlapic: Don't change ICR on x2APIC SELF_IPI write Teddy Astie
@ 2026-10-19 21:09 ` Andrew Cooper
2026-10-19 23:29 ` Andrew Cooper
2026-10-23 20:45 ` Andrew Cooper
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2026-10-19 21:09 UTC (permalink / raw)
To: Teddy Astie, xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné
[-- Attachment #1: Type: text/plain, Size: 1579 bytes --]
On 19/10/2026 3:26 pm, Teddy Astie wrote:
> The Intel SDM (Vol 3A-13.12.11, June 2026) says
>
>> The semantics of sending a self-IPI via the SELF IPI register
>> are identical to sending a self targeted edge triggered fixed
>> interrupt with the specified vector.
> However, this is not expected to alter the ICR register.
>
> Fix the Xen behavior by sending the Self-IPI directly instead of
> emulating it by modifying ICR register.
>
> Fixes: 21c4d089ba2a ("x86 hvm: x2APIC emulation")
> Assisted-by: Claude Fable 5.1
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
What matters is the behaviour on real hardware.
If we're taking a strict interpretation, then "identical" means that ICR
does get mutated.
GNR:
(XEN) '1' pressed -> Extreme debugging in progress...
(XEN) *** Testing self-IPI
(XEN) Before: ICR 00000000000010ff, IRR 00000000
(XEN) After: ICR 00000000000400fc, IRR 90000000
(XEN) *** SPIV
Turin:
(XEN) '1' pressed -> Extreme debugging in progress...
(XEN) *** Testing self-IPI
(XEN) Before: ICR 00000000000000ff, IRR 80000000
(XEN) After: ICR 00000000000000ff, IRR 90000000
(XEN) *** SPIV
So, AMD doesn't mutate ICR. Intel does.
Weirdly, Intel reads back ICR_BUSY which shouldn't exist in x2APIC,
while AMD fails to read back the shorthand.
See the attached patch. The test sends SPIV to CPU0 using ICR, then a
self EVENT_CHECK sampling ICR and IRR before and after. The SPIV
printk() only gets serviced later, which is why it's the last of the
printk()s
~Andrew
[-- Attachment #2: 0001-dbg-self-ipi.patch --]
[-- Type: text/x-patch, Size: 3218 bytes --]
From d46eee03ea23327b0a480c03fc227e8eeb6da96a Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Sat, 28 Mar 2015 18:06:47 +0000
Subject: dbg self-ipi
diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 176f1498e380..cc0ac160ea38 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -1265,23 +1265,10 @@ static void cf_check spurious_interrupt(void)
* Vectored interrupts are ACKed; spurious interrupts are not.
*/
if (apic_isr_read(SPURIOUS_APIC_VECTOR)) {
- bool is_spurious;
-
ack_APIC_irq();
- is_spurious = !nmi_check_continuation();
- if (this_cpu(state_dump_pending)) {
- this_cpu(state_dump_pending) = false;
- dump_execstate(get_irq_regs());
- is_spurious = false;
- }
-
- if ( !is_spurious )
- return;
}
- /* see sw-dev-man vol 3, chapter 7.4.13.5 */
- printk(KERN_INFO "spurious APIC interrupt on CPU#%d, should "
- "never happen.\n", smp_processor_id());
+ printk("*** SPIV\n");
}
/*
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index ac01ba08723d..b35d3ff28857 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2687,3 +2687,76 @@ void asmlinkage entry_from_xen(struct cpu_user_regs *regs)
* indent-tabs-mode: nil
* End:
*/
+
+#include <xen/keyhandler.h>
+
+static void cf_check check_self_ipi(void *_)
+{
+ unsigned long flags;
+ uint64_t icr_before, icr_after;
+ uint32_t irr_before, irr_after;
+
+ local_irq_save(flags);
+
+ asm ("mfence;lfence");
+
+ /* Force use of SPURIOUS_APIC_VECTOR, 0xff */
+ apic_wrmsr(APIC_ICR,
+ APIC_DM_FIXED | APIC_DEST_PHYSICAL | SPURIOUS_APIC_VECTOR);
+
+ asm ("mfence;lfence");
+
+ icr_before = apic_rdmsr(APIC_ICR);
+ irr_before = apic_rdmsr(APIC_IRR + (0xf0/32 * 0x10));
+
+ asm ("mfence;lfence");
+
+ /* Send ourselves EVENT_CHECK_VECTOR, 0xfc */
+ apic_wrmsr(APIC_SELF_IPI, EVENT_CHECK_VECTOR);
+
+ asm ("mfence;lfence");
+
+ icr_after = apic_rdmsr(APIC_ICR);
+ irr_after = apic_rdmsr(APIC_IRR + (0xf0/32 * 0x10));
+
+ asm ("mfence;lfence");
+
+ local_irq_restore(flags);
+
+ printk(" Before: ICR %016"PRIx64", IRR %08x\n", icr_before, irr_before);
+ printk(" After: ICR %016"PRIx64", IRR %08x\n", icr_after, irr_after);
+}
+
+static void cf_check do_extreme_debug(unsigned char key, bool unused)
+{
+ printk("'%c' pressed -> Extreme debugging in progress...\n", key);
+
+ switch ( key )
+ {
+ case '1':
+ if ( !x2apic_enabled )
+ {
+ printk("*** No x2APIC, skipping\n");
+ break;
+ }
+
+ printk("*** Testing self-IPI\n");
+ on_selected_cpus(cpumask_of(0), check_self_ipi, NULL, 1);
+ break;
+
+ case '2':
+ break;
+
+ case '3':
+ break;
+ }
+}
+
+static int __init cf_check extreme_debug_keyhandler_init(void)
+{
+ register_irq_keyhandler('1', &do_extreme_debug, "dbg 1", 0);
+ register_irq_keyhandler('2', &do_extreme_debug, "dbg 2", 0);
+ register_irq_keyhandler('3', &do_extreme_debug, "dbg 3", 0);
+ return 0;
+}
+__initcall(extreme_debug_keyhandler_init);
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] x86/vlapic: Don't change ICR on x2APIC SELF_IPI write
2026-10-19 14:26 [PATCH] x86/vlapic: Don't change ICR on x2APIC SELF_IPI write Teddy Astie
2026-10-19 21:09 ` Andrew Cooper
@ 2026-10-23 20:45 ` Andrew Cooper
1 sibling, 0 replies; 5+ messages in thread
From: Andrew Cooper @ 2026-10-23 20:45 UTC (permalink / raw)
To: Teddy Astie, xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné
On 19/10/2026 3:26 pm, Teddy Astie wrote:
> The Intel SDM (Vol 3A-13.12.11, June 2026) says
>
>> The semantics of sending a self-IPI via the SELF IPI register
>> are identical to sending a self targeted edge triggered fixed
>> interrupt with the specified vector.
> However, this is not expected to alter the ICR register.
Having spoken to Intel, all Core/P-core's implement SELF_IPI using ICR,
so do mutate the value.
More recent Atom/E-core designs implement SELF_IPI differently, and do
not mutate ICR. From the testing I performed in Gitlab-CI, "More
recent" is at least AlderLake and later, so Gracemont cores.
Either way, this is conformation that both behaviours are legitimate,
and it is certainly not a "bugfix" to change the behaviour.
There might be enough reason to change the behaviour, as the Atom
behaviour "feels" cleaner, and because it reduces the work we need to do
in the SELF_IPI path, but a justification needs to be along these lines,
not in terms of being a bugfix.
Perhaps a better reason to make the change is because the VT-x
architecture appears to not overwrite ICR, even on Core designs, and
this would be a behavioural difference between APICV or not.
> Fix the Xen behavior by sending the Self-IPI directly instead of
> emulating it by modifying ICR register.
>
> Fixes: 21c4d089ba2a ("x86 hvm: x2APIC emulation")
> Assisted-by: Claude Fable 5.1
> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
> ---
> Linux/KVM has a similar patch :
>
> KVM: x86: Do not change ICR on write to APIC_SELF_IPI
> https://github.com/torvalds/linux/commit/d22a81b304a27fca6124174a8e842e826c193466
>
> xen/arch/x86/hvm/vlapic.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
> index 885f5d304b..bb105225e2 100644
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -963,6 +963,17 @@ static int cf_check vlapic_mmio_write(
> return X86EMUL_OKAY;
> }
>
> +/*
> + * A write to the SELF IPI register sends a fixed, edge triggered IPI with
> + * the given vector to the local APIC itself. Unlike an ICR write it doesn't
> + * update the ICR, which is guest visible, in x2APIC mode even without any
> + * intervention by Xen when APIC-register virtualization is in use.
> + */
> +static void vlapic_self_ipi(struct vlapic *vlapic, uint32_t val)
> +{
> + vlapic_ipi(vlapic, APIC_DEST_SELF | (val & APIC_VECTOR_MASK), 0);
> +}
I'm not sure a wrapper is really worth it, but if you do care to
continue with the patch, then you want a uint8_t vec here, and drop the
mask.
~Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread