Re: [PATCH] x86/apic/savic: Forward APIC_SPIV writes to the hypervisor

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] x86/apic: lost interrupts due to incorrect APIC_SPIV update order
--

commit 26f070fb302ca5a93ef870d540aaf2cf4fb25a54
Author: Shruti <shrutiss@google.com>

x86/apic/savic: Forward APIC_SPIV writes to the hypervisor

This commit updates savic_write() to forward APIC_SPIV writes to the
hypervisor so that the emulated APIC state stays in sync with the guest's
local backing page.

> diff --git a/arch/x86/kernel/apic/x2apic_savic.c b/arch/x86/kernel/apic/x2apic_savic.c
> --- a/arch/x86/kernel/apic/x2apic_savic.c
> +++ b/arch/x86/kernel/apic/x2apic_savic.c
[ ... ]
> @@ -223,6 +222,10 @@ static void savic_write(u32 reg, u32 data)
>  	case APIC_EILVTn(0) ... APIC_EILVTn(3):
>  		apic_set_reg(ap, reg, data);
>  		break;
> +	case APIC_SPIV:
> +		savic_ghcb_msr_write(reg, data);
> +		apic_set_reg(ap, reg, data);

[Severity: High]
Does this specific ordering cause a regression where interrupts could be
dropped?

When a vCPU writes to APIC_SPIV to enable the APIC, savic_ghcb_msr_write()
notifies the hypervisor. The hypervisor then marks the emulated APIC as
software-enabled and may immediately inject a pending interrupt before
resuming the guest.

When the guest resumes, the hardware AVIC evaluates the injected interrupt
against the local APIC backing page. Because apic_set_reg() has not yet
executed, the local backing page still reflects a software-disabled APIC,
causing the hardware AVIC to drop the interrupt.

Would reversing the order of these two calls ensure the local backing page
is ready before the hypervisor is notified?

> +		break;
>  	case APIC_ICR:
>  		savic_icr_write(data, 0);
>  		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929055255.1380202-1-shrutiss@google.com?part=1