[U-Boot] [PATCH 0/2] sandbox: enable UEFI unit tests

[U-Boot] [PATCH 0/2] sandbox: enable UEFI unit tests

[Heinrich Schuchardt]

This patch series enables the UEFI unit tests on the sandbox.

The first patch add the missing compatible property in the device trees
which is required by on of the unit tests.

The real time clock unit test will fail if the sandbox is openend without
passing a device tree (parameter -D).

A separate series will enable building UEFI binaries for the sandbox and
enable the corresponding UEFI unit tests.

Heinrich Schuchardt (2):
  sandbox: add missing compatible property in device tree
  configs: sandbox: enable CONFIG_CMD_BOOTEFI_SELFTEST

 arch/sandbox/dts/sandbox.dts       | 1 +
 arch/sandbox/dts/sandbox64.dts     | 1 +
 configs/sandbox64_defconfig        | 1 +
 configs/sandbox_defconfig          | 1 +
 configs/sandbox_flattree_defconfig | 1 +
 configs/sandbox_spl_defconfig      | 1 +
 6 files changed, 6 insertions(+)

--
2.24.0.rc1

[U-Boot] [PATCH 1/2] sandbox: add missing compatible property in device tree

[Heinrich Schuchardt]

In the device tree UEFI unit test the compatible property of the device is
read.

Provide the missing property.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 arch/sandbox/dts/sandbox.dts   | 1 +
 arch/sandbox/dts/sandbox64.dts | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
index f1637c80f6..4dd82f6a32 100644
--- a/arch/sandbox/dts/sandbox.dts
+++ b/arch/sandbox/dts/sandbox.dts
@@ -6,6 +6,7 @@
 	#address-cells = <1>;
 	#size-cells = <1>;
 	model = "sandbox";
