* ACK: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-11-27 19:20 [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported Heinrich Schuchardt
@ 2020-11-27 19:28 ` Colin Ian King
2020-11-27 19:29 ` Ard Biesheuvel
2020-11-30 8:16 ` ivanhu
2020-12-26 10:16 ` Heinrich Schuchardt
2 siblings, 1 reply; 13+ messages in thread
From: Colin Ian King @ 2020-11-27 19:28 UTC (permalink / raw)
To: Heinrich Schuchardt, Ivan Hu, Ard Biesheuvel
Cc: linux-efi, linux-kernel, fwts-devel
On 27/11/2020 19:20, 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
>
Looks good to me. Thanks Heinrich.
The EFI driver needs to be also updated in the linux kernel - has that
fix been submitted or do you require the fwts team to do that?
Colin
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: ACK: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-11-27 19:28 ` ACK: " Colin Ian King
@ 2020-11-27 19:29 ` Ard Biesheuvel
2020-11-27 19:38 ` Colin Ian King
2020-11-27 19:39 ` Heinrich Schuchardt
0 siblings, 2 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-11-27 19:29 UTC (permalink / raw)
To: Colin Ian King
Cc: Heinrich Schuchardt, Ivan Hu, linux-efi,
Linux Kernel Mailing List, fwts-devel
On Fri, 27 Nov 2020 at 20:28, Colin Ian King <colin.king@canonical.com> wrote:
>
> On 27/11/2020 19:20, 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
> >
>
> Looks good to me. Thanks Heinrich.
>
> The EFI driver needs to be also updated in the linux kernel - has that
> fix been submitted or do you require the fwts team to do that?
>
This /is/ the EFI driver.
I'll take this as an acked-by but I'd like Ivan to chime in as well.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: ACK: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-11-27 19:29 ` Ard Biesheuvel
@ 2020-11-27 19:38 ` Colin Ian King
2020-11-27 19:39 ` Heinrich Schuchardt
1 sibling, 0 replies; 13+ messages in thread
From: Colin Ian King @ 2020-11-27 19:38 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Heinrich Schuchardt, Ivan Hu, linux-efi,
Linux Kernel Mailing List, fwts-devel
On 27/11/2020 19:29, Ard Biesheuvel wrote:
> On Fri, 27 Nov 2020 at 20:28, Colin Ian King <colin.king@canonical.com> wrote:
>>
>> On 27/11/2020 19:20, 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
>>>
>>
>> Looks good to me. Thanks Heinrich.
>>
>> The EFI driver needs to be also updated in the linux kernel - has that
>> fix been submitted or do you require the fwts team to do that?
Oops. It's been a lot week :-(
>>
>
> This /is/ the EFI driver.
>
> I'll take this as an acked-by but I'd like Ivan to chime in as well.
>
+1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: ACK: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-11-27 19:29 ` Ard Biesheuvel
2020-11-27 19:38 ` Colin Ian King
@ 2020-11-27 19:39 ` Heinrich Schuchardt
1 sibling, 0 replies; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-11-27 19:39 UTC (permalink / raw)
To: Ard Biesheuvel, Colin Ian King
Cc: Ivan Hu, linux-efi, Linux Kernel Mailing List, fwts-devel
On 11/27/20 8:29 PM, Ard Biesheuvel wrote:
> On Fri, 27 Nov 2020 at 20:28, Colin Ian King <colin.king@canonical.com> wrote:
>>
>> On 27/11/2020 19:20, 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.
>>>
<snip />
>> Looks good to me. Thanks Heinrich.
>>
>> The EFI driver needs to be also updated in the linux kernel - has that
>> fix been submitted or do you require the fwts team to do that?
>>
>
> This /is/ the EFI driver.
>
> I'll take this as an acked-by but I'd like Ivan to chime in as well.
>
Hello Ard, hello Colin,
I have tested the patch with Linux 5.8.9.
Somehow Linux managed to break the kernel for my board between Linux
5.8.9 and 5.8.18. It does not boot form iSCSI with a newer kernel.
You probably want to run a user program against your latest kernel. This
is what I used:
#include <stdio.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <sys/types.h>
#define EFI_RUNTIME_GET_SUPPORTED_MASK \
_IOR('p', 0x0C, unsigned int)
int main()
{
unsigned int i, flag;
int fd, ret;
unsigned int mask;
fd = open("/dev/efi_test", O_RDWR);
if (fd == -1) {
perror("open");
return 1;
}
ret = ioctl(fd, EFI_RUNTIME_GET_SUPPORTED_MASK, &mask);
if (ret == -1) {
perror("ioctl");
return 1;
}
printf("mask 0x%08x\n", mask);
flag = 1;
for (i = 0x80000000; i; i >>= 1) {
if (i & mask) {
printf("%4s 0x%08x\n", flag ? "=" : "|", i);
flag = 0;
}
}
close(fd);
return 0;
}
Best regards
Heinrich
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-11-27 19:20 [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported Heinrich Schuchardt
2020-11-27 19:28 ` ACK: " Colin Ian King
@ 2020-11-30 8:16 ` ivanhu
2020-11-30 9:17 ` Heinrich Schuchardt
2020-12-26 10:16 ` Heinrich Schuchardt
2 siblings, 1 reply; 13+ messages in thread
From: ivanhu @ 2020-11-30 8:16 UTC (permalink / raw)
To: Heinrich Schuchardt, Ard Biesheuvel
Cc: linux-efi, linux-kernel, Colin King, fwts-devel
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.
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-11-30 8:16 ` ivanhu
@ 2020-11-30 9:17 ` Heinrich Schuchardt
2020-11-30 10:38 ` ivanhu
0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-11-30 9:17 UTC (permalink / raw)
To: ivanhu, Ard Biesheuvel; +Cc: linux-efi, linux-kernel, Colin King, fwts-devel
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.
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
>>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-11-30 9:17 ` Heinrich Schuchardt
@ 2020-11-30 10:38 ` ivanhu
2020-12-02 11:38 ` Heinrich Schuchardt
0 siblings, 1 reply; 13+ messages in thread
From: ivanhu @ 2020-11-30 10:38 UTC (permalink / raw)
To: Heinrich Schuchardt, Ard Biesheuvel
Cc: linux-efi, linux-kernel, Colin King, fwts-devel
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
>>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-11-30 10:38 ` ivanhu
@ 2020-12-02 11:38 ` Heinrich Schuchardt
2020-12-03 1:20 ` ivanhu
0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-12-02 11:38 UTC (permalink / raw)
To: ivanhu, Ard Biesheuvel; +Cc: linux-efi, linux-kernel, Colin King, fwts-devel
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.
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
>>>>
>>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-12-02 11:38 ` Heinrich Schuchardt
@ 2020-12-03 1:20 ` ivanhu
2020-12-03 6:48 ` Heinrich Schuchardt
0 siblings, 1 reply; 13+ messages in thread
From: ivanhu @ 2020-12-03 1:20 UTC (permalink / raw)
To: Heinrich Schuchardt, Ard Biesheuvel
Cc: linux-efi, linux-kernel, Colin King, fwts-devel
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?
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
>>>>>
>>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-12-03 1:20 ` ivanhu
@ 2020-12-03 6:48 ` Heinrich Schuchardt
0 siblings, 0 replies; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-12-03 6:48 UTC (permalink / raw)
To: ivanhu, Ard Biesheuvel; +Cc: linux-efi, linux-kernel, Colin King, fwts-devel
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
>>>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-11-27 19:20 [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported Heinrich Schuchardt
2020-11-27 19:28 ` ACK: " Colin Ian King
2020-11-30 8:16 ` ivanhu
@ 2020-12-26 10:16 ` Heinrich Schuchardt
2020-12-29 13:01 ` Ard Biesheuvel
2 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2020-12-26 10:16 UTC (permalink / raw)
To: Ivan Hu, Ard Biesheuvel; +Cc: linux-efi, linux-kernel, Colin King, fwts-devel
On 11/27/20 8:20 PM, 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>
Hello Ard,
the patch has now been admitted to Linus' branch.
Could we, please, have this patch applied to the 5.10 long term release,
too.
Best regards
Heinrich
> ---
> 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
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/1] efi/efi_test: read RuntimeServicesSupported
2020-12-26 10:16 ` Heinrich Schuchardt
@ 2020-12-29 13:01 ` Ard Biesheuvel
0 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-12-29 13:01 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Ivan Hu, linux-efi, Linux Kernel Mailing List, Colin King,
fwts-devel
On Sat, 26 Dec 2020 at 11:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 11/27/20 8:20 PM, 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>
>
> Hello Ard,
>
> the patch has now been admitted to Linus' branch.
>
> Could we, please, have this patch applied to the 5.10 long term release,
> too.
>
If you think this patch needs to go to -stable, please send an email
to stable@vger.kernel.org containing the commit title and SHA1, and a
short motivation why this patch needs to be backported.
If the stable maintainers are willing to take it, I won't object to it.
Thanks,
Ard.
^ permalink raw reply [flat|nested] 13+ messages in thread