Re: [PATCH 1/1] sandbox: enable CONFIG_EFI_CAPSULE_AUTHENTICATE

2023-04-24 Thread Simon Glass
Hi Heinrich,

On Thu, 20 Apr 2023 at 01:34, Heinrich Schuchardt
 wrote:
>
> On 4/20/23 09:12, AKASHI Takahiro wrote:
> > On Thu, Apr 20, 2023 at 09:30:24AM +0300, Ilias Apalodimas wrote:
> >> Hi Simon,
> >>
> >> On Thu, 20 Apr 2023 at 01:41, Simon Glass  wrote:
> >>>
> >>> Hi Ilias,
> >>>
> >>> On Wed, 19 Apr 2023 at 18:05, Ilias Apalodimas
> >>>  wrote:
> 
>  On Tue, Apr 18, 2023 at 10:47:24AM -0600, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 14 Apr 2023 at 02:39, Heinrich Schuchardt
> >  wrote:
> >>
> >> Without CONFIG_EFI_CAPSULE_AUTHENTICATE=y the following tests are 
> >> skipped:
> >>
> >> * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> >> * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
> >>
> >> Signed-off-by: Heinrich Schuchardt 
> >> ---
> >>   configs/sandbox_defconfig  | 1 +
> >>   configs/sandbox_flattree_defconfig | 1 +
> >>   2 files changed, 2 insertions(+)
> >>
> >
> > Reviewed-by: Simon Glass 
> >
> > This still has the problem that it reboots in the middle of the test.
> > Can we get that fixed? If someone at Linaro isn't interested I could
> > take a look at it.
> 
>  I think we discussed this in the past.  We *need* to reboot as that's 
>  what
>  the EFI describes.  Why is it a problem?
> >>>
> >>> Would you like me to provide a patch that shows it not rebooting? It
> >>> is a simple matter of kicking off the update process, which we can do
> >>> directly, without a reboot.
> >>
> >> I know it's a single efidebug command to trigger this without a
> >> reboot.  However it is not going to test the final code which is meant
> >> to run capsule updates on a reboot as the spec defines.  So again, why
> >> is rebooting a problem?
> >
> > I guess that what we are looking for are different.
> > Simon's aim is to do, so called, an unit test first, using an internal 
> > function,
> > while what I, rather we?, intended to do a system test or 
> > standard-conformance test.
> >
> > I believe that we need to have some consensus on test methodology to be
> > added to U-Boot repository.
> >
> > -Takahiro Akashi
>
> Unit tests could cover aspects of capsule updates. But what we enable
> here are the integration tests. For such a complicated functionality as
> capsule updates unit tests alone are inadequate. They could be added on
> top to test individual functions.

I would be happy with an architecture that splits the tests into
parts. But today all we have is the functional test. When it fails,
where do I look?

I'm not so wedded to unit tests, or at least not in the traditional
sense. Most of the tests in U-Boot are hybrid, in that they test one
subsystem / function while relying on lots of others to work as
expected. For example, we require block devices, partitions,
environment variables, etc. to work in the case of capsule updates,
but we are not actually trying to test those things. We mostly use
traditional unit tests only for 'leaf' functionality, like GPIOs,
string processing, environment.

In any case, we need to design our code for testing, to actually find
bugs and to tell you where the bug is, so far as possible.

Regards,
Simon


Re: [PATCH 1/1] sandbox: enable CONFIG_EFI_CAPSULE_AUTHENTICATE

2023-04-20 Thread Heinrich Schuchardt

On 4/20/23 09:12, AKASHI Takahiro wrote:

On Thu, Apr 20, 2023 at 09:30:24AM +0300, Ilias Apalodimas wrote:

Hi Simon,

On Thu, 20 Apr 2023 at 01:41, Simon Glass  wrote:


Hi Ilias,

On Wed, 19 Apr 2023 at 18:05, Ilias Apalodimas
 wrote:


On Tue, Apr 18, 2023 at 10:47:24AM -0600, Simon Glass wrote:

Hi Heinrich,