+	compatible = "sandbox";

 	aliases {
 		i2c0 = &i2c_0;
diff --git a/arch/sandbox/dts/sandbox64.dts b/arch/sandbox/dts/sandbox64.dts
index 37a5539ff4..5c95cee9d7 100644
--- a/arch/sandbox/dts/sandbox64.dts
+++ b/arch/sandbox/dts/sandbox64.dts
@@ -6,6 +6,7 @@
 	#address-cells = <2>;
 	#size-cells = <2>;
 	model = "sandbox";
+	compatible = "sandbox";

 	aliases {
 		i2c0 = &i2c_0;
--
2.24.0.rc1

[U-Boot] [PATCH 2/2] configs: sandbox: enable CONFIG_CMD_BOOTEFI_SELFTEST

[Heinrich Schuchardt]

Activate UEFI unit tests on the sandbox.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 configs/sandbox64_defconfig        | 1 +
 configs/sandbox_defconfig          | 1 +
 configs/sandbox_flattree_defconfig | 1 +
 configs/sandbox_spl_defconfig      | 1 +
 4 files changed, 4 insertions(+)

diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 1fea683d89..329645d362 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -21,6 +21,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_BOOTEFI_SELFTEST=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 20ebc68997..b6d0d3f2fa 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -24,6 +24,7 @@ CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_BOOTEFI_SELFTEST=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index 898815fe53..02bbda071e 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -18,6 +18,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_BOOTEFI_SELFTEST=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index 409b8a38d5..9c24ea89ea 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -27,6 +27,7 @@ CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_BOOTEFI_SELFTEST=y
 # CONFIG_CMD_ELF is not set
 CONFIG_CMD_ASKENV=y
 CONFIG_CMD_GREPENV=y
--
2.24.0.rc1

[PATCH 0/2] sandbox: enable UEFI unit tests

[Heinrich Schuchardt]

On 11/9/19 9:39 AM, Heinrich Schuchardt wrote:
> This patch series enables the UEFI unit tests on the sandbox.
>
> The first patch add the missing compatible property in the device trees
> which is required by on of the unit tests.
>
> The real time clock unit test will fail if the sandbox is openend without
> passing a device tree (parameter -D).
>
> A separate series will enable building UEFI binaries for the sandbox and
> enable the corresponding UEFI unit tests.
>
> Heinrich Schuchardt (2):
>    sandbox: add missing compatible property in device tree
>    configs: sandbox: enable CONFIG_CMD_BOOTEFI_SELFTEST
>
>   arch/sandbox/dts/sandbox.dts       | 1 +
>   arch/sandbox/dts/sandbox64.dts     | 1 +
>   configs/sandbox64_defconfig        | 1 +
>   configs/sandbox_defconfig          | 1 +
>   configs/sandbox_flattree_defconfig | 1 +
>   configs/sandbox_spl_defconfig      | 1 +
>   6 files changed, 6 insertions(+)
>
> --
> 2.24.0.rc1
>

Hello Simon,

this patch series is currently assigned to you as maintainer of the
sandbox. I hope you can add it to your next tree.

Best regards

Heinrich

[PATCH 0/2] sandbox: enable UEFI unit tests

[Simon Glass]

Hi Heinrich,

On Sat, 7 Dec 2019 at 17:31, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 11/9/19 9:39 AM, Heinrich Schuchardt wrote:
> > This patch series enables the UEFI unit tests on the sandbox.
> >
> > The first patch add the missing compatible property in the device trees
> > which is required by on of the unit tests.
> >
> > The real time clock unit test will fail if the sandbox is openend without
> > passing a device tree (parameter -D).
> >
> > A separate series will enable building UEFI binaries for the sandbox and
> > enable the corresponding UEFI unit tests.
> >
> > Heinrich Schuchardt (2):
> >    sandbox: add missing compatible property in device tree
> >    configs: sandbox: enable CONFIG_CMD_BOOTEFI_SELFTEST
> >
> >   arch/sandbox/dts/sandbox.dts       | 1 +
> >   arch/sandbox/dts/sandbox64.dts     | 1 +
> >   configs/sandbox64_defconfig        | 1 +
> >   configs/sandbox_defconfig          | 1 +
> >   configs/sandbox_flattree_defconfig | 1 +
> >   configs/sandbox_spl_defconfig      | 1 +
> >   6 files changed, 6 insertions(+)
> >
> > --
> > 2.24.0.rc1
> >
>
> Hello Simon,
>
> this patch series is currently assigned to you as maintainer of the
> sandbox. I hope you can add it to your next tree.

Yes I'm planning to set up one up for the next release soon.

Regards,
Simon

[PATCH 2/2] configs: sandbox: enable CONFIG_CMD_BOOTEFI_SELFTEST

[Simon Glass]

Hi Heinrich,

On Sat, 9 Nov 2019 at 01:44, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Activate UEFI unit tests on the sandbox.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  configs/sandbox64_defconfig        | 1 +
>  configs/sandbox_defconfig          | 1 +
>  configs/sandbox_flattree_defconfig | 1 +
>  configs/sandbox_spl_defconfig      | 1 +
>  4 files changed, 4 insertions(+)

Unfortunately this slows down the testing too much, nearly doubling
the time in my tests.

I think the EFI console tests need to be modified to run in C instead
of all the drain_console() and p.timeout stuff. We need an effort to
speed up the tests, but certainly cannot make them any slower.

Regards,
Simon

[PATCH 1/2] sandbox: add missing compatible property in device tree

[Simon Glass]

On Sat, 9 Nov 2019 at 01:39, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> In the device tree UEFI unit test the compatible property of the device is
> read.
>
> Provide the missing property.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/sandbox/dts/sandbox.dts   | 1 +
>  arch/sandbox/dts/sandbox64.dts | 1 +
>  2 files changed, 2 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH 2/2] configs: sandbox: enable CONFIG_CMD_BOOTEFI_SELFTEST

[Heinrich Schuchardt]

On 12/10/19 1:58 PM, Simon Glass wrote:
> Hi Heinrich,
>
> On Sat, 9 Nov 2019 at 01:44, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> Activate UEFI unit tests on the sandbox.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   configs/sandbox64_defconfig        | 1 +
>>   configs/sandbox_defconfig          | 1 +
>>   configs/sandbox_flattree_defconfig | 1 +
>>   configs/sandbox_spl_defconfig      | 1 +
>>   4 files changed, 4 insertions(+)
>
> Unfortunately this slows down the testing too much, nearly doubling
> the time in my tests.
>
> I think the EFI console tests need to be modified to run in C instead
> of all the drain_console() and p.timeout stuff. We need an effort to
> speed up the tests, but certainly cannot make them any slower.

Hello Simon,

thanks for pointing at the excessive timeout.

In test_efi_selftest_text_input() and test_efi_selftest_text_input_ex()
we call drain_console() a lot indeed.

I am using the Python tests on real hardware. The text input and output
on the test systems uses the serial console. Whether you use C or Python
code to feed the serial adapter will not change the time needed to drain
the console.

We expect less then 50 characters of output per test step. At 9600 baud
draining 50 characters would require 52 ms.

So I would suggest that we add a parameter timeout to drain_console()
which defaults to 1000 ms as currently. But in the test we can set it to
50 ms. p.timeout could be changed likewise.

What are your thoughts?

Best regards

Heinrich

[PATCH 2/2] configs: sandbox: enable CONFIG_CMD_BOOTEFI_SELFTEST

[Simon Glass]

Hi Heinrich,

On Tue, 10 Dec 2019 at 11:17, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 12/10/19 1:58 PM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Sat, 9 Nov 2019 at 01:44, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> Activate UEFI unit tests on the sandbox.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >>   configs/sandbox64_defconfig        | 1 +
> >>   configs/sandbox_defconfig          | 1 +
> >>   configs/sandbox_flattree_defconfig | 1 +
> >>   configs/sandbox_spl_defconfig      | 1 +
> >>   4 files changed, 4 insertions(+)
> >
> > Unfortunately this slows down the testing too much, nearly doubling
> > the time in my tests.
> >
> > I think the EFI console tests need to be modified to run in C instead
> > of all the drain_console() and p.timeout stuff. We need an effort to
> > speed up the tests, but certainly cannot make them any slower.
>
> Hello Simon,
>
> thanks for pointing at the excessive timeout.
>
> In test_efi_selftest_text_input() and test_efi_selftest_text_input_ex()
> we call drain_console() a lot indeed.
>
> I am using the Python tests on real hardware. The text input and output
> on the test systems uses the serial console. Whether you use C or Python
> code to feed the serial adapter will not change the time needed to drain
> the console.
>
> We expect less then 50 characters of output per test step. At 9600 baud
> draining 50 characters would require 52 ms.
>
> So I would suggest that we add a parameter timeout to drain_console()
> which defaults to 1000 ms as currently. But in the test we can set it to
> 50 ms. p.timeout could be changed likewise.
>
> What are your thoughts?

I think you might be missing my point.

I think as a matter of policy we should require that test run mostly
on the device, with just the results being output. By using C I mean
to write the test in C instead of Python - just do the same steps.
E.g. do a lot of run_command() calls and check the results. This
should be much faster.

Regards,
Simon

[PATCH 2/2] configs: sandbox: enable CONFIG_CMD_BOOTEFI_SELFTEST

[Heinrich Schuchardt]

On 12/11/19 4:43 PM, Simon Glass wrote:
> Hi Heinrich,
>
> On Tue, 10 Dec 2019 at 11:17, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 12/10/19 1:58 PM, Simon Glass wrote:
>>> Hi Heinrich,
>>>
>>> On Sat, 9 Nov 2019 at 01:44, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>
>>>> Activate UEFI unit tests on the sandbox.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>> ---
>>>>    configs/sandbox64_defconfig        | 1 +
>>>>    configs/sandbox_defconfig          | 1 +
>>>>    configs/sandbox_flattree_defconfig | 1 +
>>>>    configs/sandbox_spl_defconfig      | 1 +
>>>>    4 files changed, 4 insertions(+)
>>>
>>> Unfortunately this slows down the testing too much, nearly doubling
>>> the time in my tests.
>>>
>>> I think the EFI console tests need to be modified to run in C instead
>>> of all the drain_console() and p.timeout stuff. We need an effort to
>>> speed up the tests, but certainly cannot make them any slower.
>>
>> Hello Simon,
>>
>> thanks for pointing at the excessive timeout.
>>
>> In test_efi_selftest_text_input() and test_efi_selftest_text_input_ex()
>> we call drain_console() a lot indeed.
>>
>> I am using the Python tests on real hardware. The text input and output
>> on the test systems uses the serial console. Whether you use C or Python
>> code to feed the serial adapter will not change the time needed to drain
>> the console.
>>
>> We expect less then 50 characters of output per test step. At 9600 baud
>> draining 50 characters would require 52 ms.
>>
>> So I would suggest that we add a parameter timeout to drain_console()
>> which defaults to 1000 ms as currently. But in the test we can set it to
>> 50 ms. p.timeout could be changed likewise.
>>
>> What are your thoughts?
>
> I think you might be missing my point.
>
> I think as a matter of policy we should require that test run mostly
> on the device, with just the results being output. By using C I mean
> to write the test in C instead of Python - just do the same steps.
> E.g. do a lot of run_command() calls and check the results. This
> should be much faster.
>

The current test checks if Unicode signs sent over the physical serial
interface are correctly interpreted in U-Boot. One reason for failure
could be that the UART driver uses bit 7 as a parity bit.

U-Boot's internal function run_command() cannot send electrical signals
inbound to the device's UART.

You may implement an additional test with a different scope. But that
should not replace the current test.

Best regards

Heinrich

[PATCH 2/2] configs: sandbox: enable CONFIG_CMD_BOOTEFI_SELFTEST

[Simon Glass]

Hi Heinrich,

On Wed, 11 Dec 2019 at 09:59, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 12/11/19 4:43 PM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Tue, 10 Dec 2019 at 11:17, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> On 12/10/19 1:58 PM, Simon Glass wrote:
> >>> Hi Heinrich,
> >>>
> >>> On Sat, 9 Nov 2019 at 01:44, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>>>
> >>>> Activate UEFI unit tests on the sandbox.
> >>>>
> >>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>>> ---
> >>>>    configs/sandbox64_defconfig        | 1 +
> >>>>    configs/sandbox_defconfig          | 1 +
> >>>>    configs/sandbox_flattree_defconfig | 1 +
> >>>>    configs/sandbox_spl_defconfig      | 1 +
> >>>>    4 files changed, 4 insertions(+)
> >>>
> >>> Unfortunately this slows down the testing too much, nearly doubling
> >>> the time in my tests.
> >>>
> >>> I think the EFI console tests need to be modified to run in C instead
> >>> of all the drain_console() and p.timeout stuff. We need an effort to
> >>> speed up the tests, but certainly cannot make them any slower.
> >>
> >> Hello Simon,
> >>
> >> thanks for pointing at the excessive timeout.
> >>
> >> In test_efi_selftest_text_input() and test_efi_selftest_text_input_ex()
> >> we call drain_console() a lot indeed.
> >>
> >> I am using the Python tests on real hardware. The text input and output
> >> on the test systems uses the serial console. Whether you use C or Python
> >> code to feed the serial adapter will not change the time needed to drain
> >> the console.
> >>
> >> We expect less then 50 characters of output per test step. At 9600 baud
> >> draining 50 characters would require 52 ms.
> >>
> >> So I would suggest that we add a parameter timeout to drain_console()
> >> which defaults to 1000 ms as currently. But in the test we can set it to
> >> 50 ms. p.timeout could be changed likewise.
> >>
> >> What are your thoughts?
> >
> > I think you might be missing my point.
> >
> > I think as a matter of policy we should require that test run mostly
> > on the device, with just the results being output. By using C I mean
> > to write the test in C instead of Python - just do the same steps.
> > E.g. do a lot of run_command() calls and check the results. This
> > should be much faster.
> >
>
> The current test checks if Unicode signs sent over the physical serial
> interface are correctly interpreted in U-Boot. One reason for failure
> could be that the UART driver uses bit 7 as a parity bit.

How is this testing U-Boot? This sounds like a test of the user environment?

Even if the test passes on gitlab, if the user is using some sort of
different serial access then it might fail...?

If you want a test for 8-bit serial, you should add one.

>
> U-Boot's internal function run_command() cannot send electrical signals
> inbound to the device's UART.
>
> You may implement an additional test with a different scope. But that
> should not replace the current test.
>
> Best regards
>
> Heinrich

Regards,
Simon