[PATCH 1/1] efi/efi_test: read RuntimeServicesSupported

ivanhu ivan.hu at canonical.com
Thu Dec 3 08:56:01 UTC 2020


On 12/3/20 2:48 PM, Heinrich Schuchardt wrote:
> On 12/3/20 2:20 AM, ivanhu wrote:
>>
>>
>> On 12/2/20 7:38 PM, Heinrich Schuchardt wrote:
>>> On 11/30/20 11:38 AM, ivanhu wrote:
>>>>
>>>>
>>>> On 11/30/20 5:17 PM, Heinrich Schuchardt wrote:
>>>>> On 11/30/20 9:16 AM, ivanhu wrote:
>>>>>> Hi Heinrich,
>>>>>>
>>>>>> Thanks for the patch.
>>>>>> It looks good to me, but I noticed that the
>>>>>> runtime_supported_mask was
>>>>>> introduced after 5.7-rc1.
>>>>>> Maybe we should add the kernel version checking for the old kernels.
>>>>>
>>>>> This is a kernel patch. Why should we check the kernel version in the
>>>>> kernel code?
>>>>>
>>>>> As patches may be back-ported we should not make any assumptions
>>>>> in fwts
>>>>> based on the kernel version. If the ioctl() call fails with errno =
>>>>> ENOTTY, we know that the kernel does not implement the ioctl call
>>>>> and we
>>>>> have to assume that all runtime services are available.
>>>>
>>>> Sounds good to me,
>>>> Acked-by: Ivan Hu <ivan.hu at canonical.com>
>>>>
>>>> And I will replace the reading RuntimeServicesSupported efi
>>>> variable by
>>>> using efi_test in fwts RuntimeServicesSupported tests.
>>>>
>>>> FWTS will still test those Unsupported Runtime services to check if it
>>>> returns EFI_UNSUPPORTED correctly.
>>>> Is that could solve your problem?
>>>> If I remember correctly, the problem from you is not to test those
>>>> marked Unsupported Runtime services. But from the Spec. 8.1 Runtime
>>>> Services Rules and Restrictions,
>>>
>>> The problem I reported was that it is impossible to test UEFI runtime
>>> services on U-Boot because FWTS tries to read the non-existent
>>> RuntimeServicesSupported UEFI variable and mistakenly assumes that if
>>> the variable does not exist none of the runtime services is
>>> implemented.
>>
>> Could you provide the result log for me to check?
>
> https://github.com/U-Boot-EFI/u-boot-fwts-results/blob/master/fwts_20_11_fails.txt
>

>From the result log, they all skip from "Cannot open EFI test driver.
Aborted." which seems fail on open the /dev/efi_test, not related to the
RuntimeServicesSupported.

I also tested with my X86 machine which hasn't supported the
RuntimeServicesSupported variable as well, it won't meet the issue.

FWTS currently will run all the UEFI tests and try to get
RuntimeServicesSupported for testing in some runtime services support
test, such as,

Test 36 of 36: Test UEFI RT time services supported status.
SKIPPED: Test 36, Cannot get the RuntimeServicesSupported variable,
maybe the
runtime service GetVariable is not supported or RuntimeServicesSupported not
provided by firmware.


Cheers,

Ivan