On Fri, 14 Apr 2023 at 02:39, Heinrich Schuchardt
 wrote:


Without CONFIG_EFI_CAPSULE_AUTHENTICATE=y the following tests are skipped:

* test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
* test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py

Signed-off-by: Heinrich Schuchardt 
---
  configs/sandbox_defconfig  | 1 +
  configs/sandbox_flattree_defconfig | 1 +
  2 files changed, 2 insertions(+)



Reviewed-by: Simon Glass 

This still has the problem that it reboots in the middle of the test.
Can we get that fixed? If someone at Linaro isn't interested I could
take a look at it.


I think we discussed this in the past.  We *need* to reboot as that's what
the EFI describes.  Why is it a problem?


Would you like me to provide a patch that shows it not rebooting? It
is a simple matter of kicking off the update process, which we can do
directly, without a reboot.


I know it's a single efidebug command to trigger this without a
reboot.  However it is not going to test the final code which is meant
to run capsule updates on a reboot as the spec defines.  So again, why
is rebooting a problem?


I guess that what we are looking for are different.
Simon's aim is to do, so called, an unit test first, using an internal function,
while what I, rather we?, intended to do a system test or standard-conformance 
test.

I believe that we need to have some consensus on test methodology to be
added to U-Boot repository.

-Takahiro Akashi


Unit tests could cover aspects of capsule updates. But what we enable 
here are the integration tests. For such a complicated functionality as 
capsule updates unit tests alone are inadequate. They could be added on 
top to test individual functions.


Best regards

Heinrich



Re: [PATCH 1/1] sandbox: enable CONFIG_EFI_CAPSULE_AUTHENTICATE

2023-04-20 Thread AKASHI Takahiro
On Thu, Apr 20, 2023 at 09:30:24AM +0300, Ilias Apalodimas wrote:
> Hi Simon,
> 
> On Thu, 20 Apr 2023 at 01:41, Simon Glass  wrote:
> >
> > Hi Ilias,
> >
> > On Wed, 19 Apr 2023 at 18:05, Ilias Apalodimas
> >  wrote:
> > >
> > > On Tue, Apr 18, 2023 at 10:47:24AM -0600, Simon Glass wrote:
> > > > Hi Heinrich,
> > > >
> > > > On Fri, 14 Apr 2023 at 02:39, Heinrich Schuchardt
> > > >  wrote:
> > > > >
> > > > > Without CONFIG_EFI_CAPSULE_AUTHENTICATE=y the following tests are 
> > > > > skipped:
> > > > >
> > > > > * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> > > > > * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
> > > > >
> > > > > Signed-off-by: Heinrich Schuchardt 
> > > > > ---
> > > > >  configs/sandbox_defconfig  | 1 +
> > > > >  configs/sandbox_flattree_defconfig | 1 +
> > > > >  2 files changed, 2 insertions(+)
> > > > >
> > > >
> > > > Reviewed-by: Simon Glass 
> > > >
> > > > This still has the problem that it reboots in the middle of the test.
> > > > Can we get that fixed? If someone at Linaro isn't interested I could
> > > > take a look at it.
> > >
> > > I think we discussed this in the past.  We *need* to reboot as that's what
> > > the EFI describes.  Why is it a problem?
> >
> > Would you like me to provide a patch that shows it not rebooting? It
> > is a simple matter of kicking off the update process, which we can do
> > directly, without a reboot.
> 
> I know it's a single efidebug command to trigger this without a
> reboot.  However it is not going to test the final code which is meant
> to run capsule updates on a reboot as the spec defines.  So again, why
> is rebooting a problem?

I guess that what we are looking for are different.
Simon's aim is to do, so called, an unit test first, using an internal function,
while what I, rather we?, intended to do a system test or standard-conformance 
test.

I believe that we need to have some consensus on test methodology to be
added to U-Boot repository.

-Takahiro Akashi

> Regards
> /Ilias
> >
> > Regards,
> > Simon


Re: [PATCH 1/1] sandbox: enable CONFIG_EFI_CAPSULE_AUTHENTICATE

