mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ardb@kernel.org>
To: "Melody Wang" <huibo.wang@amd.com>, x86@kernel.org
Cc: LKML <linux-kernel@vger.kernel.org>,
	"Tom Lendacky" <thomas.lendacky@amd.com>,
	"Jon Lange" <jlange@microsoft.com>
Subject: Re: [PATCH v3 0/8] Alternate Injection: Secure Interrupt Delivery for SEV-SNP Guests - Guest Support
Date: Mon, 28 Sep 2026 09:14:41 +0200	[thread overview]
Message-ID: <335670cd-db18-4442-a3a9-5c99daa7d262@app.fastmail.com> (raw)
In-Reply-To: <d6c409da-44b8-4c17-bba8-dc5aeee219c5@amd.com>

Hi Melody,

On Mon, 28 Sep 2026, at 04:55, Melody Wang wrote:
> Hi Ard,
>
> On 9/24/26 12:06 PM, Ard Biesheuvel wrote:
>> 
>> That is not what I am suggesting.
>> 
>> What I would like to see is an abstraction implemented in OVMF that encapsulates
>> the logic that you are adding here. All the EFI stub would have to do is call
>> the protocol, nothing more.
>> 
>> After ExitBootServices() is a different matter, and actually, I think doing
>> the memory acceptance at that point was a mistake, and I'd like to fix that
>> but that is a separate discussion.
>> 
>> Before ExitBootServices(), we should not be poking MSRs directly. We should
>> be relying on the abstractions exposed by the firmware.
>> 
>
> ok, see below what I did, I think the following is perhaps what you had 
> in mind but please let me know if that is ok this way.
>
> In there, the guest kernel will register Alternate Injection through 
> OVMF if Alternate Injection is enabled in the special sev_status MSR.
>
> The OVMF will check if there is an SVSM present and Alternate Injection 
> is enabled or not, if yes, it will register Alternate Injection for the 
> guest kernel. Otherwise, it will not. A failure of registering Alternate 
> Injection will terminate the guest.
>
> Cc Jon Lange, the Alternate Injection spec author as an FYI.
>

This looks much better thanks.

