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

[ivanhu <ivan.hu@canonical.com>]

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@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,
"
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@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
>>>
>