>
> is the results log from FWTS 20.11.
>
> https://github.com/U-Boot-EFI/u-boot-fwts-results/blob/master/results-2020-10-31.txt
>
>
> is the results log from a FWTS built from this branch:
> https://github.com/xypron/fwts/commits/bugfixes
>
> Best regards
>
> Heinrich
>
>>
>> Ivan
>>>
>>> The correct thing to do in FWTS is:
>>>
>>> * read RuntimeServicesSupported via the ioctl
>>> * if the ioctl fails assume that all runtime services
>>>    are implemented
>>> * if the ioctl fails with errno != ENOTTY write an error message
>>> * for each runtime service marked as not supported
>>>    check that it returns EFI_UNSUPPORTED
>>> * for each service marked as supported
>>>    check that it works correctly
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>>> "
>>>> Note that this is merely a hint to the OS, which it is free to ignore,
>>>> and so the platform is still required to provide callable
>>>> implementations of unsupported runtime services that simply return
>>>> EFI_UNSUPPORTED.
>>>> "
>>>>
>>>> Cheers,
>>>> Ivan
>>>>>
>>>>> Best regards
>>>>>
>>>>> Heinrich
>>>>>
>>>>>>
>>>>>> Cheers,
>>>>>> Ivan
>>>>>>
>>>>>> On 11/28/20 3:20 AM, Heinrich Schuchardt wrote:
>>>>>>> Since the UEFI 2.8A specification the UEFI enabled firmware
>>>>>>> provides a
>>>>>>> configuration table EFI_RT_PROPERTIES_TABLE which indicates which
>>>>>>> runtime
>>>>>>> services are enabled. The EFI stub reads this table and saves the
>>>>>>> value of
>>>>>>> the field RuntimeServicesSupported internally.
>>>>>>>
>>>>>>> The Firmware Test Suite requires the value to determine if UEFI
>>>>>>> runtime
>>>>>>> services are correctly implemented.
>>>>>>>
>>>>>>> With this patch an IOCTL call is provided to read the value of the
>>>>>>> field
>>>>>>> RuntimeServicesSupported, e.g.
>>>>>>>
>>>>>>>        #define EFI_RUNTIME_GET_SUPPORTED_MASK \
>>>>>>>                _IOR('p', 0x0C, unsigned int)
>>>>>>>        unsigned int mask;
>>>>>>>        fd = open("/dev/efi_test", O_RDWR);
>>>>>>>        ret = ioctl(fd, EFI_RUNTIME_GET_SUPPORTED_MASK, &mask);
>>>>>>>
>>>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>>>>> ---
>>>>>>>     drivers/firmware/efi/test/efi_test.c | 16 ++++++++++++++++
>>>>>>>     drivers/firmware/efi/test/efi_test.h |  3 +++
>>>>>>>     2 files changed, 19 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/firmware/efi/test/efi_test.c
>>>>>>> b/drivers/firmware/efi/test/efi_test.c
>>>>>>> index ddf9eae396fe..47d67bb0a516 100644
>>>>>>> --- a/drivers/firmware/efi/test/efi_test.c
>>>>>>> +++ b/drivers/firmware/efi/test/efi_test.c
>>>>>>> @@ -663,6 +663,19 @@ static long
>>>>>>> efi_runtime_query_capsulecaps(unsigned long arg)
>>>>>>>         return rv;
>>>>>>>     }
>>>>>>>
>>>>>>> +static long efi_runtime_get_supported_mask(unsigned long arg)
>>>>>>> +{
>>>>>>> +    unsigned int __user *supported_mask;
>>>>>>> +    int rv = 0;
>>>>>>> +
>>>>>>> +    supported_mask = (unsigned int *)arg;
>>>>>>> +
>>>>>>> +    if (put_user(efi.runtime_supported_mask, supported_mask))
>>>>>>> +        rv = -EFAULT;
>>>>>>> +
>>>>>>> +    return rv;
>>>>>>> +}
>>>>>>> +
>>>>>>>     static long efi_test_ioctl(struct file *file, unsigned int cmd,
>>>>>>>                                 unsigned long arg)
>>>>>>>     {
>>>>>>> @@ -699,6 +712,9 @@ static long efi_test_ioctl(struct file *file,
>>>>>>> unsigned int cmd,
>>>>>>>
>>>>>>>         case EFI_RUNTIME_RESET_SYSTEM:
>>>>>>>             return efi_runtime_reset_system(arg);
>>>>>>> +
>>>>>>> +    case EFI_RUNTIME_GET_SUPPORTED_MASK:
>>>>>>> +        return efi_runtime_get_supported_mask(arg);
>>>>>>>         }
>>>>>>>
>>>>>>>         return -ENOTTY;
>>>>>>> diff --git a/drivers/firmware/efi/test/efi_test.h
>>>>>>> b/drivers/firmware/efi/test/efi_test.h
>>>>>>> index f2446aa1c2e3..117349e57993 100644
>>>>>>> --- a/drivers/firmware/efi/test/efi_test.h
>>>>>>> +++ b/drivers/firmware/efi/test/efi_test.h
>>>>>>> @@ -118,4 +118,7 @@ struct efi_resetsystem {
>>>>>>>     #define EFI_RUNTIME_RESET_SYSTEM \
>>>>>>>         _IOW('p', 0x0B, struct efi_resetsystem)
>>>>>>>
>>>>>>> +#define EFI_RUNTIME_GET_SUPPORTED_MASK \
>>>>>>> +    _IOR('p', 0x0C, unsigned int)
>>>>>>> +
>>>>>>>     #endif /* _DRIVERS_FIRMWARE_EFI_TEST_H_ */
>>>>>>> -- 
>>>>>>> 2.29.2
>>>>>>>
>>>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/fwts-devel/attachments/20201203/b9de4528/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: results.log
Type: text/x-log
Size: 24527 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/fwts-devel/attachments/20201203/b9de4528/attachment-0001.bin>


More information about the fwts-devel mailing list