[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 = &gtestguid1;
>> --
>> 2.29.2
>>




More information about the fwts-devel mailing list