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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/09/2025 11:05 am, Gerald Elder-Vass wrote:
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index e12fa1a7ec04..e7e3dffa7ddc 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;
> +    EFI_STATUS status;
> +    uint8_t data = 0;
> +    UINTN size = sizeof(data);
> +    UINT32 attr = 0;
> +
> +    status = efi_rs->GetVariable((CHAR16 *)L"SecureBoot", &gv_uuid, &attr,
> +                                 &size, &data);

This turns out to be a MISRA R7.4 violation, complaining about casing a
string literal to a non-const pointer.

The real problem here is that the EFI spec.  GetVariable() ought to take
a const CHAR16 *, but doesn't.

We could fix this with:

diff --git a/xen/include/efi/efiapi.h b/xen/include/efi/efiapi.h
index a616d1238aa4..56775d553109 100644
--- a/xen/include/efi/efiapi.h
+++ b/xen/include/efi/efiapi.h
@@ -224,7 +224,7 @@ VOID
 typedef
 EFI_STATUS
 (EFIAPI *EFI_GET_VARIABLE) (
-    IN CHAR16                       *VariableName,
+    IN const CHAR16                 *VariableName,
     IN EFI_GUID                     *VendorGuid,
     OUT UINT32                      *Attributes OPTIONAL,
     IN OUT UINTN                    *DataSize,

but I fear this might get some objections.

I don't think we want to be deviating every use of GetVariable() for a
problem ultimately outside of our control.

Another option would be to have a wrapper for GetVariable() which does
the cast once, which lets us deviate in one place only.

Thoughts?

~Andrew