[v5,19/20] x86/efistub: Perform SNP feature test while running in the firmware

Message ID 20230607072342.4054036-20-ardb@kernel.org
State New
Headers
Series efi/x86: Avoid bare metal decompressor during EFI boot |

Commit Message

Ard Biesheuvel June 7, 2023, 7:23 a.m. UTC
  Before refactoring the EFI stub boot flow to avoid the legacy bare metal
decompressor, duplicate the SNP feature check in the EFI stub before
handing over to the kernel proper.

The SNP feature check can be performed while running under the EFI boot
services, which means we can fail gracefully and return an error to the
bootloader if the loaded kernel does not implement support for all the
features that the hypervisor enabled.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/boot/compressed/sev.c          | 71 +++++++++++---------
 arch/x86/include/asm/sev.h              |  4 ++
 drivers/firmware/efi/libstub/x86-stub.c | 17 +++++
 3 files changed, 62 insertions(+), 30 deletions(-)
  

Comments

Tom Lendacky June 7, 2023, 4:07 p.m. UTC | #1
On 6/7/23 02:23, Ard Biesheuvel wrote:
> Before refactoring the EFI stub boot flow to avoid the legacy bare metal
> decompressor, duplicate the SNP feature check in the EFI stub before
> handing over to the kernel proper.
> 
> The SNP feature check can be performed while running under the EFI boot
> services, which means we can fail gracefully and return an error to the
> bootloader if the loaded kernel does not implement support for all the
> features that the hypervisor enabled.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>   arch/x86/boot/compressed/sev.c          | 71 +++++++++++---------
>   arch/x86/include/asm/sev.h              |  4 ++
>   drivers/firmware/efi/libstub/x86-stub.c | 17 +++++
>   3 files changed, 62 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> index 09dc8c187b3cc752..9593bc80c9c6b89d 100644
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c

...

