[PATCH 2/3] uefi: fix fwts_uefi_rt_support_status_get()
ivanhu
ivan.hu at canonical.com
Mon Dec 28 07:09:58 UTC 2020
On 12/28/20 4:52 AM, Heinrich Schuchardt wrote:
> The UEFI 2.8 Errata A specification has clarified that
> RuntimeServicesSupported is not a UEFI variable but a value in the
> EFI_RT_PROPERTIES_TABLE configuration table. Since Linux 5.11 the value can
> be retrieved via an IOCTL call.
>
> Replace the code trying to read the non-existent RuntimeServicesSupported
> variable by the IOCTL call.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
> src/lib/include/fwts_uefi.h | 4 +-
> src/lib/src/fwts_uefi.c | 47 ++++++------------------
> src/uefi/uefirtmisc/uefirtmisc.c | 12 +-----
> src/uefi/uefirttime/uefirttime.c | 13 +------
> src/uefi/uefirtvariable/uefirtvariable.c | 13 +------
> 5 files changed, 18 insertions(+), 71 deletions(-)
>
> diff --git a/src/lib/include/fwts_uefi.h b/src/lib/include/fwts_uefi.h
> index 36cd5824..5e69ec62 100644
> --- a/src/lib/include/fwts_uefi.h
> +++ b/src/lib/include/fwts_uefi.h
> @@ -137,6 +137,8 @@ enum {
> #define EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES 0x1000
> #define EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO 0x2000
>
> +#define EFI_RT_SUPPORTED_ALL 0x3fff
> +
> #define EFI_CERT_SHA256_GUID \
> { 0xc1c41626, 0x504c, 0x4092, { 0xac, 0xa9, 0x41, 0xf9, 0x36, 0x93, 0x43, 0x28 }}
>
> @@ -680,7 +682,7 @@ char *fwts_uefi_attribute_info(uint32_t attr);
>
> bool fwts_uefi_efivars_iface_exist(void);
>
> -void fwts_uefi_rt_support_status_get(int fd, bool *getvar_supported, uint32_t *var_rtsupported);
> +void fwts_uefi_rt_support_status_get(int fd, uint32_t *var_rtsupported);
>
> PRAGMA_POP
>
> diff --git a/src/lib/src/fwts_uefi.c b/src/lib/src/fwts_uefi.c
> index 0fb6b799..83799a98 100644
> --- a/src/lib/src/fwts_uefi.c
> +++ b/src/lib/src/fwts_uefi.c
> @@ -541,45 +541,22 @@ bool fwts_uefi_efivars_iface_exist(void)
>
> /*
> * fwts_uefi_rt_support_status_get()
> - * get the status of runtime service support and the value of
> - * the RuntimeServicesSupported variable
> + * get the status of runtime service support.
> + *
> + * Since UEFI 2.8 Errata A the EFI_RT_PROPERTIES_TABLE configuration table
> + * can be used to indicate which UEFI runtime services are not implemented
> + * via the bitmask RuntimeServicesSupported. If the table is not present,
> + * all runtime services are to be considered available. Since Linux 5.11
> + * this bitmask can be read via an IOCTL call. Before Linux 5.11 the value
> + * cannot be determined.
> */
> -void fwts_uefi_rt_support_status_get(int fd, bool *getvar_supported, uint32_t *var_rtsupported)
> +void fwts_uefi_rt_support_status_get(int fd, uint32_t *var_rtsupported)
> {
> long ioret;
> - struct efi_getvariable getvariable;
> - uint64_t status = ~0ULL;
> - uint8_t data[512];
> - uint64_t getdatasize = sizeof(data);
> - *var_rtsupported = 0xFFFF;
> -
> - uint32_t attributes = FWTS_UEFI_VAR_NON_VOLATILE |
> - FWTS_UEFI_VAR_BOOTSERVICE_ACCESS |
> - FWTS_UEFI_VAR_RUNTIME_ACCESS;
> - static uint16_t varname[] = {'R', 'u', 'n', 't', 'i', 'm', 'e', 'S', 'e',
> - 'r', 'v', 'i', 'c', 'e', 's', 'S', 'u', 'p',
> - 'p', 'o', 'r', 't', 'e', 'd', '\0'};
> - EFI_GUID global_var_guid = EFI_GLOBAL_VARIABLE;
> - getvariable.VariableName = varname;
> - getvariable.VendorGuid = &global_var_guid;
> - getvariable.Attributes = &attributes;
> - getvariable.DataSize = &getdatasize;
> - getvariable.Data = data;
> - getvariable.status = &status;
> -
> - ioret = ioctl(fd, EFI_RUNTIME_GET_VARIABLE, &getvariable);
> - if (ioret == -1) {
> - if (status == EFI_NOT_FOUND) {
> - *getvar_supported = true;
> - } else {
> - *getvar_supported = false;
> - }
> - return;
> - }
>
> - *getvar_supported = true;
> - *var_rtsupported = data[0] | data[1] << 8;
> + ioret = ioctl(fd, EFI_RUNTIME_GET_SUPPORTED_MASK, var_rtsupported);
> + if (ioret == -1)
> + *var_rtsupported = EFI_RT_SUPPORTED_ALL;
Hi Heinrich,
Thanks for the patches.
We should check the RuntimeServicesSupported supported or not and skip
the tests with some info messages, not just assume all supported. Or we
will get the false results. Such as, for those not support
RuntimeServicesSupported machines, they will be tested and get pass results.
Cheers,
Ivan
>
> return;
> -
> }
> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
> index a2dc6967..4b04f78a 100644
> --- a/src/uefi/uefirtmisc/uefirtmisc.c
> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
> @@ -245,22 +245,12 @@ static int uefirtmisc_test4(fwts_framework *fw)
> {
> long ioret;
> uint64_t status;
> - bool getvar_supported;
> uint32_t var_runtimeservicessupported;
>
> struct efi_getnexthighmonotoniccount getnexthighmonotoniccount;
> uint32_t highcount;
>
> - fwts_uefi_rt_support_status_get(fd, &getvar_supported,
> - &var_runtimeservicessupported);
> - if (!getvar_supported || (var_runtimeservicessupported == 0xFFFF)) {
> - fwts_skipped(fw, "Cannot get the RuntimeServicesSupported "
> - "variable, maybe the runtime service "
> - "GetVariable is not supported or "
> - "RuntimeServicesSupported not provided by "
> - "firmware.");
> - return FWTS_SKIP;
> - }
> + fwts_uefi_rt_support_status_get(fd, &var_runtimeservicessupported);
>
> getnexthighmonotoniccount.HighCount = &highcount;
> getnexthighmonotoniccount.status = &status;
> diff --git a/src/uefi/uefirttime/uefirttime.c b/src/uefi/uefirttime/uefirttime.c
> index e0aa7071..67365759 100644
> --- a/src/uefi/uefirttime/uefirttime.c
> +++ b/src/uefi/uefirttime/uefirttime.c
> @@ -1142,7 +1142,6 @@ static int uefirttime_test37(fwts_framework *fw)
> static int uefirttime_test38(fwts_framework *fw)
> {
> long ioret;
> - bool getvar_supported;
> uint32_t var_runtimeservicessupported;
>
> struct efi_settime settime;
> @@ -1155,17 +1154,7 @@ static int uefirttime_test38(fwts_framework *fw)
> EFI_TIME efi_time;
> EFI_TIME_CAPABILITIES efi_time_cap;
>
> - fwts_uefi_rt_support_status_get(fd, &getvar_supported,
> - &var_runtimeservicessupported);
> -
> - if (!getvar_supported || (var_runtimeservicessupported == 0xFFFF)) {
> - fwts_skipped(fw, "Cannot get the RuntimeServicesSupported "
> - "variable, maybe the runtime service "
> - "GetVariable is not supported or "
> - "RuntimeServicesSupported not provided by "
> - "firmware.");
> - return FWTS_SKIP;
> - }
> + fwts_uefi_rt_support_status_get(fd, &var_runtimeservicessupported);
>
> gettime.Capabilities = &efi_time_cap;
> gettime.Time = &efi_time;
> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
> index 713803dc..385c5405 100644
> --- a/src/uefi/uefirtvariable/uefirtvariable.c
> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
> @@ -2020,7 +2020,6 @@ static int uefirtvariable_test9(fwts_framework *fw)
> {
> long ioret;
>
> - bool getvar_supported;
> uint32_t var_runtimeservicessupported;
>
> struct efi_getvariable getvariable;
> @@ -2041,17 +2040,7 @@ static int uefirtvariable_test9(fwts_framework *fw)
> uint64_t getdatasize = sizeof(testdata);
> uint32_t attr;
>
> - fwts_uefi_rt_support_status_get(fd, &getvar_supported,
> - &var_runtimeservicessupported);
> -
> - if (!getvar_supported || (var_runtimeservicessupported == 0xFFFF)) {
> - fwts_skipped(fw, "Cannot get the RuntimeServicesSupported "
> - "variable, maybe the runtime service "
> - "GetVariable is not supported or "
> - "RuntimeServicesSupported not provided by "
> - "firmware.");
> - return FWTS_SKIP;
> - }
> + fwts_uefi_rt_support_status_get(fd, &var_runtimeservicessupported);
>
> setvariable.VariableName = variablenametest;
> setvariable.VendorGuid = >estguid1;
> --
> 2.29.2
>
More information about the fwts-devel
mailing list