CC'ing the EFI maintainers.
On 06/05/2025 5:24 pm, Kevin Lampis wrote:
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index e39fbc3529..7c528cd5dd 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -870,6 +870,27 @@ static void __init pre_parse(const struct file *file)
> " last line will be ignored.\r\n");
> }
>
> +static void __init init_secure_boot_mode(void)
> +{
> + EFI_STATUS status;
> + EFI_GUID gv_uuid = EFI_GLOBAL_VARIABLE;
> + uint8_t data = 0;
> + UINTN size = sizeof(data);
> + UINT32 attr = 0;
Newline between variables and code please.
> + status = efi_rs->GetVariable((CHAR16 *)L"SecureBoot", &gv_uuid, &attr,
> + &size, &data);
> +
> + if ( status == EFI_NOT_FOUND ||
> + (status == EFI_SUCCESS &&
> + attr == (EFI_VARIABLE_BOOTSERVICE_ACCESS |
> EFI_VARIABLE_RUNTIME_ACCESS) &&
> + 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;
> +}
I'm not sure this logic does what you want when a weird answer comes
back from GetVariable().
Also, you can't have this be a common function, yet ...
> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index 7e1fce291d..b63d21f16c 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -40,6 +40,9 @@ void efi_rs_leave(struct efi_rs_state *state);
> unsigned int __read_mostly efi_num_ct;
> const EFI_CONFIGURATION_TABLE *__read_mostly efi_ct;
>
> +#if defined(CONFIG_X86) && !defined(CONFIG_PV_SHIM)
> +bool __ro_after_init efi_secure_boot;
> +#endif
... this variable exist only on x86.
Also, why adjust it for PV shim? None of this is even compiled for PV shim.
~Andrew