> -void sev_enable(struct boot_params *bp)
> +u64 sev_get_status(void)
>   {
>   	unsigned int eax, ebx, ecx, edx;
>   	struct msr m;
> +
> +	/* Check for the SME/SEV support leaf */
> +	eax = 0x80000000;
> +	ecx = 0;
> +	native_cpuid(&eax, &ebx, &ecx, &edx);
> +	if (eax < 0x8000001f)
> +		return 0;
> +
> +	/*
> +	 * Check for the SME/SEV feature:
> +	 *   CPUID Fn8000_001F[EAX]
> +	 *   - Bit 0 - Secure Memory Encryption support
> +	 *   - Bit 1 - Secure Encrypted Virtualization support
> +	 *   CPUID Fn8000_001F[EBX]
> +	 *   - Bits 5:0 - Pagetable bit position used to indicate encryption
> +	 */
> +	eax = 0x8000001f;
> +	ecx = 0;
> +	native_cpuid(&eax, &ebx, &ecx, &edx);
> +	/* Check whether SEV is supported */
> +	if (!(eax & BIT(1)))
> +		return 0;
> +
> +	/* Set the SME mask if this is an SEV guest. */
> +	sme_me_mask = BIT_ULL(ebx & 0x3f);
> +
> +	boot_rdmsr(MSR_AMD64_SEV, &m);
> +	return m.q;
> +}
> +
> +void sev_enable(struct boot_params *bp)
> +{
>   	bool snp;
>   
>   	/*
> @@ -410,37 +447,13 @@ void sev_enable(struct boot_params *bp)
>   	 */
>   	snp = snp_init(bp);
>   
> -	/* Check for the SME/SEV support leaf */
> -	eax = 0x80000000;
> -	ecx = 0;
> -	native_cpuid(&eax, &ebx, &ecx, &edx);
> -	if (eax < 0x8000001f)
> -		return;
> -
> -	/*
> -	 * Check for the SME/SEV feature:
> -	 *   CPUID Fn8000_001F[EAX]
> -	 *   - Bit 0 - Secure Memory Encryption support
> -	 *   - Bit 1 - Secure Encrypted Virtualization support
> -	 *   CPUID Fn8000_001F[EBX]
> -	 *   - Bits 5:0 - Pagetable bit position used to indicate encryption
> -	 */
> -	eax = 0x8000001f;
> -	ecx = 0;
> -	native_cpuid(&eax, &ebx, &ecx, &edx);
> -	/* Check whether SEV is supported */
> -	if (!(eax & BIT(1))) {
> +	sev_status = sev_get_status();
> +	if (!(sev_status & MSR_AMD64_SEV_ENABLED)) {
>   		if (snp)
>   			error("SEV-SNP support indicated by CC blob, but not CPUID.");

This ends up checking the CPUID path because if SEV isn't advertised in 
CPUID the returned status value is 0. But it also checks the SEV_STATUS 
MSR as well. So I think you can remove the SNP / SEV_STATUS check at the 
end of this function (since that check is identical to this now) and just 
update the message to indicate not CPUID or SEV status MSR.

The sme_me_mask should probably be cleared at this point before returning, 
too. Or, alternately, in sev_get_status(), you can update the setting of 
sme_me_mask to based on MSR_AMD64_SEV_ENABLED being set in the SEV_STATUS MSR.

>   		return;
>   	}
>   
> -	/* Set the SME mask if this is an SEV guest. */
> -	boot_rdmsr(MSR_AMD64_SEV, &m);
> -	sev_status = m.q;
> -	if (!(sev_status & MSR_AMD64_SEV_ENABLED))
> -		return;
> -
>   	/* Negotiate the GHCB protocol version. */
>   	if (sev_status & MSR_AMD64_SEV_ES_ENABLED) {
>   		if (!sev_es_negotiate_protocol())
> @@ -460,8 +473,6 @@ void sev_enable(struct boot_params *bp)
>   
>   	if (snp && !(sev_status & MSR_AMD64_SEV_SNP_ENABLED))
>   		error("SEV-SNP supported indicated by CC blob, but not SEV status MSR.");
> -
> -	sme_me_mask = BIT_ULL(ebx & 0x3f);
>   }
>   
>   /* Search for Confidential Computing blob in the EFI config table. */
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 86e1296e87f513b7..081c39b0e8d0d208 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -207,6 +207,8 @@ bool snp_init(struct boot_params *bp);
>   void __init __noreturn snp_abort(void);
>   int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio);
>   void snp_accept_memory(phys_addr_t start, phys_addr_t end);
> +u64 snp_get_unsupported_features(u64 status);
> +u64 sev_get_status(void);
>   #else
>   static inline void sev_es_ist_enter(struct pt_regs *regs) { }
>   static inline void sev_es_ist_exit(void) { }
> @@ -232,6 +234,8 @@ static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *in
>   }
>   
>   static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
> +static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
> +static inline u64 sev_get_status(void) { return 0; }
>   #endif
>   
>   #endif
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index abcd5703e9f3f980..1015ef883f5850a4 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -15,6 +15,7 @@
>   #include <asm/setup.h>
>   #include <asm/desc.h>
>   #include <asm/boot.h>
> +#include <asm/sev.h>
>   
>   #include "efistub.h"
>   #include "x86-stub.h"
> @@ -790,6 +791,19 @@ static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
>   	return EFI_SUCCESS;
>   }
>   
> +static bool have_unsupported_snp_features(void)
> +{
> +	u64 unsupported;
> +
> +	unsupported = snp_get_unsupported_features(sev_get_status());

This will also set sme_me_mask, but I think that is ok, since on error 
things will terminate, otherwise sev_enable() should update appropriately 
later.

Thanks,
Tom

> +	if (unsupported) {
> +		efi_err("Unsupported SEV-SNP features detected: 0x%llx\n",
> +			unsupported);
> +		return true;
> +	}
> +	return false;
> +}
> +
>   static void __noreturn enter_kernel(unsigned long kernel_addr,
>   				    struct boot_params *boot_params)
>   {
> @@ -820,6 +834,9 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
>   	if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
>   		efi_exit(handle, EFI_INVALID_PARAMETER);
>   
> +	if (have_unsupported_snp_features())
> +		efi_exit(handle, EFI_UNSUPPORTED);
> +
>   	if (IS_ENABLED(CONFIG_EFI_DXE_MEM_ATTRIBUTES)) {
>   		efi_dxe_table = get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID);
>   		if (efi_dxe_table &&
  
Ard Biesheuvel June 7, 2023, 4:51 p.m. UTC | #2
On Wed, 7 Jun 2023 at 18:08, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 6/7/23 02:23, Ard Biesheuvel wrote:
> > Before refactoring the EFI stub boot flow to avoid the legacy bare metal
> > decompressor, duplicate the SNP feature check in the EFI stub before
> > handing over to the kernel proper.
> >
> > The SNP feature check can be performed while running under the EFI boot
> > services, which means we can fail gracefully and return an error to the
> > bootloader if the loaded kernel does not implement support for all the
> > features that the hypervisor enabled.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >   arch/x86/boot/compressed/sev.c          | 71 +++++++++++---------
> >   arch/x86/include/asm/sev.h              |  4 ++
> >   drivers/firmware/efi/libstub/x86-stub.c | 17 +++++
> >   3 files changed, 62 insertions(+), 30 deletions(-)
> >
> > diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
> > index 09dc8c187b3cc752..9593bc80c9c6b89d 100644
> > --- a/arch/x86/boot/compressed/sev.c
> > +++ b/arch/x86/boot/compressed/sev.c
>
> ...
>
> > -void sev_enable(struct boot_params *bp)
> > +u64 sev_get_status(void)
> >   {
> >       unsigned int eax, ebx, ecx, edx;
> >       struct msr m;
> > +
> > +     /* Check for the SME/SEV support leaf */
> > +     eax = 0x80000000;
> > +     ecx = 0;
> > +     native_cpuid(&eax, &ebx, &ecx, &edx);
> > +     if (eax < 0x8000001f)
> > +             return 0;
> > +
> > +     /*
> > +      * Check for the SME/SEV feature:
> > +      *   CPUID Fn8000_001F[EAX]
> > +      *   - Bit 0 - Secure Memory Encryption support
> > +      *   - Bit 1 - Secure Encrypted Virtualization support
> > +      *   CPUID Fn8000_001F[EBX]
> > +      *   - Bits 5:0 - Pagetable bit position used to indicate encryption
> > +      */
> > +     eax = 0x8000001f;
> > +     ecx = 0;
> > +     native_cpuid(&eax, &ebx, &ecx, &edx);
> > +     /* Check whether SEV is supported */
> > +     if (!(eax & BIT(1)))
> > +             return 0;
> > +
> > +     /* Set the SME mask if this is an SEV guest. */
> > +     sme_me_mask = BIT_ULL(ebx & 0x3f);
> > +
> > +     boot_rdmsr(MSR_AMD64_SEV, &m);
> > +     return m.q;
> > +}
> > +
> > +void sev_enable(struct boot_params *bp)
> > +{
> >       bool snp;
> >
> >       /*
> > @@ -410,37 +447,13 @@ void sev_enable(struct boot_params *bp)
> >        */
> >       snp = snp_init(bp);
> >
> > -     /* Check for the SME/SEV support leaf */
> > -     eax = 0x80000000;
> > -     ecx = 0;
> > -     native_cpuid(&eax, &ebx, &ecx, &edx);
> > -     if (eax < 0x8000001f)
> > -             return;
> > -
> > -     /*
> > -      * Check for the SME/SEV feature:
> > -      *   CPUID Fn8000_001F[EAX]
> > -      *   - Bit 0 - Secure Memory Encryption support
> > -      *   - Bit 1 - Secure Encrypted Virtualization support
> > -      *   CPUID Fn8000_001F[EBX]
> > -      *   - Bits 5:0 - Pagetable bit position used to indicate encryption
> > -      */
> > -     eax = 0x8000001f;
> > -     ecx = 0;
> > -     native_cpuid(&eax, &ebx, &ecx, &edx);
> > -     /* Check whether SEV is supported */
> > -     if (!(eax & BIT(1))) {
> > +     sev_status = sev_get_status();
> > +     if (!(sev_status & MSR_AMD64_SEV_ENABLED)) {
> >               if (snp)
> >                       error("SEV-SNP support indicated by CC blob, but not CPUID.");
>
> This ends up checking the CPUID path because if SEV isn't advertised in
> CPUID the returned status value is 0. But it also checks the SEV_STATUS
> MSR as well. So I think you can remove the SNP / SEV_STATUS check at the
> end of this function (since that check is identical to this now) and just
> update the message to indicate not CPUID or SEV status MSR.
>

But that one checks for MSR_AMD64_SEV_SNP_ENABLED not
MSR_AMD64_SEV_ENABLED. Does that matter at all?

> The sme_me_mask should probably be cleared at this point before returning,
> too. Or, alternately, in sev_get_status(), you can update the setting of
> sme_me_mask to based on MSR_AMD64_SEV_ENABLED being set in the SEV_STATUS MSR.
>

I'll go for the latter, seems cleaner not to touch it in that case.

--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -422,10 +422,12 @@ u64 sev_get_status(void)
        if (!(eax & BIT(1)))
                return 0;

-       /* Set the SME mask if this is an SEV guest. */
-       sme_me_mask = BIT_ULL(ebx & 0x3f);
-
        boot_rdmsr(MSR_AMD64_SEV, &m);
+
+       /* Set the SME mask if this is an SEV guest. */
+       if (m.q & MSR_AMD64_SEV_ENABLED)
+               sme_me_mask = BIT_ULL(ebx & 0x3f);
+
        return m.q;
 }


> >   /* Search for Confidential Computing blob in the EFI config table. */
> > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> > index 86e1296e87f513b7..081c39b0e8d0d208 100644
> > --- a/arch/x86/include/asm/sev.h
> > +++ b/arch/x86/include/asm/sev.h
> > @@ -207,6 +207,8 @@ bool snp_init(struct boot_params *bp);
> >   void __init __noreturn snp_abort(void);
> >   int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio);
> >   void snp_accept_memory(phys_addr_t start, phys_addr_t end);
> > +u64 snp_get_unsupported_features(u64 status);
> > +u64 sev_get_status(void);
> >   #else
> >   static inline void sev_es_ist_enter(struct pt_regs *regs) { }
> >   static inline void sev_es_ist_exit(void) { }
> > @@ -232,6 +234,8 @@ static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *in
> >   }
> >
> >   static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
> > +static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
> > +static inline u64 sev_get_status(void) { return 0; }
> >   #endif
> >
> >   #endif
> > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> > index abcd5703e9f3f980..1015ef883f5850a4 100644
> > --- a/drivers/firmware/efi/libstub/x86-stub.c
> > +++ b/drivers/firmware/efi/libstub/x86-stub.c
> > @@ -15,6 +15,7 @@
> >   #include <asm/setup.h>
> >   #include <asm/desc.h>
> >   #include <asm/boot.h>
> > +#include <asm/sev.h>
> >
> >   #include "efistub.h"
> >   #include "x86-stub.h"
> > @@ -790,6 +791,19 @@ static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
> >       return EFI_SUCCESS;
> >   }
> >
> > +static bool have_unsupported_snp_features(void)
> > +{
> > +     u64 unsupported;
> > +
> > +     unsupported = snp_get_unsupported_features(sev_get_status());
>
> This will also set sme_me_mask, but I think that is ok, since on error
> things will terminate, otherwise sev_enable() should update appropriately
> later.
>

OK

> > +     if (unsupported) {
> > +             efi_err("Unsupported SEV-SNP features detected: 0x%llx\n",
> > +                     unsupported);
> > +             return true;
> > +     }
> > +     return false;
> > +}
> > +
> >   static void __noreturn enter_kernel(unsigned long kernel_addr,
> >                                   struct boot_params *boot_params)
> >   {
> > @@ -820,6 +834,9 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> >       if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
> >               efi_exit(handle, EFI_INVALID_PARAMETER);
> >
> > +     if (have_unsupported_snp_features())
> > +             efi_exit(handle, EFI_UNSUPPORTED);
> > +
> >       if (IS_ENABLED(CONFIG_EFI_DXE_MEM_ATTRIBUTES)) {
> >               efi_dxe_table = get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID);
> >               if (efi_dxe_table &&
  
Tom Lendacky June 7, 2023, 5:29 p.m. UTC | #3
On 6/7/23 11:51, Ard Biesheuvel wrote:
> On Wed, 7 Jun 2023 at 18:08, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>>
>> On 6/7/23 02:23, Ard Biesheuvel wrote:
>>> Before refactoring the EFI stub boot flow to avoid the legacy bare metal
>>> decompressor, duplicate the SNP feature check in the EFI stub before
>>> handing over to the kernel proper.
>>>
>>> The SNP feature check can be performed while running under the EFI boot
>>> services, which means we can fail gracefully and return an error to the
>>> bootloader if the loaded kernel does not implement support for all the
>>> features that the hypervisor enabled.
>>>
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> ---
>>>    arch/x86/boot/compressed/sev.c          | 71 +++++++++++---------
>>>    arch/x86/include/asm/sev.h              |  4 ++
>>>    drivers/firmware/efi/libstub/x86-stub.c | 17 +++++
>>>    3 files changed, 62 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
>>> index 09dc8c187b3cc752..9593bc80c9c6b89d 100644
>>> --- a/arch/x86/boot/compressed/sev.c
>>> +++ b/arch/x86/boot/compressed/sev.c
>>
>> ...
>>
>>> -void sev_enable(struct boot_params *bp)
>>> +u64 sev_get_status(void)
>>>    {
>>>        unsigned int eax, ebx, ecx, edx;
>>>        struct msr m;
>>> +
>>> +     /* Check for the SME/SEV support leaf */
>>> +     eax = 0x80000000;
>>> +     ecx = 0;
>>> +     native_cpuid(&eax, &ebx, &ecx, &edx);
>>> +     if (eax < 0x8000001f)
>>> +             return 0;
>>> +
>>> +     /*
>>> +      * Check for the SME/SEV feature:
>>> +      *   CPUID Fn8000_001F[EAX]
>>> +      *   - Bit 0 - Secure Memory Encryption support
>>> +      *   - Bit 1 - Secure Encrypted Virtualization support
>>> +      *   CPUID Fn8000_001F[EBX]
>>> +      *   - Bits 5:0 - Pagetable bit position used to indicate encryption
>>> +      */
>>> +     eax = 0x8000001f;
>>> +     ecx = 0;
>>> +     native_cpuid(&eax, &ebx, &ecx, &edx);
>>> +     /* Check whether SEV is supported */
>>> +     if (!(eax & BIT(1)))
>>> +             return 0;
>>> +
>>> +     /* Set the SME mask if this is an SEV guest. */
>>> +     sme_me_mask = BIT_ULL(ebx & 0x3f);
>>> +
>>> +     boot_rdmsr(MSR_AMD64_SEV, &m);
>>> +     return m.q;
>>> +}
>>> +
>>> +void sev_enable(struct boot_params *bp)
>>> +{
>>>        bool snp;
>>>
>>>        /*
>>> @@ -410,37 +447,13 @@ void sev_enable(struct boot_params *bp)
>>>         */
>>>        snp = snp_init(bp);
>>>
>>> -     /* Check for the SME/SEV support leaf */
>>> -     eax = 0x80000000;
>>> -     ecx = 0;
>>> -     native_cpuid(&eax, &ebx, &ecx, &edx);
>>> -     if (eax < 0x8000001f)
>>> -             return;
>>> -
>>> -     /*
>>> -      * Check for the SME/SEV feature:
>>> -      *   CPUID Fn8000_001F[EAX]
>>> -      *   - Bit 0 - Secure Memory Encryption support
>>> -      *   - Bit 1 - Secure Encrypted Virtualization support
>>> -      *   CPUID Fn8000_001F[EBX]
>>> -      *   - Bits 5:0 - Pagetable bit position used to indicate encryption
>>> -      */
>>> -     eax = 0x8000001f;
>>> -     ecx = 0;
>>> -     native_cpuid(&eax, &ebx, &ecx, &edx);
>>> -     /* Check whether SEV is supported */
>>> -     if (!(eax & BIT(1))) {
>>> +     sev_status = sev_get_status();
>>> +     if (!(sev_status & MSR_AMD64_SEV_ENABLED)) {
>>>                if (snp)
>>>                        error("SEV-SNP support indicated by CC blob, but not CPUID.");
>>
>> This ends up checking the CPUID path because if SEV isn't advertised in
>> CPUID the returned status value is 0. But it also checks the SEV_STATUS
>> MSR as well. So I think you can remove the SNP / SEV_STATUS check at the
>> end of this function (since that check is identical to this now) and just
>> update the message to indicate not CPUID or SEV status MSR.
>>
> 
> But that one checks for MSR_AMD64_SEV_SNP_ENABLED not
> MSR_AMD64_SEV_ENABLED. Does that matter at all?

Ugh, my bad, I misread that last check. Ignore my comment.

> 
>> The sme_me_mask should probably be cleared at this point before returning,
>> too. Or, alternately, in sev_get_status(), you can update the setting of
>> sme_me_mask to based on MSR_AMD64_SEV_ENABLED being set in the SEV_STATUS MSR.
>>
> 
> I'll go for the latter, seems cleaner not to touch it in that case.

Sounds good.

Thanks
Tom

> 
> --- a/arch/x86/boot/compressed/sev.c
> +++ b/arch/x86/boot/compressed/sev.c
> @@ -422,10 +422,12 @@ u64 sev_get_status(void)
>          if (!(eax & BIT(1)))
>                  return 0;
> 
> -       /* Set the SME mask if this is an SEV guest. */
> -       sme_me_mask = BIT_ULL(ebx & 0x3f);
> -
>          boot_rdmsr(MSR_AMD64_SEV, &m);
> +
> +       /* Set the SME mask if this is an SEV guest. */
> +       if (m.q & MSR_AMD64_SEV_ENABLED)
> +               sme_me_mask = BIT_ULL(ebx & 0x3f);
> +
>          return m.q;
>   }
> 
> 
>>>    /* Search for Confidential Computing blob in the EFI config table. */
>>> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
>>> index 86e1296e87f513b7..081c39b0e8d0d208 100644
>>> --- a/arch/x86/include/asm/sev.h
>>> +++ b/arch/x86/include/asm/sev.h
>>> @@ -207,6 +207,8 @@ bool snp_init(struct boot_params *bp);
>>>    void __init __noreturn snp_abort(void);
>>>    int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio);
>>>    void snp_accept_memory(phys_addr_t start, phys_addr_t end);
>>> +u64 snp_get_unsupported_features(u64 status);
>>> +u64 sev_get_status(void);
>>>    #else
>>>    static inline void sev_es_ist_enter(struct pt_regs *regs) { }
>>>    static inline void sev_es_ist_exit(void) { }
>>> @@ -232,6 +234,8 @@ static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *in
>>>    }
>>>
>>>    static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
>>> +static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
>>> +static inline u64 sev_get_status(void) { return 0; }
>>>    #endif
>>>
>>>    #endif
>>> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
>>> index abcd5703e9f3f980..1015ef883f5850a4 100644
>>> --- a/drivers/firmware/efi/libstub/x86-stub.c
>>> +++ b/drivers/firmware/efi/libstub/x86-stub.c
>>> @@ -15,6 +15,7 @@
>>>    #include <asm/setup.h>
>>>    #include <asm/desc.h>
>>>    #include <asm/boot.h>
>>> +#include <asm/sev.h>
>>>
>>>    #include "efistub.h"
>>>    #include "x86-stub.h"
>>> @@ -790,6 +791,19 @@ static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
>>>        return EFI_SUCCESS;
>>>    }
>>>
>>> +static bool have_unsupported_snp_features(void)
>>> +{
>>> +     u64 unsupported;
>>> +
>>> +     unsupported = snp_get_unsupported_features(sev_get_status());
>>
>> This will also set sme_me_mask, but I think that is ok, since on error
>> things will terminate, otherwise sev_enable() should update appropriately
>> later.
>>
> 
> OK
> 
>>> +     if (unsupported) {
>>> +             efi_err("Unsupported SEV-SNP features detected: 0x%llx\n",
>>> +                     unsupported);
>>> +             return true;
>>> +     }
>>> +     return false;
>>> +}
>>> +
>>>    static void __noreturn enter_kernel(unsigned long kernel_addr,
>>>                                    struct boot_params *boot_params)
>>>    {
>>> @@ -820,6 +834,9 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
>>>        if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
>>>                efi_exit(handle, EFI_INVALID_PARAMETER);
>>>
>>> +     if (have_unsupported_snp_features())
>>> +             efi_exit(handle, EFI_UNSUPPORTED);
>>> +
>>>        if (IS_ENABLED(CONFIG_EFI_DXE_MEM_ATTRIBUTES)) {
>>>                efi_dxe_table = get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID);
>>>                if (efi_dxe_table &&
  

Patch

diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c
index 09dc8c187b3cc752..9593bc80c9c6b89d 100644
--- a/arch/x86/boot/compressed/sev.c
+++ b/arch/x86/boot/compressed/sev.c
@@ -367,6 +367,11 @@  static void enforce_vmpl0(void)
  */
 #define SNP_FEATURES_PRESENT (0)
 
+u64 snp_get_unsupported_features(u64 status)
+{
+	return status & SNP_FEATURES_IMPL_REQ & ~SNP_FEATURES_PRESENT;
+}
+
 void snp_check_features(void)
 {
 	u64 unsupported;
@@ -380,7 +385,7 @@  void snp_check_features(void)
 	 * EXIT_INFO_2 of the GHCB protocol so that those features can be reported
 	 * as part of the guest boot failure.
 	 */
-	unsupported = sev_status & SNP_FEATURES_IMPL_REQ & ~SNP_FEATURES_PRESENT;
+	unsupported = snp_get_unsupported_features(sev_status);
 	if (unsupported) {
 		if (ghcb_version < 2 || (!boot_ghcb && !early_setup_ghcb()))
 			sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
@@ -390,10 +395,42 @@  void snp_check_features(void)
 	}
 }
 
-void sev_enable(struct boot_params *bp)
+u64 sev_get_status(void)
 {
 	unsigned int eax, ebx, ecx, edx;
 	struct msr m;
+
+	/* Check for the SME/SEV support leaf */
+	eax = 0x80000000;
+	ecx = 0;
+	native_cpuid(&eax, &ebx, &ecx, &edx);
+	if (eax < 0x8000001f)
+		return 0;
+
+	/*
+	 * Check for the SME/SEV feature:
+	 *   CPUID Fn8000_001F[EAX]
+	 *   - Bit 0 - Secure Memory Encryption support
+	 *   - Bit 1 - Secure Encrypted Virtualization support
+	 *   CPUID Fn8000_001F[EBX]
+	 *   - Bits 5:0 - Pagetable bit position used to indicate encryption
+	 */
+	eax = 0x8000001f;
+	ecx = 0;
+	native_cpuid(&eax, &ebx, &ecx, &edx);
+	/* Check whether SEV is supported */
+	if (!(eax & BIT(1)))
+		return 0;
+
+	/* Set the SME mask if this is an SEV guest. */
+	sme_me_mask = BIT_ULL(ebx & 0x3f);
+
+	boot_rdmsr(MSR_AMD64_SEV, &m);
+	return m.q;
+}
+
+void sev_enable(struct boot_params *bp)
+{
 	bool snp;
 
 	/*
@@ -410,37 +447,13 @@  void sev_enable(struct boot_params *bp)
 	 */
 	snp = snp_init(bp);
 
-	/* Check for the SME/SEV support leaf */
-	eax = 0x80000000;
-	ecx = 0;
-	native_cpuid(&eax, &ebx, &ecx, &edx);
-	if (eax < 0x8000001f)
-		return;
-
-	/*
-	 * Check for the SME/SEV feature:
-	 *   CPUID Fn8000_001F[EAX]
-	 *   - Bit 0 - Secure Memory Encryption support
-	 *   - Bit 1 - Secure Encrypted Virtualization support
-	 *   CPUID Fn8000_001F[EBX]
-	 *   - Bits 5:0 - Pagetable bit position used to indicate encryption
-	 */
-	eax = 0x8000001f;
-	ecx = 0;
-	native_cpuid(&eax, &ebx, &ecx, &edx);
-	/* Check whether SEV is supported */
-	if (!(eax & BIT(1))) {
+	sev_status = sev_get_status();
+	if (!(sev_status & MSR_AMD64_SEV_ENABLED)) {
 		if (snp)
 			error("SEV-SNP support indicated by CC blob, but not CPUID.");
 		return;
 	}
 
-	/* Set the SME mask if this is an SEV guest. */
-	boot_rdmsr(MSR_AMD64_SEV, &m);
-	sev_status = m.q;
-	if (!(sev_status & MSR_AMD64_SEV_ENABLED))
-		return;
-
 	/* Negotiate the GHCB protocol version. */
 	if (sev_status & MSR_AMD64_SEV_ES_ENABLED) {
 		if (!sev_es_negotiate_protocol())
@@ -460,8 +473,6 @@  void sev_enable(struct boot_params *bp)
 
 	if (snp && !(sev_status & MSR_AMD64_SEV_SNP_ENABLED))
 		error("SEV-SNP supported indicated by CC blob, but not SEV status MSR.");
-
-	sme_me_mask = BIT_ULL(ebx & 0x3f);
 }
 
 /* Search for Confidential Computing blob in the EFI config table. */
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 86e1296e87f513b7..081c39b0e8d0d208 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -207,6 +207,8 @@  bool snp_init(struct boot_params *bp);
 void __init __noreturn snp_abort(void);
 int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio);
 void snp_accept_memory(phys_addr_t start, phys_addr_t end);
+u64 snp_get_unsupported_features(u64 status);
+u64 sev_get_status(void);
 #else
 static inline void sev_es_ist_enter(struct pt_regs *regs) { }
 static inline void sev_es_ist_exit(void) { }
@@ -232,6 +234,8 @@  static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *in
 }
 
 static inline void snp_accept_memory(phys_addr_t start, phys_addr_t end) { }
+static inline u64 snp_get_unsupported_features(u64 status) { return 0; }
+static inline u64 sev_get_status(void) { return 0; }
 #endif
 
 #endif
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index abcd5703e9f3f980..1015ef883f5850a4 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -15,6 +15,7 @@ 
 #include <asm/setup.h>
 #include <asm/desc.h>
 #include <asm/boot.h>
+#include <asm/sev.h>
 
 #include "efistub.h"
 #include "x86-stub.h"
@@ -790,6 +791,19 @@  static efi_status_t exit_boot(struct boot_params *boot_params, void *handle)
 	return EFI_SUCCESS;
 }
 
+static bool have_unsupported_snp_features(void)
+{
+	u64 unsupported;
+
+	unsupported = snp_get_unsupported_features(sev_get_status());
+	if (unsupported) {
+		efi_err("Unsupported SEV-SNP features detected: 0x%llx\n",
+			unsupported);
+		return true;
+	}
+	return false;
+}
+
 static void __noreturn enter_kernel(unsigned long kernel_addr,
 				    struct boot_params *boot_params)
 {
@@ -820,6 +834,9 @@  void __noreturn efi_stub_entry(efi_handle_t handle,
 	if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
 		efi_exit(handle, EFI_INVALID_PARAMETER);
 
+	if (have_unsupported_snp_features())
+		efi_exit(handle, EFI_UNSUPPORTED);
+
 	if (IS_ENABLED(CONFIG_EFI_DXE_MEM_ATTRIBUTES)) {
 		efi_dxe_table = get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID);
 		if (efi_dxe_table &&