2023-04-20 Thread Ilias Apalodimas
Hi Simon,

On Thu, 20 Apr 2023 at 01:41, Simon Glass  wrote:
>
> Hi Ilias,
>
> On Wed, 19 Apr 2023 at 18:05, Ilias Apalodimas
>  wrote:
> >
> > On Tue, Apr 18, 2023 at 10:47:24AM -0600, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Fri, 14 Apr 2023 at 02:39, Heinrich Schuchardt
> > >  wrote:
> > > >
> > > > Without CONFIG_EFI_CAPSULE_AUTHENTICATE=y the following tests are 
> > > > skipped:
> > > >
> > > > * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> > > > * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
> > > >
> > > > Signed-off-by: Heinrich Schuchardt 
> > > > ---
> > > >  configs/sandbox_defconfig  | 1 +
> > > >  configs/sandbox_flattree_defconfig | 1 +
> > > >  2 files changed, 2 insertions(+)
> > > >
> > >
> > > Reviewed-by: Simon Glass 
> > >
> > > This still has the problem that it reboots in the middle of the test.
> > > Can we get that fixed? If someone at Linaro isn't interested I could
> > > take a look at it.
> >
> > I think we discussed this in the past.  We *need* to reboot as that's what
> > the EFI describes.  Why is it a problem?
>
> Would you like me to provide a patch that shows it not rebooting? It
> is a simple matter of kicking off the update process, which we can do
> directly, without a reboot.

I know it's a single efidebug command to trigger this without a
reboot.  However it is not going to test the final code which is meant
to run capsule updates on a reboot as the spec defines.  So again, why
is rebooting a problem?

Regards
/Ilias
>
> Regards,
> Simon


Re: [PATCH 1/1] sandbox: enable CONFIG_EFI_CAPSULE_AUTHENTICATE

2023-04-19 Thread Simon Glass
Hi Ilias,

On Wed, 19 Apr 2023 at 18:05, Ilias Apalodimas
 wrote:
>
> On Tue, Apr 18, 2023 at 10:47:24AM -0600, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 14 Apr 2023 at 02:39, Heinrich Schuchardt
> >  wrote:
> > >
> > > Without CONFIG_EFI_CAPSULE_AUTHENTICATE=y the following tests are skipped:
> > >
> > > * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> > > * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
> > >
> > > Signed-off-by: Heinrich Schuchardt 
> > > ---
> > >  configs/sandbox_defconfig  | 1 +
> > >  configs/sandbox_flattree_defconfig | 1 +
> > >  2 files changed, 2 insertions(+)
> > >
> >
> > Reviewed-by: Simon Glass 
> >
> > This still has the problem that it reboots in the middle of the test.
> > Can we get that fixed? If someone at Linaro isn't interested I could
> > take a look at it.
>
> I think we discussed this in the past.  We *need* to reboot as that's what
> the EFI describes.  Why is it a problem?

Would you like me to provide a patch that shows it not rebooting? It
is a simple matter of kicking off the update process, which we can do
directly, without a reboot.

Regards,
Simon


Re: [PATCH 1/1] sandbox: enable CONFIG_EFI_CAPSULE_AUTHENTICATE

2023-04-19 Thread Ilias Apalodimas
On Tue, Apr 18, 2023 at 10:47:24AM -0600, Simon Glass wrote:
> Hi Heinrich,
>
> On Fri, 14 Apr 2023 at 02:39, Heinrich Schuchardt
>  wrote:
> >
> > Without CONFIG_EFI_CAPSULE_AUTHENTICATE=y the following tests are skipped:
> >
> > * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> > * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
> >
> > Signed-off-by: Heinrich Schuchardt 
> > ---
> >  configs/sandbox_defconfig  | 1 +
> >  configs/sandbox_flattree_defconfig | 1 +
> >  2 files changed, 2 insertions(+)
> >
>
> Reviewed-by: Simon Glass 
>
> This still has the problem that it reboots in the middle of the test.
> Can we get that fixed? If someone at Linaro isn't interested I could
> take a look at it.