> -----------------------------------------------------------------------
>
> Kernel code:
>
> diff --git a/arch/x86/boot/compressed/sev.h b/arch/x86/boot/compressed/sev.h
> index 22637b416b46..62e50c2e71ed 100644
> --- a/arch/x86/boot/compressed/sev.h
> +++ b/arch/x86/boot/compressed/sev.h
> @@ -14,7 +14,6 @@
>
>   void snp_accept_memory(phys_addr_t start, phys_addr_t end);
>   u64 sev_get_status(void);
> -bool early_is_sevsnp_guest(void);
>
>   static inline u64 sev_es_rd_ghcb_msr(void)
>   {
> @@ -37,7 +36,6 @@ static inline void sev_es_wr_ghcb_msr(u64 val)
>
>   static inline void snp_accept_memory(phys_addr_t start, phys_addr_t 
> end) { }
>   static inline u64 sev_get_status(void) { return 0; }
> -static inline bool early_is_sevsnp_guest(void) { return false; }
>
>   #endif
>
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 93161b663d9b..13f2e8c009b4 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -471,6 +471,8 @@ static __always_inline void sev_es_nmi_complete(void)
>   extern int __init sev_es_efi_map_ghcbs_cas(pgd_t *pgd);
>   extern void sev_enable(struct boot_params *bp);
>
> +bool early_is_sevsnp_guest(void);
> +
>   /*
>    * RMPADJUST modifies the RMP permissions of a page of a lesser-
>    * privileged (numerically higher) VMPL.
> diff --git a/drivers/firmware/efi/libstub/efistub.h 
> b/drivers/firmware/efi/libstub/efistub.h
> index fd91fc15ec81..d65cdc8f2acd 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -177,6 +177,17 @@ void efi_set_u64_split(u64 data, u32 *lo, u32 *hi)
>    */
>   #define EFI_MMAP_NR_SLACK_SLOTS        32
>
> +typedef union sev_alt_inj_register_protocol 
> sev_alt_inj_register_protocol_t;
> +union sev_alt_inj_register_protocol {
> +       struct {
> +               efi_status_t (__efiapi * sev_register_alt_inj)
> +                       (sev_alt_inj_register_protocol_t *);
> +       };
> +       struct {
> +               u32 sev_register_alt_inj;
> +       } mixed_mode;
> +};
> +
>   typedef struct efi_generic_dev_path efi_device_path_protocol_t;
>
>   union efi_device_path_to_text_protocol {
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c 
> b/drivers/firmware/efi/libstub/x86-stub.c
> index 2a1fa773d241..b6e58ade4162 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -783,6 +783,36 @@ static efi_status_t exit_boot(struct boot_params 
> *boot_params, void *handle)
>          return EFI_SUCCESS;
>   }
>
> +/*
> + * When the guest is running at VMPL2 with Alternate Injection enabled,
> + * register Alternate Injection before ExitBootServices().
> + */
> +static int svsm_register_alt_inj(void)
> +{
> +       efi_guid_t alt_inj_proto = OVMF_SEV_ALT_INJ_REGISTER_PROTOCOL_GUID;
> +       sev_alt_inj_register_protocol_t *proto;
> +       efi_status_t status;
> +
> +       if (early_is_sevsnp_guest() && snp_vmpl) {
> +               if (!(sev_get_status() & MSR_AMD64_SNP_ALTERNATE_INJ))
> +                       return 0;
> +       }
> +

No need to test this - the protocol should deal with this.

> +       status = efi_bs_call(locate_protocol, &alt_inj_proto, NULL, 
> (void **)&proto);
> +       if (status != EFI_SUCCESS) {
> +               efi_err("Alternate Injection registration protocol not 
> exist\n");
> +               return 1;
> +       }
> +

Just ignore the error

> +       status = efi_call_proto(proto, sev_register_alt_inj);
> +       if (status != EFI_SUCCESS) {
> +               efi_err("Alternate Injection registration protocol 
> failed\n");
> +               return 2;
> +       }
> +

Distinguish here between EFI_UNSUPPORTED (which can be ignored) and
other errors.

Only question is whether EFI stub and OVMF are guaranteed to be in sync
wrt snp_vmpl versus AmdSvsmIsSvsmPresent(), but I guess things would
not work at all if that is not the case?


> +       return 0;
> +}
> +
>   static bool sev_prepare(void)
>   {
>          u64 unsupported;
> @@ -793,6 +823,10 @@ static bool sev_prepare(void)
>                          unsupported);
>                  return false;
>          }
> +
> +       if (svsm_register_alt_inj())
> +               return false;
> +
>          return true;
>   }
>
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index aa15ff88539b..ebf7b4b9a292 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -444,6 +444,8 @@ void efi_native_runtime_setup(void);
>   #define OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL_GUID 
> EFI_GUID(0xc5a010fe, 0x38a7, 0x4531,  0x8a, 0x4a, 0x05, 0x00, 0xd2, 
> 0xfd, 0x16, 0x49)
>   #define OVMF_MEMORY_LOG_TABLE_GUID             EFI_GUID(0x95305139, 
> 0xb20f, 0x4723,  0x84, 0x25, 0x62, 0x7c, 0x88, 0x8f, 0xf1, 0x21)
>
> +#define OVMF_SEV_ALT_INJ_REGISTER_PROTOCOL_GUID 
> EFI_GUID(0x3cf00587, 0x2329, 0x4c44,  0xb1, 0x84, 0x8f, 0xd9, 0x87, 
> 0x00, 0x42, 0xe0)
> +
>   typedef struct {
>          efi_guid_t guid;
>          u64 table;
> ---------------------------------------------------------------------
>
> OVMF code:
>
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index f10f37719527..c7f471edd838 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -24,6 +24,7 @@
>   #include <Library/PcdLib.h>
>   #include <Pi/PiDxeCis.h>
>   #include <Protocol/SevMemoryAcceptance.h>
> +#include <Protocol/SevAltInjRegister.h>^M
>   #include <Protocol/MemoryAccept.h>
>   #include <Uefi/UefiSpec.h>
>
> @@ -193,6 +194,42 @@ STATIC EDKII_MEMORY_ACCEPT_PROTOCOL 
> mMemoryAcceptProtocol = {
>     AmdSevMemoryAccept
>   };
>

Add STATIC here

> +VOID^M

> +EFIAPI^M
> +SvsmRegisterAltInj (^M
> +  VOID^M
> +  )^M
> +{^M
> +  UINT8 vector = 0x02;^M
> +  AmdSvsmSnpApicConfigEmulation(vector);^M
> +}^M
> +^M
> +/**^M
> +  Register the guest kernel with Alternate Injection.^M
> +**/^M
> +STATIC^M
> +EFI_STATUS^M
> +EFIAPI^M
> +SevRegisterAltInj (^M
> +  IN OVMF_SEV_ALT_INJ_REGISTER_PROTOCOL  *This^M
> +  )^M
> +{^M
> +  if (This == NULL) {^M
> +    return EFI_INVALID_PARAMETER;^M
> +  }^M
> +^M
> +  if (!(AmdSvsmIsSvsmPresent() && AlternateInjectionEnabled())) {^M

Please make this !cond || !cond

> +    return EFI_UNSUPPORTED;^M
> +  }^M
> +^M
> +  SvsmRegisterAltInj ();^M
> +^M
> +  return EFI_SUCCESS;^M
> +}^M
> +^M
> +STATIC^M
> +OVMF_SEV_ALT_INJ_REGISTER_PROTOCOL^M
> +  mSevAltInjRegisterProtocol = { SevRegisterAltInj };^M
>
>   EFI_EVENT  mDeregisterAlternateInjectionEvent = NULL;
>   /**
> @@ -355,6 +392,8 @@ AmdSevDxeEntryPoint (
>                       &mMemoryAcceptProtocol,
>                       &gOvmfSevMemoryAcceptanceProtocolGuid,
>                       &mMemoryAcceptanceProtocol,
> +                    &gOvmfSevAltInjRegisterProtocolGuid,^M
> +                    &mSevAltInjRegisterProtocol,^M
>                       NULL
>                       );
>       ASSERT_EFI_ERROR (Status);
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf 
> b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> index 0db38471020a..bc6c1827e649 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> @@ -53,6 +53,7 @@
>   [Protocols]
>     gEdkiiMemoryAcceptProtocolGuid
>     gOvmfSevMemoryAcceptanceProtocolGuid
> +  gOvmfSevAltInjRegisterProtocolGuid^M
>
>   [Guids]
>     gConfidentialComputingSevSnpBlobGuid
> diff --git a/OvmfPkg/Include/Protocol/SevAltInjRegister.h 
> b/OvmfPkg/Include/Protocol/SevAltInjRegister.h
> new file mode 100644
> index 000000000000..cd6c32384a51
> --- /dev/null
> +++ b/OvmfPkg/Include/Protocol/SevAltInjRegister.h
> @@ -0,0 +1,28 @@
> +#pragma once
> +
> +#define OVMF_SEV_ALT_INJ_REGISTER_PROTOCOL_GUID \
> +  { 0x3cf00587, \
> +    0x2329, \
> +    0x4c44, \
> +    {0xb1, 0x84, 0x8f, 0xd9, 0x87, 0x00, 0x42, 0xe0}}
> +
> +typedef struct _OVMF_SEV_ALT_INJ_REGISTER_PROTOCOL
> +    OVMF_SEV_ALT_INJ_REGISTER_PROTOCOL;
> +
> +/**
> +  Register the caller (guest kernel) as an Alternate Injection consumer.
> +  Wraps the SVSM registration; increments the SVSM registration counter.
> +
> +  @retval EFI_SUCCESS           Registration succeeded.
> +  @retval EFI_UNSUPPORTED       Alt-Injection not available on this 
> platform.
> +  @retval EFI_DEVICE_ERROR      SVSM call failed.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *OVMF_SEV_REGISTER_ALT_INJ)(
> +  IN OVMF_SEV_ALT_INJ_REGISTER_PROTOCOL  *This
> +  );
> +
> +struct _OVMF_SEV_ALT_INJ_REGISTER_PROTOCOL {
> +  OVMF_SEV_REGISTER_ALT_INJ  SevRegisterAltInj;
> +};
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 50de41b7fcc8..cc3fa86a30d4 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -213,6 +213,7 @@
>     gQemuAcpiTableNotifyProtocolGuid      = {0x928939b2, 0x4235, 0x462f, 
> {0x95, 0x80, 0xf6, 0xa2, 0xb2, 0xc2, 0x1a, 0x4f}}
>     gEfiMpInitLibMpDepProtocolGuid        = {0xbb00a5ca, 0x8ce,  0x462f, 
> {0xa5, 0x37, 0x43, 0xc7, 0x4a, 0x82, 0x5c, 0xa4}}
>     gEfiMpInitLibUpDepProtocolGuid        = {0xa9e7cef1, 0x5682, 0x42cc, 
> {0xb1, 0x23, 0x99, 0x30, 0x97, 0x3f, 0x4a, 0x9f}}
> +  gOvmfSevAltInjRegisterProtocolGuid    = {0x3cf00587, 0x2329, 0x4c44, 
> {0xb1, 0x84, 0x8f, 0xd9, 0x87, 0x00, 0x42, 0xe0}}^M
>
>   [PcdsFixedAtBuild]
>     gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|0x0|UINT32|0
>
> -- 
> Thanks,
> Melody

      reply	other threads:[~2026-09-28  7:15 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-20 18:12 Melody Wang
2026-09-20 18:12 ` [PATCH v3 1/8] x86/sev: Make SVSM calls preemption-safe Melody Wang
2026-09-20 18:16   ` Melody Wang
2026-09-20 18:12 ` [PATCH v3 2/8] x86/sev: Add support for Alternate Injection Melody Wang
2026-09-20 18:16   ` Melody Wang
2026-09-20 18:16 ` [PATCH v3 0/8] Alternate Injection: Secure Interrupt Delivery for SEV-SNP Guests - Guest Support Melody Wang
2026-09-20 18:16 ` [PATCH v3 3/8] x86/apic: Add an SVSM APIC driver Melody Wang
2026-09-22  1:31   ` Melody Wang
2026-09-23 20:31   ` Borislav Petkov
2026-09-20 18:16 ` [PATCH v3 4/8] x86/sev: Route unsupported APIC register accesses to the hypervisor APIC emulation Melody Wang
2026-09-20 18:16 ` [PATCH v3 5/8] x86/sev: Add a function to contain all SEV-specific setup operations Melody Wang
2026-09-20 18:16 ` [PATCH v3 6/8] x86/sev: Register the guest with the SVSM APIC protocol Melody Wang
2026-09-23 20:46   ` Melody Wang
2026-09-20 18:16 ` [PATCH v3 7/8] x86/sev: Allow the guest to configure interrupt vectors for the hypervisor Melody Wang
2026-09-23 20:46   ` Melody Wang
2026-09-20 18:17 ` [PATCH v3 8/8] x86/sev: Indicate that Alternate Injection is supported in the guest Melody Wang
2026-09-23 20:56 ` [PATCH v3 0/8] Alternate Injection: Secure Interrupt Delivery for SEV-SNP Guests - Guest Support Ard Biesheuvel
2026-09-24 18:34   ` Melody Wang
2026-09-24 19:06     ` Ard Biesheuvel
2026-09-25  4:47       ` Borislav Petkov
2026-09-25  5:44         ` Ard Biesheuvel
2026-09-25  5:47           ` Borislav Petkov
2026-09-28  2:55       ` Melody Wang
2026-09-28  7:14         ` Ard Biesheuvel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=335670cd-db18-4442-a3a9-5c99daa7d262@app.fastmail.com \
    --to=ardb@kernel.org \
    --cc=huibo.wang@amd.com \
    --cc=jlange@microsoft.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=thomas.lendacky@amd.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®