Re: [PATCH v2] x86/vlapic: Don't change ICR on x2APIC SELF_IPI write

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.10.2026 15:03, 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 doesn't require altering the ICR register.
> On most Intel processors (at least Core/P-Core), it does alter ICR,
> but it's not the case on AMD and Atom/E-Core ones, and APICv Self-IPI
> appears to follow a similar behavior.
> 
> For consistency, adjust the Xen behavior to send the Self-IPI
> directly instead of emulating it by modifying ICR register.
> 
> It also clarify the logic.
> 
> Assisted-by: Claude Fable 5.1
> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

However, ...

> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -970,11 +970,13 @@ int vlapic_apicv_write(struct vcpu *v, unsigned int 
> offset)
>  
>      if ( vlapic_x2apic_mode(vlapic) )
>      {
> -        if ( offset != APIC_SELF_IPI )
> -            return X86EMUL_UNHANDLEABLE;

... why the extra churn here? The overall hunk here ...

> +        if ( offset == APIC_SELF_IPI )
> +        {
> +            vlapic_ipi(vlapic, APIC_DEST_SELF | (val & APIC_VECTOR_MASK), 0);
> +            return X86EMUL_OKAY;
> +        }
>  
> -        offset = APIC_ICR;
> -        val = APIC_DEST_SELF | (val & APIC_VECTOR_MASK);
> +        return X86EMUL_UNHANDLEABLE;
>      }
>  
>      vlapic_reg_write(v, offset, val);

... could be

@@ -973,8 +973,8 @@ int vlapic_apicv_write(struct vcpu *v, unsigned int offset)
         if ( offset != APIC_SELF_IPI )
             return X86EMUL_UNHANDLEABLE;
 
-        offset = APIC_ICR;
-        val = APIC_DEST_SELF | (val & APIC_VECTOR_MASK);
+        vlapic_ipi(vlapic, APIC_DEST_SELF | (val & APIC_VECTOR_MASK), 0);
+        return X86EMUL_OKAY;
     }
 
     vlapic_reg_write(v, offset, val);

Much closer to ...

> @@ -1050,9 +1052,8 @@ int guest_wrmsr_x2apic(struct vcpu *v, uint32_t msr, 
> uint64_t val)
>      case APIC_SELF_IPI:
>          if ( val & ~APIC_VECTOR_MASK )
>              return X86EMUL_EXCEPTION;
> -        offset = APIC_ICR;
> -        val = APIC_DEST_SELF | (val & APIC_VECTOR_MASK);
> -        break;
> +        vlapic_ipi(vlapic, APIC_DEST_SELF | (val & APIC_VECTOR_MASK), 0);
> +        return X86EMUL_OKAY;
>  
>      case APIC_EOI:
>      case APIC_ESR:

... this hunk then. (I may take the liberty of making that adjustment while
committing, unless you provide a reason for the bigger change.)

Jan