Re: [PATCH] efi: add efi_test driver for exporting UEFI runtime service interfaces

[ivanhu <ivan.hu-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>]

Hi Joey,

Thanks for your comments.

On 2016年07月22日 15:26, joeyli wrote:
> Hi Ivan,
>
> On Fri, Jul 15, 2016 at 03:58:18PM +0800, Ivan Hu wrote:
>> This driver is used by the Firmware Test Suite (FWTS) for testing the UEFI
>> runtime interfaces readiness of the firmware.
>>
>> This driver exports UEFI runtime service interfaces into userspace,
>> which allows to use and test UEFI runtime services provided by the
>> firmware.
>>
>> This driver uses the efi.<service> function pointers directly instead of
>> going through the efivar API to allow for direct testing of the UEFI
>> runtime service interfaces provided by the firmware.
>>
>> Details for FWTS are available from,
>> <https://wiki.ubuntu.com/FirmwareTestSuite>
>>
>> Signed-off-by: Ivan Hu <ivan.hu-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>> ---
>>   MAINTAINERS                              |   6 +
>>   drivers/firmware/efi/Kconfig             |  15 +
>>   drivers/firmware/efi/Makefile            |   1 +
>>   drivers/firmware/efi/efi_test/Makefile   |   1 +
>>   drivers/firmware/efi/efi_test/efi_test.c | 713 +++++++++++++++++++++++++++++++
>>   drivers/firmware/efi/efi_test/efi_test.h | 110 +++++
>>   6 files changed, 846 insertions(+)
>>   create mode 100644 drivers/firmware/efi/efi_test/Makefile
>>   create mode 100644 drivers/firmware/efi/efi_test/efi_test.c
>>   create mode 100644 drivers/firmware/efi/efi_test/efi_test.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1209323..1f888cc 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -4471,6 +4471,12 @@ M:	Peter Jones <pjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>   S:	Maintained
>>   F:	drivers/video/fbdev/efifb.c
>>
>> +EFI TEST DRIVER
>> +L:	linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> +M:      Ivan Hu <ivan.hu-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>> +S:      Maintained
>> +F:      drivers/firmware/efi/efi_test/
>> +
>>   EFS FILESYSTEM
>>   W:	http://aeschi.ch.eu.org/efs/
>>   S:	Orphan
>> diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
>> index 6394152..1cc02bd 100644
>> --- a/drivers/firmware/efi/Kconfig
>> +++ b/drivers/firmware/efi/Kconfig
>> @@ -112,6 +112,21 @@ config EFI_CAPSULE_LOADER
>>
>>   	  Most users should say N.
>>
>> +config EFI_TEST
>> +	tristate "EFI Runtime Services Support"
>
> Because this is a driver for debugging tool, I suggest that it doesn't need to
> allow user to build-in to kernel.

Right now, fwts is a defacto standard test suite and efi_runtime driver 
is packed as dkms package. We would like to make it as part of the 
kernel which removes a lot of dkms hassles, such as dkms can not be used 
when secureboot is enabled.

And you are right, this driver is for fwts test and that's why we set 
kernel build as "default n".

>
> Please considering to put this driver to tools/testing folder. Or you have
> a reason to put this testing driver to drivers/firmware/efi ?

I think drivers/firmware/efi is right place since it is a driver which 
exports efi.<service> functions.

>
>> +	depends on EFI
>> +	default n
>> +	help
>> +	  Say Y here to enable the runtime services support via /dev/efi_test.
> 								^^^^^^^^^^^^^^
>
> I have tried this testing driver with the master branch of fwts git. But I
> found that fwts tries to load efi_runtime kernel module and tries to use
> /dev/efi_runtime.
>
> I cloned fwts code from "http://kernel.ubuntu.com/git/hwe/fwts.git" that it
> is written on FirmwareTestSuite wiki page. And, I found there have a
> efi_runtime driver in fwts/efi_runtime folder.
>
> Did I miss anything for the module name doesn't match?

A patch had been sent out and under reviewing for the developer.
https://lists.ubuntu.com/archives/fwts-devel/2016-July/008168.html

I believe it will be on the branch very soon and will be included in the 
fwts version V16.07.00 next week.

Cheers,
Ivan
>
>
> Thanks a lot!
> Joey Lee
>