[PATCH 2/3] uefi: fix fwts_uefi_rt_support_status_get()
Heinrich Schuchardt
xypron.glpk at gmx.de
Mon Dec 28 07:37:17 UTC 2020
On 12/28/20 8:09 AM, ivanhu wrote:
>
>
> 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.
Thanks for reviewing.
fwts_uefi_rt_support_status_get() only determines the bits describing if
a runtime service should be implemented. In uefirtmisc.c, uefirttime.c,
uefirtvariable.c a message is written if a test is skipped because the
the bit is not set or the if the bit is set but the runtime service
returns EFI_UNSUPPORTED. You can see such a SKIPPED message at
https://github.com/U-Boot-EFI/u-boot-fwts-results/blob/master/results-2020-12-27.txt#L158
So it is unclear to me what you are missing.
Best regards
Heinrich
>
> 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