Re: [PATCH v4 4/4] efi: Do not use command line if secure boot is enabled.

["Roger Pau Monné" <roger.pau@citrix.com>]

On Mon, Sep 14, 2020 at 07:50:13AM -0400, Trammell Hudson wrote:
> If secure boot is enabled, the Xen command line arguments are ignored.
> If a unified Xen image is used, then the bundled configuration, dom0
> kernel, and initrd are prefered over the ones listed in the config file.

I understand that you must ignore the cfg option when using the
bundled image, but is there then an alternative way for passing the
basevideo and mapbs parameters?

Or there's simply no way of doing so when using secure boot with a
bundled image?

> Unlike the shim based verification, the PE signature on a unified image
> covers the all of the Xen+config+kernel+initrd modules linked into the

Extra 'the'.

> unified image. This also ensures that properly configured platforms
> will measure the entire runtime into the TPM for unsealing secrets or
> remote attestation.
> 
> Signed-off-by: Trammell Hudson <hudson@trmm.net>
> ---
>  xen/common/efi/boot.c | 44 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index 4b1fbc9643..e65c1f1a09 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -949,6 +949,39 @@ static void __init setup_efi_pci(void)
>      efi_bs->FreePool(handles);
>  }
>  
> +/*
> + * Logic should remain sync'ed with linux/arch/x86/xen/efi.c
> + * Secure Boot is enabled iff 'SecureBoot' is set and the system is
> + * not in Setup Mode.
> + */
> +static bool __init efi_secure_boot(void)
> +{
> +    static const __initconst EFI_GUID global_guid = EFI_GLOBAL_VARIABLE;
> +    uint8_t secboot, setupmode;
> +    UINTN secboot_size = sizeof(secboot);
> +    UINTN setupmode_size = sizeof(setupmode);
> +    EFI_STATUS rc;
> +
> +    rc = efi_rs->GetVariable(L"SecureBoot", (EFI_GUID *)&global_guid,
> +                             NULL, &secboot_size, &secboot);
> +    if ( rc != EFI_SUCCESS )
> +        return false;
> +
> +    rc = efi_rs->GetVariable(L"SetupMode", (EFI_GUID *)&global_guid,
> +                             NULL, &setupmode_size, &setupmode);
> +    if ( rc != EFI_SUCCESS )
> +        return false;
> +
> +    if ( secboot > 1)

Nit: missing space before closing parentheses.

> +    {
> +        PrintStr(L"Invalid SecureBoot variable=0x");
> +        DisplayUint(secboot, 2);

Maybe better to use secboot_size * 2 here since you already have the
size of the variable anyway?

Thanks, Roger.