Re: [PATCH v4 1/2] efi: Add a function to check if Secure Boot mode is enabled

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Sep 05, 2025 at 12:10:17PM +0000, Gerald Elder-Vass wrote:
> From: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> 
> Also cache it to avoid needing to repeatedly ask the firmware.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> Signed-off-by: Gerald Elder-Vass <gerald.elder-vass@xxxxxxxxx>

Acked-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>

> ---
> CC: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> CC: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> 
> v4:
> - Fix MISRA warning regarding SecureBoot string
> v3:
> - Fix build on ARM
> ---
>  xen/common/efi/boot.c    | 24 ++++++++++++++++++++++++
>  xen/common/efi/runtime.c |  1 +
>  xen/include/xen/efi.h    |  2 ++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index e12fa1a7ec04..ccbfc401f7ba 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -901,6 +901,28 @@ static void __init pre_parse(const struct file *file)
>                     " last line will be ignored.\r\n");
>  }
>  
> +static void __init init_secure_boot_mode(void)
> +{
> +    static EFI_GUID __initdata gv_uuid = EFI_GLOBAL_VARIABLE;
> +    static CHAR16 __initdata str_SecureBoot[] = L"SecureBoot";
> +    EFI_STATUS status;
> +    uint8_t data = 0;
> +    UINTN size = sizeof(data);
> +    UINT32 attr = 0;
> +
> +    status = efi_rs->GetVariable(str_SecureBoot, &gv_uuid, &attr, &size, 
> &data);
> +
> +    if ( status == EFI_NOT_FOUND ||
> +         (status == EFI_SUCCESS &&
> +          attr == (EFI_VARIABLE_BOOTSERVICE_ACCESS | 
> EFI_VARIABLE_RUNTIME_ACCESS) &&

FYI, I was unsure if checking this is a good idea, but UEFI spec does
define exactly those attributes for the "SecureBoot" variable, so it
sounds okay.

> +          size == 1 && data == 0) )
> +        /* Platform does not support Secure Boot or it's disabled. */
> +        efi_secure_boot = false;
> +    else
> +        /* Everything else play it safe and assume enabled. */
> +        efi_secure_boot = true;
> +}
> +
>  static void __init efi_init(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
> *SystemTable)
>  {
>      efi_ih = ImageHandle;
> @@ -915,6 +937,8 @@ static void __init efi_init(EFI_HANDLE ImageHandle, 
> EFI_SYSTEM_TABLE *SystemTabl
>  
>      StdOut = SystemTable->ConOut;
>      StdErr = SystemTable->StdErr ?: StdOut;
> +
> +    init_secure_boot_mode();
>  }
>  
>  static void __init efi_console_set_mode(void)
> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index 42386c6bde42..30d649ca5c1b 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -41,6 +41,7 @@ void efi_rs_leave(struct efi_rs_state *state);
>  unsigned int __read_mostly efi_num_ct;
>  const EFI_CONFIGURATION_TABLE *__read_mostly efi_ct;
>  
> +bool __ro_after_init efi_secure_boot;
>  unsigned int __read_mostly efi_version;
>  unsigned int __read_mostly efi_fw_revision;
>  const CHAR16 *__read_mostly efi_fw_vendor;
> diff --git a/xen/include/xen/efi.h b/xen/include/xen/efi.h
> index 623ed2ccdf31..723cb8085270 100644
> --- a/xen/include/xen/efi.h
> +++ b/xen/include/xen/efi.h
> @@ -36,6 +36,8 @@ static inline bool efi_enabled(unsigned int feature)
>  }
>  #endif
>  
> +extern bool efi_secure_boot;
> +
>  void efi_init_memory(void);
>  bool efi_boot_mem_unused(unsigned long *start, unsigned long *end);
>  bool efi_rs_using_pgtables(void);
> -- 
> 2.47.3
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature