Messages in this thread |  | | Subject | Re: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported | From | Heinrich Schuchardt <> | Date | Thu, 3 Dec 2020 07:48:57 +0100 |
| |
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@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
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@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 >>>>>> >>>> >>
|  |