I think we discussed this in the past.  We *need* to reboot as that's what
the EFI describes.  Why is it a problem?

Regards
/Ilias
>
> Regards,
> Simon


Re: [PATCH 1/1] sandbox: enable CONFIG_EFI_CAPSULE_AUTHENTICATE

2023-04-18 Thread Simon Glass
Hi Heinrich,

On Fri, 14 Apr 2023 at 02:39, Heinrich Schuchardt
 wrote:
>
> Without CONFIG_EFI_CAPSULE_AUTHENTICATE=y the following tests are skipped:
>
> * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  configs/sandbox_defconfig  | 1 +
>  configs/sandbox_flattree_defconfig | 1 +
>  2 files changed, 2 insertions(+)
>

Reviewed-by: Simon Glass 

This still has the problem that it reboots in the middle of the test.
Can we get that fixed? If someone at Linaro isn't interested I could
take a look at it.

Regards,
Simon


Re: [PATCH 1/1] sandbox: enable CONFIG_EFI_CAPSULE_AUTHENTICATE

2023-04-18 Thread Ilias Apalodimas
On Fri, 14 Apr 2023 at 11:39, Heinrich Schuchardt
 wrote:
>
> Without CONFIG_EFI_CAPSULE_AUTHENTICATE=y the following tests are skipped:
>
> * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
> * test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  configs/sandbox_defconfig  | 1 +
>  configs/sandbox_flattree_defconfig | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index ca95b2c5d2..01c191818d 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -335,6 +335,7 @@ CONFIG_ERRNO_STR=y
>  CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
>  CONFIG_EFI_CAPSULE_ON_DISK=y
>  CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> +CONFIG_EFI_CAPSULE_AUTHENTICATE=y
>  CONFIG_EFI_SECURE_BOOT=y
>  CONFIG_TEST_FDTDEC=y
>  CONFIG_UNIT_TEST=y
> diff --git a/configs/sandbox_flattree_defconfig 
> b/configs/sandbox_flattree_defconfig
> index e9fcc5b0cb..8c2bceae22 100644
> --- a/configs/sandbox_flattree_defconfig
> +++ b/configs/sandbox_flattree_defconfig
> @@ -226,6 +226,7 @@ CONFIG_ERRNO_STR=y
>  CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
>  CONFIG_EFI_CAPSULE_ON_DISK=y
>  CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> +CONFIG_EFI_CAPSULE_AUTHENTICATE=y
>  CONFIG_UNIT_TEST=y
>  CONFIG_UT_TIME=y
>  CONFIG_UT_DM=y
> --
> 2.39.2
>
Acked-by: Ilias Apalodimas 


[PATCH 1/1] sandbox: enable CONFIG_EFI_CAPSULE_AUTHENTICATE

2023-04-14 Thread Heinrich Schuchardt
Without CONFIG_EFI_CAPSULE_AUTHENTICATE=y the following tests are skipped:

* test/py/tests/test_efi_capsule/test_capsule_firmware_signed_fit.py
* test/py/tests/test_efi_capsule/test_capsule_firmware_signed_raw.py

Signed-off-by: Heinrich Schuchardt 
---
 configs/sandbox_defconfig  | 1 +
 configs/sandbox_flattree_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index ca95b2c5d2..01c191818d 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -335,6 +335,7 @@ CONFIG_ERRNO_STR=y
 CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
+CONFIG_EFI_CAPSULE_AUTHENTICATE=y
 CONFIG_EFI_SECURE_BOOT=y
 CONFIG_TEST_FDTDEC=y
 CONFIG_UNIT_TEST=y
diff --git a/configs/sandbox_flattree_defconfig 
b/configs/sandbox_flattree_defconfig
index e9fcc5b0cb..8c2bceae22 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -226,6 +226,7 @@ CONFIG_ERRNO_STR=y
 CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
+CONFIG_EFI_CAPSULE_AUTHENTICATE=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
-- 
2.39.2