Re: [PATCH v9 07/18] x86/apic: Add support to send IPI for Secure AVIC

[Borislav Petkov <bp@alien8.de>]

On Thu, Aug 21, 2025 at 10:57:24AM +0530, Upadhyay, Neeraj wrote:
> Is below better?

I was only reacting to that head-spinning, conglomerate of abbreviations "AVIC
GHCB APIC MSR".

> x86/apic: Add support to send IPI for Secure AVIC
> 
> Secure AVIC hardware only accelerates Self-IPI, i.e. on WRMSR to
> APIC_SELF_IPI and APIC_ICR (with destination shorthand equal to Self)
> registers, hardware takes care of updating the APIC_IRR in the APIC
> backing page of the vCPU. For other IPI types (cross-vCPU, broadcast IPIs),
> software needs to take care of updating the APIC_IRR state of the target
> CPUs and to ensure that the target vCPUs notice the new pending interrupt.
> 
> Add new callbacks in the Secure AVIC driver for sending IPI requests. These
> callbacks update the IRR in the target guest vCPU's APIC backing page. To
> ensure that the remote vCPU notices the new pending interrupt, reuse the
> GHCB MSR handling code in vc_handle_msr() to issue APIC_ICR MSR-write GHCB
> protocol event to the hypervisor. For Secure AVIC guests, on APIC_ICR write
> MSR exits, the hypervisor notifies the target vCPU by either sending an AVIC
> doorbell (if target vCPU is running) or by waking up the non-running target
> vCPU.

But I'll take a definitely better commit message too! :-)

> Ok moving it to x2apic_savic.c requires below 4 sev-internal declarations to
> be moved to arch/x86/include/asm/sev.h
> 
> struct ghcb_state;
> struct ghcb *__sev_get_ghcb(struct ghcb_state *state);
> void __sev_put_ghcb(struct ghcb_state *state);
> enum es_result sev_es_ghcb_handle_msr(...);

Well, do you anticipate needing any more sev* facilities for SAVIC?

If so, you probably should carve them out into arch/x86/coco/sev/savic.c

If only 4 functions, I guess they're probably still ok in .../sev/core.c

> This comment explains why WRMSR is sufficient for sending SELF_IPI. On
> WRMSR by vCPU, Secure AVIC hardware takes care of updating APIC_IRR in
> backing page. Hardware also ensures that new APIC_IRR state is evaluated
> for new pending interrupts. So, WRMSR is hardware-accelerated.
> 
> For non-self-IPI case, software need to do APIC_IRR update and sending of
> wakeup-request/doorbell to the target vCPU.

Yeah, you need to rewrite it like the commit message above - it needs to say
that upon the MSR write, hw does this and that and therefore accelerates this
type of IPI.

Then it is clear what you mean by "acceleration."

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette