mbox series

[v8,00/18] efi_loader: add capsule update support

Message ID 20201113041511.48207-1-takahiro.akashi@linaro.org
Headers show
Series efi_loader: add capsule update support | expand

Message

AKASHI Takahiro Nov. 13, 2020, 4:14 a.m. UTC
Summary
=======
'UpdateCapsule' is one of runtime services defined in UEFI specification
and its aim is to allow a caller (OS) to pass information to the firmware,
i.e. U-Boot. This is mostly used to update firmware binary on devices by
instructions from OS.

While 'UpdateCapsule' is a runtime services function, it is, at least
initially, supported only before exiting boot services alike other runtime
functions, [Get/]SetVariable. This is because modifying storage which may
be shared with OS must be carefully designed and there is no general
assumption that we can do it.

Therefore, we practically support only "capsule on disk"; any capsule can
be handed over to UEFI subsystem as a file on a specific file system.

In this patch series, all the related definitions and structures are given
as UEFI specification describes, and basic framework for capsule support
is provided. Currently supported is
 * firmware update (Firmware Management Protocol or simply FMP)

Most of functionality of firmware update is provided by FMP driver and
it can be, by nature, system/platform-specific. So you can and should
implement your own FMP driver(s) based on your system requirements.
Under the current implementation, we provide two basic but generic
drivers with two formats:
  * FIT image format (as used in TFTP update and dfu)
  * raw image format

It's totally up to users which one, or both, should be used on users'
system depending on user requirements.

Quick usage
===========
1. You can create a capsule file with the following host command:

  $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

2. Put the file under:

  /EFI/UpdateCapsule of UEFI system partition

3. Specify firmware storage to be updated in "dfu_alt_info" variable
   (Please follow README.dfu for details.)

  ==> env set dfu_alt_info '...'

4. After setting up UEFI's OsIndications variable, reboot U-Boot:

  OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

Patch structure
===============
Patch#1-#4,#12: preparatory patches
Patch#5-#11,#13: main part of implementation
Patch#14-#15: utilities
Patch#16-#17: pytests
Patch#18: for sandbox test

[1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

Prerequisite patches
====================
None

Test
====
* passed all the pytests which are included in this patch series
  on sandbox build locally.
* skipped (or 'S', but it's not a failure, 'F') in Travis CI because
  "virt-make-fs" cannot be executed.

Issues
======
* Timing of executing capsules-on-disk
  Currently, processing a capsule is triggered only as part of
  UEFI subsystem initialization. This means that, for example,
  firmware update, may not take place at system booting time and
  will potentially be delayed until a first call of any UEFI functions.
    => See patch#5 for my proposal
* A bunch of warnings like
    WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef'
    where possible
  I don't think that fixing those improves anything.
* Add a document in uefi.rst

TODO's
======
(Won't be addressed in this series.)
* capsule authentication
* capsule dependency (dependency expression instruction set, or depex)
* loading drivers in a capsule
* handling RESET flag in a capsule and QeuryCapsuleCaps
* full semantics of ESRT (EFI System Resource Table)
* enabling capsule API at runtime
* json capsule
* recovery from update failure

Changes
=======
v8 (November 13, 2020)
* fix a "not used" warning against update_load() in some configuration
  (Patch#3)
* fix failures (marked as 'F') in *secure boot* test in Travis CI by
  making "EFI_CAPSULE_ON_DISK_EARLY" 'default n' (Patch#8)
* fix failures (marked as 'E') at pytest setup in Travis CI by changing
  a python file's name along with removing unused definitions (Patch#16)
* add Patch#18 to enable tests to be run with default sandbox config
  (Patch#18)

v7 (October 29, 2020)
* rename CONFIG_DFU_ALT to CONFIG_DFU_WRITE_ALT (Patch#1,#3,#13)

v6 RESEND (October 29, 2020)
* rebased on v2021.01-rc1

v6 (September 7, 2020)
* temporarily drop the prerequisite patch[2]
* add a missing field (dependencies) in efi_api.h (but never used) (Patch#10)
* add a missing field (image_capsule_support) and related definitions
  in efi_api.h (Patch#10, #15)
* cosmetic changes on constant definitions in efi_api.h (Patch#10)
* strict check for INVALID_PARAMETER at GET_IMAGE_INFO api (Patch#11,#13)
* fix warnings in pytest (Patch#16,#17)

v5 (August 3, 2020)
* removed superfluous type conversion at dfu_write_from_mem_addr()
  (Patch#2)
* introduced a common helper function, efi_create_indexed_name()
  (Patch#6,#7,#8)
* use efi_[get|set]_variable_int(), if necessary, with READ_ONLY
  (Patch#7,#8)
* return EFI_UNSUPPORTED at Patch#7
* changed the word, number, to 'index' (Patch#7,#8)
* removed 'ifdef CONFIG_EFI_CAPSULE_ON_DISK' from a header (Patch#8)
* initialize 'CapsuleLast' in efi_init_obj_list() (Patch#7,#8)
* added 'const' qualifier for filename argument at
  efi_capsule_[read|delete]_file() (Patch#8)

v4 (July 22, 2020)
* rebased to Heinrich's current efi-2020-10
* rework dfu-related code to align with Heinrich's change (Patch#1,#3)
* change a type of 'addr' argument from int to 'void *' per Sughosh's
  comment (Patch#2-#3,#11-#12)
* rework/simplify pytests (Patch#15-#16)
  - utilize virt-make-fs
  - drop Test Case 1 (updating U-Boot environment data)
  - remove useless definitions (MNT_PNT, EFI_CAPSULE_IMAGE_NAME)
  - apply autopep8

v3 (July 10, 2020)
* rebased to Heinrich's current efi-2020-10-rc1
* refactor efi_firmware_[fit|raw]_get_image_info() (patch#11,#13)

v2 (June 17, 2020)
* rebased to v2020.07-rc4
* add preparatory patches for dfu (Patch#1-#5, #12)
* rework FIT capsule driver to utilize dfu_alt_info instead of CONFIG_xxx
  (patch#11)
* extend get_image_info() to correspond to dfu_alt_info
  (patch#11)
* add a 'raw binary' capsule support
  (patch#13, #17)
* allow multiple capsule formats (with different GUIDs) to be installed
  (patch#11, #13)
* extend mkeficapsule command to accept additional parameters, like
    version/index/hardware instance for a capsule header info.
  (patch#15)
* mkeficapsule can now also generate raw-binary capsule
  (patch#16)
* add function descriptions
* apply autopep8 to pytests and fix more against pylint

v1 (April 27, 2020)
* rebased to v2020.07-rc
* removed already-merged patches (RFC's #1 to #4)
* dropped 'variable update' capsule support (RFC's patch#10)
* dropped 'variable configuration table' support (RFC's patch#11)
  (Those two should be discussed separately.)
* add preparatory patches (patch#1/#2)
* fix several build errors
* rename some Kconfig options to be aligned with UEFI specification's terms
  (patch#3,4,6,7)
* enforce UpdateCapsule API to be disabled after ExitBootServices (patch#3)
* use config table, runtime_services_supported, instead of variable (patch#3)
* make EFI_CAPSULE_ON_DISK buildable even if UpdateCapsule API is disabled
  (patch4)
* support OsIndications, invoking capsule-on-disk only if the variable
  indicates so (patch#4)
* introduced EFI_CAPSULE_ON_DISK_EARLY to invoke capsule-on-disk in U-Boot
  initialization (patch#4)
* detect capsule files only if they are on EFI system partition (patch#4)
* use printf, rather than EFI_PRINT, in error cases (patch#4)
* use 'header_size' field to retrieve capsule data, adding sanity checks
  against capsule size (patch#6)
* call fmpt driver interfaces with EFI_CALL (patch#6)
* remove 'variable update capsule'-related code form mkeficapsule (patch#9)
* add a test case of OsIndications not being set properly (patch#10)
* adjust test scenario for EFI_CAPSULE_ON_DISK_EARLY (patch#10)
* revise pytest scripts (patch#10)

Initial release as RFC (March 17, 2020)

AKASHI Takahiro (18):
  dfu: rename dfu_tftp_write() to dfu_write_by_name()
  dfu: modify an argument type for an address
  common: update: add a generic interface for FIT image
  dfu: export dfu_list
  efi_loader: add option to initialise EFI subsystem early
  efi_loader: add efi_create_indexed_name()
  efi_loader: define UpdateCapsule api
  efi_loader: capsule: add capsule_on_disk support
  efi_loader: capsule: add memory range capsule definitions
  efi_loader: capsule: support firmware update
  efi_loader: add firmware management protocol for FIT image
  dfu: add dfu_write_by_alt()
  efi_loader: add firmware management protocol for raw image
  cmd: add "efidebug capsule" command
  tools: add mkeficapsule command for UEFI capsule update
  test/py: efi_capsule: test for FIT image capsule
  test/py: efi_capsule: test for raw image capsule
  sandbox: enable capsule update for testing

 cmd/efidebug.c                                | 235 +++++
 common/Kconfig                                |  15 +
 common/Makefile                               |   3 +-
 common/board_r.c                              |   6 +
 common/main.c                                 |   4 +
 common/update.c                               |  77 +-
 configs/sandbox64_defconfig                   |   6 +
 configs/sandbox_defconfig                     |   6 +
 drivers/dfu/Kconfig                           |   5 +
 drivers/dfu/Makefile                          |   2 +-
 drivers/dfu/dfu.c                             |   2 +-
 drivers/dfu/dfu_alt.c                         | 125 +++
 drivers/dfu/dfu_tftp.c                        |  65 --
 include/dfu.h                                 |  57 +-
 include/efi_api.h                             | 166 ++++
 include/efi_loader.h                          |  30 +
 include/image.h                               |  12 +
 lib/efi_loader/Kconfig                        |  72 ++
 lib/efi_loader/Makefile                       |   2 +
 lib/efi_loader/efi_capsule.c                  | 917 ++++++++++++++++++
 lib/efi_loader/efi_firmware.c                 | 403 ++++++++
 lib/efi_loader/efi_runtime.c                  | 104 +-
 lib/efi_loader/efi_setup.c                    | 106 +-
 .../py/tests/test_efi_capsule/capsule_defs.py |   5 +
 test/py/tests/test_efi_capsule/conftest.py    |  74 ++
 .../test_efi_capsule/test_capsule_firmware.py | 241 +++++
 .../tests/test_efi_capsule/uboot_bin_env.its  |  36 +
 tools/Makefile                                |   2 +
 tools/mkeficapsule.c                          | 239 +++++
 29 files changed, 2877 insertions(+), 140 deletions(-)
 create mode 100644 drivers/dfu/dfu_alt.c
 delete mode 100644 drivers/dfu/dfu_tftp.c
 create mode 100644 lib/efi_loader/efi_capsule.c
 create mode 100644 lib/efi_loader/efi_firmware.c
 create mode 100644 test/py/tests/test_efi_capsule/capsule_defs.py
 create mode 100644 test/py/tests/test_efi_capsule/conftest.py
 create mode 100644 test/py/tests/test_efi_capsule/test_capsule_firmware.py
 create mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
 create mode 100644 tools/mkeficapsule.c

-- 
2.28.0

Comments

Heinrich Schuchardt Nov. 13, 2020, 7:18 a.m. UTC | #1
On 11/13/20 5:14 AM, AKASHI Takahiro wrote:
> Summary

> =======

> 'UpdateCapsule' is one of runtime services defined in UEFI specification

> and its aim is to allow a caller (OS) to pass information to the firmware,

> i.e. U-Boot. This is mostly used to update firmware binary on devices by

> instructions from OS.

>

> While 'UpdateCapsule' is a runtime services function, it is, at least

> initially, supported only before exiting boot services alike other runtime

> functions, [Get/]SetVariable. This is because modifying storage which may

> be shared with OS must be carefully designed and there is no general

> assumption that we can do it.

>

> Therefore, we practically support only "capsule on disk"; any capsule can

> be handed over to UEFI subsystem as a file on a specific file system.

>

> In this patch series, all the related definitions and structures are given

> as UEFI specification describes, and basic framework for capsule support

> is provided. Currently supported is

>  * firmware update (Firmware Management Protocol or simply FMP)

>

> Most of functionality of firmware update is provided by FMP driver and

> it can be, by nature, system/platform-specific. So you can and should

> implement your own FMP driver(s) based on your system requirements.

> Under the current implementation, we provide two basic but generic

> drivers with two formats:

>   * FIT image format (as used in TFTP update and dfu)

>   * raw image format

>

> It's totally up to users which one, or both, should be used on users'

> system depending on user requirements.

>

> Quick usage

> ===========

> 1. You can create a capsule file with the following host command:

>

>   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

>

> 2. Put the file under:

>

>   /EFI/UpdateCapsule of UEFI system partition

>

> 3. Specify firmware storage to be updated in "dfu_alt_info" variable

>    (Please follow README.dfu for details.)

>

>   ==> env set dfu_alt_info '...'

>

> 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

>

>   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

>

> Patch structure

> ===============

> Patch#1-#4,#12: preparatory patches

> Patch#5-#11,#13: main part of implementation

> Patch#14-#15: utilities

> Patch#16-#17: pytests

> Patch#18: for sandbox test

>

> [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

>

> Prerequisite patches

> ====================

> None

>

> Test

> ====

> * passed all the pytests which are included in this patch series

>   on sandbox build locally.

> * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

>   "virt-make-fs" cannot be executed.


Dear Takahiro,

please, rebase your series on origin/master. A prior version of the
first patches is already applied.

Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging
the remaining patches.

Best regards

Heinrich

>

> Issues

> ======

> * Timing of executing capsules-on-disk

>   Currently, processing a capsule is triggered only as part of

>   UEFI subsystem initialization. This means that, for example,

>   firmware update, may not take place at system booting time and

>   will potentially be delayed until a first call of any UEFI functions.

>     => See patch#5 for my proposal

> * A bunch of warnings like

>     WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef'

>     where possible

>   I don't think that fixing those improves anything.

> * Add a document in uefi.rst

>

> TODO's

> ======

> (Won't be addressed in this series.)

> * capsule authentication

> * capsule dependency (dependency expression instruction set, or depex)

> * loading drivers in a capsule

> * handling RESET flag in a capsule and QeuryCapsuleCaps

> * full semantics of ESRT (EFI System Resource Table)

> * enabling capsule API at runtime

> * json capsule

> * recovery from update failure

>

> Changes

> =======

> v8 (November 13, 2020)

> * fix a "not used" warning against update_load() in some configuration

>   (Patch#3)

> * fix failures (marked as 'F') in *secure boot* test in Travis CI by

>   making "EFI_CAPSULE_ON_DISK_EARLY" 'default n' (Patch#8)

> * fix failures (marked as 'E') at pytest setup in Travis CI by changing

>   a python file's name along with removing unused definitions (Patch#16)

> * add Patch#18 to enable tests to be run with default sandbox config

>   (Patch#18)

>

> v7 (October 29, 2020)

> * rename CONFIG_DFU_ALT to CONFIG_DFU_WRITE_ALT (Patch#1,#3,#13)

>

> v6 RESEND (October 29, 2020)

> * rebased on v2021.01-rc1

>

> v6 (September 7, 2020)

> * temporarily drop the prerequisite patch[2]

> * add a missing field (dependencies) in efi_api.h (but never used) (Patch#10)

> * add a missing field (image_capsule_support) and related definitions

>   in efi_api.h (Patch#10, #15)

> * cosmetic changes on constant definitions in efi_api.h (Patch#10)

> * strict check for INVALID_PARAMETER at GET_IMAGE_INFO api (Patch#11,#13)

> * fix warnings in pytest (Patch#16,#17)

>

> v5 (August 3, 2020)

> * removed superfluous type conversion at dfu_write_from_mem_addr()

>   (Patch#2)

> * introduced a common helper function, efi_create_indexed_name()

>   (Patch#6,#7,#8)

> * use efi_[get|set]_variable_int(), if necessary, with READ_ONLY

>   (Patch#7,#8)

> * return EFI_UNSUPPORTED at Patch#7

> * changed the word, number, to 'index' (Patch#7,#8)

> * removed 'ifdef CONFIG_EFI_CAPSULE_ON_DISK' from a header (Patch#8)

> * initialize 'CapsuleLast' in efi_init_obj_list() (Patch#7,#8)

> * added 'const' qualifier for filename argument at

>   efi_capsule_[read|delete]_file() (Patch#8)

>

> v4 (July 22, 2020)

> * rebased to Heinrich's current efi-2020-10

> * rework dfu-related code to align with Heinrich's change (Patch#1,#3)

> * change a type of 'addr' argument from int to 'void *' per Sughosh's

>   comment (Patch#2-#3,#11-#12)

> * rework/simplify pytests (Patch#15-#16)

>   - utilize virt-make-fs

>   - drop Test Case 1 (updating U-Boot environment data)

>   - remove useless definitions (MNT_PNT, EFI_CAPSULE_IMAGE_NAME)

>   - apply autopep8

>

> v3 (July 10, 2020)

> * rebased to Heinrich's current efi-2020-10-rc1

> * refactor efi_firmware_[fit|raw]_get_image_info() (patch#11,#13)

>

> v2 (June 17, 2020)

> * rebased to v2020.07-rc4

> * add preparatory patches for dfu (Patch#1-#5, #12)

> * rework FIT capsule driver to utilize dfu_alt_info instead of CONFIG_xxx

>   (patch#11)

> * extend get_image_info() to correspond to dfu_alt_info

>   (patch#11)

> * add a 'raw binary' capsule support

>   (patch#13, #17)

> * allow multiple capsule formats (with different GUIDs) to be installed

>   (patch#11, #13)

> * extend mkeficapsule command to accept additional parameters, like

>     version/index/hardware instance for a capsule header info.

>   (patch#15)

> * mkeficapsule can now also generate raw-binary capsule

>   (patch#16)

> * add function descriptions

> * apply autopep8 to pytests and fix more against pylint

>

> v1 (April 27, 2020)

> * rebased to v2020.07-rc

> * removed already-merged patches (RFC's #1 to #4)

> * dropped 'variable update' capsule support (RFC's patch#10)

> * dropped 'variable configuration table' support (RFC's patch#11)

>   (Those two should be discussed separately.)

> * add preparatory patches (patch#1/#2)

> * fix several build errors

> * rename some Kconfig options to be aligned with UEFI specification's terms

>   (patch#3,4,6,7)

> * enforce UpdateCapsule API to be disabled after ExitBootServices (patch#3)

> * use config table, runtime_services_supported, instead of variable (patch#3)

> * make EFI_CAPSULE_ON_DISK buildable even if UpdateCapsule API is disabled

>   (patch4)

> * support OsIndications, invoking capsule-on-disk only if the variable

>   indicates so (patch#4)

> * introduced EFI_CAPSULE_ON_DISK_EARLY to invoke capsule-on-disk in U-Boot

>   initialization (patch#4)

> * detect capsule files only if they are on EFI system partition (patch#4)

> * use printf, rather than EFI_PRINT, in error cases (patch#4)

> * use 'header_size' field to retrieve capsule data, adding sanity checks

>   against capsule size (patch#6)

> * call fmpt driver interfaces with EFI_CALL (patch#6)

> * remove 'variable update capsule'-related code form mkeficapsule (patch#9)

> * add a test case of OsIndications not being set properly (patch#10)

> * adjust test scenario for EFI_CAPSULE_ON_DISK_EARLY (patch#10)

> * revise pytest scripts (patch#10)

>

> Initial release as RFC (March 17, 2020)

>

> AKASHI Takahiro (18):

>   dfu: rename dfu_tftp_write() to dfu_write_by_name()

>   dfu: modify an argument type for an address

>   common: update: add a generic interface for FIT image

>   dfu: export dfu_list

>   efi_loader: add option to initialise EFI subsystem early

>   efi_loader: add efi_create_indexed_name()

>   efi_loader: define UpdateCapsule api

>   efi_loader: capsule: add capsule_on_disk support

>   efi_loader: capsule: add memory range capsule definitions

>   efi_loader: capsule: support firmware update

>   efi_loader: add firmware management protocol for FIT image

>   dfu: add dfu_write_by_alt()

>   efi_loader: add firmware management protocol for raw image

>   cmd: add "efidebug capsule" command

>   tools: add mkeficapsule command for UEFI capsule update

>   test/py: efi_capsule: test for FIT image capsule

>   test/py: efi_capsule: test for raw image capsule

>   sandbox: enable capsule update for testing

>

>  cmd/efidebug.c                                | 235 +++++

>  common/Kconfig                                |  15 +

>  common/Makefile                               |   3 +-

>  common/board_r.c                              |   6 +

>  common/main.c                                 |   4 +

>  common/update.c                               |  77 +-

>  configs/sandbox64_defconfig                   |   6 +

>  configs/sandbox_defconfig                     |   6 +

>  drivers/dfu/Kconfig                           |   5 +

>  drivers/dfu/Makefile                          |   2 +-

>  drivers/dfu/dfu.c                             |   2 +-

>  drivers/dfu/dfu_alt.c                         | 125 +++

>  drivers/dfu/dfu_tftp.c                        |  65 --

>  include/dfu.h                                 |  57 +-

>  include/efi_api.h                             | 166 ++++

>  include/efi_loader.h                          |  30 +

>  include/image.h                               |  12 +

>  lib/efi_loader/Kconfig                        |  72 ++

>  lib/efi_loader/Makefile                       |   2 +

>  lib/efi_loader/efi_capsule.c                  | 917 ++++++++++++++++++

>  lib/efi_loader/efi_firmware.c                 | 403 ++++++++

>  lib/efi_loader/efi_runtime.c                  | 104 +-

>  lib/efi_loader/efi_setup.c                    | 106 +-

>  .../py/tests/test_efi_capsule/capsule_defs.py |   5 +

>  test/py/tests/test_efi_capsule/conftest.py    |  74 ++

>  .../test_efi_capsule/test_capsule_firmware.py | 241 +++++

>  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 +

>  tools/Makefile                                |   2 +

>  tools/mkeficapsule.c                          | 239 +++++

>  29 files changed, 2877 insertions(+), 140 deletions(-)

>  create mode 100644 drivers/dfu/dfu_alt.c

>  delete mode 100644 drivers/dfu/dfu_tftp.c

>  create mode 100644 lib/efi_loader/efi_capsule.c

>  create mode 100644 lib/efi_loader/efi_firmware.c

>  create mode 100644 test/py/tests/test_efi_capsule/capsule_defs.py

>  create mode 100644 test/py/tests/test_efi_capsule/conftest.py

>  create mode 100644 test/py/tests/test_efi_capsule/test_capsule_firmware.py

>  create mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its

>  create mode 100644 tools/mkeficapsule.c

>
AKASHI Takahiro Nov. 16, 2020, 12:37 a.m. UTC | #2
Heinrich,

On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:
> On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > Summary

> > =======

> > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > and its aim is to allow a caller (OS) to pass information to the firmware,

> > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > instructions from OS.

> >

> > While 'UpdateCapsule' is a runtime services function, it is, at least

> > initially, supported only before exiting boot services alike other runtime

> > functions, [Get/]SetVariable. This is because modifying storage which may

> > be shared with OS must be carefully designed and there is no general

> > assumption that we can do it.

> >

> > Therefore, we practically support only "capsule on disk"; any capsule can

> > be handed over to UEFI subsystem as a file on a specific file system.

> >

> > In this patch series, all the related definitions and structures are given

> > as UEFI specification describes, and basic framework for capsule support

> > is provided. Currently supported is

> >  * firmware update (Firmware Management Protocol or simply FMP)

> >

> > Most of functionality of firmware update is provided by FMP driver and

> > it can be, by nature, system/platform-specific. So you can and should

> > implement your own FMP driver(s) based on your system requirements.

> > Under the current implementation, we provide two basic but generic

> > drivers with two formats:

> >   * FIT image format (as used in TFTP update and dfu)

> >   * raw image format

> >

> > It's totally up to users which one, or both, should be used on users'

> > system depending on user requirements.

> >

> > Quick usage

> > ===========

> > 1. You can create a capsule file with the following host command:

> >

> >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> >

> > 2. Put the file under:

> >

> >   /EFI/UpdateCapsule of UEFI system partition

> >

> > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> >    (Please follow README.dfu for details.)

> >

> >   ==> env set dfu_alt_info '...'

> >

> > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> >

> >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> >

> > Patch structure

> > ===============

> > Patch#1-#4,#12: preparatory patches

> > Patch#5-#11,#13: main part of implementation

> > Patch#14-#15: utilities

> > Patch#16-#17: pytests

> > Patch#18: for sandbox test

> >

> > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> >

> > Prerequisite patches

> > ====================

> > None

> >

> > Test

> > ====

> > * passed all the pytests which are included in this patch series

> >   on sandbox build locally.

> > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> >   "virt-make-fs" cannot be executed.

> 

> Dear Takahiro,

> 

> please, rebase your series on origin/master. A prior version of the

> first patches is already applied.


You can simply pick up and apply non-merged patches without problems
except a function prototype change of efi_create_indexed_name() you made,
which is not trivial to me.

But anyway, I will post a clean patch set soon.

> Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> the remaining patches.


Where can we find the results?
I don't think there is no official information on those CI's.

# I normally watch out Travis with "github" only.

In addition, can you clarify your view about "virt-make-fs"/sudo issue?
You should have common understandings with Tom here.

-Takahiro Akashi


> Best regards

> 

> Heinrich

> 

> >

> > Issues

> > ======

> > * Timing of executing capsules-on-disk

> >   Currently, processing a capsule is triggered only as part of

> >   UEFI subsystem initialization. This means that, for example,

> >   firmware update, may not take place at system booting time and

> >   will potentially be delayed until a first call of any UEFI functions.

> >     => See patch#5 for my proposal

> > * A bunch of warnings like

> >     WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef'

> >     where possible

> >   I don't think that fixing those improves anything.

> > * Add a document in uefi.rst

> >

> > TODO's

> > ======

> > (Won't be addressed in this series.)

> > * capsule authentication

> > * capsule dependency (dependency expression instruction set, or depex)

> > * loading drivers in a capsule

> > * handling RESET flag in a capsule and QeuryCapsuleCaps

> > * full semantics of ESRT (EFI System Resource Table)

> > * enabling capsule API at runtime

> > * json capsule

> > * recovery from update failure

> >

> > Changes

> > =======

> > v8 (November 13, 2020)

> > * fix a "not used" warning against update_load() in some configuration

> >   (Patch#3)

> > * fix failures (marked as 'F') in *secure boot* test in Travis CI by

> >   making "EFI_CAPSULE_ON_DISK_EARLY" 'default n' (Patch#8)

> > * fix failures (marked as 'E') at pytest setup in Travis CI by changing

> >   a python file's name along with removing unused definitions (Patch#16)

> > * add Patch#18 to enable tests to be run with default sandbox config

> >   (Patch#18)

> >

> > v7 (October 29, 2020)

> > * rename CONFIG_DFU_ALT to CONFIG_DFU_WRITE_ALT (Patch#1,#3,#13)

> >

> > v6 RESEND (October 29, 2020)

> > * rebased on v2021.01-rc1

> >

> > v6 (September 7, 2020)

> > * temporarily drop the prerequisite patch[2]

> > * add a missing field (dependencies) in efi_api.h (but never used) (Patch#10)

> > * add a missing field (image_capsule_support) and related definitions

> >   in efi_api.h (Patch#10, #15)

> > * cosmetic changes on constant definitions in efi_api.h (Patch#10)

> > * strict check for INVALID_PARAMETER at GET_IMAGE_INFO api (Patch#11,#13)

> > * fix warnings in pytest (Patch#16,#17)

> >

> > v5 (August 3, 2020)

> > * removed superfluous type conversion at dfu_write_from_mem_addr()

> >   (Patch#2)

> > * introduced a common helper function, efi_create_indexed_name()

> >   (Patch#6,#7,#8)

> > * use efi_[get|set]_variable_int(), if necessary, with READ_ONLY

> >   (Patch#7,#8)

> > * return EFI_UNSUPPORTED at Patch#7

> > * changed the word, number, to 'index' (Patch#7,#8)

> > * removed 'ifdef CONFIG_EFI_CAPSULE_ON_DISK' from a header (Patch#8)

> > * initialize 'CapsuleLast' in efi_init_obj_list() (Patch#7,#8)

> > * added 'const' qualifier for filename argument at

> >   efi_capsule_[read|delete]_file() (Patch#8)

> >

> > v4 (July 22, 2020)

> > * rebased to Heinrich's current efi-2020-10

> > * rework dfu-related code to align with Heinrich's change (Patch#1,#3)

> > * change a type of 'addr' argument from int to 'void *' per Sughosh's

> >   comment (Patch#2-#3,#11-#12)

> > * rework/simplify pytests (Patch#15-#16)

> >   - utilize virt-make-fs

> >   - drop Test Case 1 (updating U-Boot environment data)

> >   - remove useless definitions (MNT_PNT, EFI_CAPSULE_IMAGE_NAME)

> >   - apply autopep8

> >

> > v3 (July 10, 2020)

> > * rebased to Heinrich's current efi-2020-10-rc1

> > * refactor efi_firmware_[fit|raw]_get_image_info() (patch#11,#13)

> >

> > v2 (June 17, 2020)

> > * rebased to v2020.07-rc4

> > * add preparatory patches for dfu (Patch#1-#5, #12)

> > * rework FIT capsule driver to utilize dfu_alt_info instead of CONFIG_xxx

> >   (patch#11)

> > * extend get_image_info() to correspond to dfu_alt_info

> >   (patch#11)

> > * add a 'raw binary' capsule support

> >   (patch#13, #17)

> > * allow multiple capsule formats (with different GUIDs) to be installed

> >   (patch#11, #13)

> > * extend mkeficapsule command to accept additional parameters, like

> >     version/index/hardware instance for a capsule header info.

> >   (patch#15)

> > * mkeficapsule can now also generate raw-binary capsule

> >   (patch#16)

> > * add function descriptions

> > * apply autopep8 to pytests and fix more against pylint

> >

> > v1 (April 27, 2020)

> > * rebased to v2020.07-rc

> > * removed already-merged patches (RFC's #1 to #4)

> > * dropped 'variable update' capsule support (RFC's patch#10)

> > * dropped 'variable configuration table' support (RFC's patch#11)

> >   (Those two should be discussed separately.)

> > * add preparatory patches (patch#1/#2)

> > * fix several build errors

> > * rename some Kconfig options to be aligned with UEFI specification's terms

> >   (patch#3,4,6,7)

> > * enforce UpdateCapsule API to be disabled after ExitBootServices (patch#3)

> > * use config table, runtime_services_supported, instead of variable (patch#3)

> > * make EFI_CAPSULE_ON_DISK buildable even if UpdateCapsule API is disabled

> >   (patch4)

> > * support OsIndications, invoking capsule-on-disk only if the variable

> >   indicates so (patch#4)

> > * introduced EFI_CAPSULE_ON_DISK_EARLY to invoke capsule-on-disk in U-Boot

> >   initialization (patch#4)

> > * detect capsule files only if they are on EFI system partition (patch#4)

> > * use printf, rather than EFI_PRINT, in error cases (patch#4)

> > * use 'header_size' field to retrieve capsule data, adding sanity checks

> >   against capsule size (patch#6)

> > * call fmpt driver interfaces with EFI_CALL (patch#6)

> > * remove 'variable update capsule'-related code form mkeficapsule (patch#9)

> > * add a test case of OsIndications not being set properly (patch#10)

> > * adjust test scenario for EFI_CAPSULE_ON_DISK_EARLY (patch#10)

> > * revise pytest scripts (patch#10)

> >

> > Initial release as RFC (March 17, 2020)

> >

> > AKASHI Takahiro (18):

> >   dfu: rename dfu_tftp_write() to dfu_write_by_name()

> >   dfu: modify an argument type for an address

> >   common: update: add a generic interface for FIT image

> >   dfu: export dfu_list

> >   efi_loader: add option to initialise EFI subsystem early

> >   efi_loader: add efi_create_indexed_name()

> >   efi_loader: define UpdateCapsule api

> >   efi_loader: capsule: add capsule_on_disk support

> >   efi_loader: capsule: add memory range capsule definitions

> >   efi_loader: capsule: support firmware update

> >   efi_loader: add firmware management protocol for FIT image

> >   dfu: add dfu_write_by_alt()

> >   efi_loader: add firmware management protocol for raw image

> >   cmd: add "efidebug capsule" command

> >   tools: add mkeficapsule command for UEFI capsule update

> >   test/py: efi_capsule: test for FIT image capsule

> >   test/py: efi_capsule: test for raw image capsule

> >   sandbox: enable capsule update for testing

> >

> >  cmd/efidebug.c                                | 235 +++++

> >  common/Kconfig                                |  15 +

> >  common/Makefile                               |   3 +-

> >  common/board_r.c                              |   6 +

> >  common/main.c                                 |   4 +

> >  common/update.c                               |  77 +-

> >  configs/sandbox64_defconfig                   |   6 +

> >  configs/sandbox_defconfig                     |   6 +

> >  drivers/dfu/Kconfig                           |   5 +

> >  drivers/dfu/Makefile                          |   2 +-

> >  drivers/dfu/dfu.c                             |   2 +-

> >  drivers/dfu/dfu_alt.c                         | 125 +++

> >  drivers/dfu/dfu_tftp.c                        |  65 --

> >  include/dfu.h                                 |  57 +-

> >  include/efi_api.h                             | 166 ++++

> >  include/efi_loader.h                          |  30 +

> >  include/image.h                               |  12 +

> >  lib/efi_loader/Kconfig                        |  72 ++

> >  lib/efi_loader/Makefile                       |   2 +

> >  lib/efi_loader/efi_capsule.c                  | 917 ++++++++++++++++++

> >  lib/efi_loader/efi_firmware.c                 | 403 ++++++++

> >  lib/efi_loader/efi_runtime.c                  | 104 +-

> >  lib/efi_loader/efi_setup.c                    | 106 +-

> >  .../py/tests/test_efi_capsule/capsule_defs.py |   5 +

> >  test/py/tests/test_efi_capsule/conftest.py    |  74 ++

> >  .../test_efi_capsule/test_capsule_firmware.py | 241 +++++

> >  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 +

> >  tools/Makefile                                |   2 +

> >  tools/mkeficapsule.c                          | 239 +++++

> >  29 files changed, 2877 insertions(+), 140 deletions(-)

> >  create mode 100644 drivers/dfu/dfu_alt.c

> >  delete mode 100644 drivers/dfu/dfu_tftp.c

> >  create mode 100644 lib/efi_loader/efi_capsule.c

> >  create mode 100644 lib/efi_loader/efi_firmware.c

> >  create mode 100644 test/py/tests/test_efi_capsule/capsule_defs.py

> >  create mode 100644 test/py/tests/test_efi_capsule/conftest.py

> >  create mode 100644 test/py/tests/test_efi_capsule/test_capsule_firmware.py

> >  create mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its

> >  create mode 100644 tools/mkeficapsule.c

> >

>
Tom Rini Nov. 16, 2020, 4:10 p.m. UTC | #3
On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:
> Heinrich,

> 

> On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:

> > On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > > Summary

> > > =======

> > > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > > and its aim is to allow a caller (OS) to pass information to the firmware,

> > > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > > instructions from OS.

> > >

> > > While 'UpdateCapsule' is a runtime services function, it is, at least

> > > initially, supported only before exiting boot services alike other runtime

> > > functions, [Get/]SetVariable. This is because modifying storage which may

> > > be shared with OS must be carefully designed and there is no general

> > > assumption that we can do it.

> > >

> > > Therefore, we practically support only "capsule on disk"; any capsule can

> > > be handed over to UEFI subsystem as a file on a specific file system.

> > >

> > > In this patch series, all the related definitions and structures are given

> > > as UEFI specification describes, and basic framework for capsule support

> > > is provided. Currently supported is

> > >  * firmware update (Firmware Management Protocol or simply FMP)

> > >

> > > Most of functionality of firmware update is provided by FMP driver and

> > > it can be, by nature, system/platform-specific. So you can and should

> > > implement your own FMP driver(s) based on your system requirements.

> > > Under the current implementation, we provide two basic but generic

> > > drivers with two formats:

> > >   * FIT image format (as used in TFTP update and dfu)

> > >   * raw image format

> > >

> > > It's totally up to users which one, or both, should be used on users'

> > > system depending on user requirements.

> > >

> > > Quick usage

> > > ===========

> > > 1. You can create a capsule file with the following host command:

> > >

> > >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> > >

> > > 2. Put the file under:

> > >

> > >   /EFI/UpdateCapsule of UEFI system partition

> > >

> > > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> > >    (Please follow README.dfu for details.)

> > >

> > >   ==> env set dfu_alt_info '...'

> > >

> > > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> > >

> > >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> > >

> > > Patch structure

> > > ===============

> > > Patch#1-#4,#12: preparatory patches

> > > Patch#5-#11,#13: main part of implementation

> > > Patch#14-#15: utilities

> > > Patch#16-#17: pytests

> > > Patch#18: for sandbox test

> > >

> > > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> > >

> > > Prerequisite patches

> > > ====================

> > > None

> > >

> > > Test

> > > ====

> > > * passed all the pytests which are included in this patch series

> > >   on sandbox build locally.

> > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> > >   "virt-make-fs" cannot be executed.

> > 

> > Dear Takahiro,

> > 

> > please, rebase your series on origin/master. A prior version of the

> > first patches is already applied.

> 

> You can simply pick up and apply non-merged patches without problems

> except a function prototype change of efi_create_indexed_name() you made,

> which is not trivial to me.

> 

> But anyway, I will post a clean patch set soon.

> 

> > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> > the remaining patches.

> 

> Where can we find the results?

> I don't think there is no official information on those CI's.


If you submit a pull request against https://github.com/u-boot/u-boot
Azure will run automatically and show the results.  We don't have
anything similar setup for GitLab, but since it uses the same Docker
images as Azure, so long as the syntax is right (and there are linters
if you're unsure of a change), it would be expected to work too.

-- 
Tom
AKASHI Takahiro Nov. 17, 2020, 12:16 a.m. UTC | #4
On Mon, Nov 16, 2020 at 11:10:12AM -0500, Tom Rini wrote:
> On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:

> > Heinrich,

> > 

> > On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:

> > > On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > > > Summary

> > > > =======

> > > > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > > > and its aim is to allow a caller (OS) to pass information to the firmware,

> > > > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > > > instructions from OS.

> > > >

> > > > While 'UpdateCapsule' is a runtime services function, it is, at least

> > > > initially, supported only before exiting boot services alike other runtime

> > > > functions, [Get/]SetVariable. This is because modifying storage which may

> > > > be shared with OS must be carefully designed and there is no general

> > > > assumption that we can do it.

> > > >

> > > > Therefore, we practically support only "capsule on disk"; any capsule can

> > > > be handed over to UEFI subsystem as a file on a specific file system.

> > > >

> > > > In this patch series, all the related definitions and structures are given

> > > > as UEFI specification describes, and basic framework for capsule support

> > > > is provided. Currently supported is

> > > >  * firmware update (Firmware Management Protocol or simply FMP)

> > > >

> > > > Most of functionality of firmware update is provided by FMP driver and

> > > > it can be, by nature, system/platform-specific. So you can and should

> > > > implement your own FMP driver(s) based on your system requirements.

> > > > Under the current implementation, we provide two basic but generic

> > > > drivers with two formats:

> > > >   * FIT image format (as used in TFTP update and dfu)

> > > >   * raw image format

> > > >

> > > > It's totally up to users which one, or both, should be used on users'

> > > > system depending on user requirements.

> > > >

> > > > Quick usage

> > > > ===========

> > > > 1. You can create a capsule file with the following host command:

> > > >

> > > >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> > > >

> > > > 2. Put the file under:

> > > >

> > > >   /EFI/UpdateCapsule of UEFI system partition

> > > >

> > > > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> > > >    (Please follow README.dfu for details.)

> > > >

> > > >   ==> env set dfu_alt_info '...'

> > > >

> > > > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> > > >

> > > >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> > > >

> > > > Patch structure

> > > > ===============

> > > > Patch#1-#4,#12: preparatory patches

> > > > Patch#5-#11,#13: main part of implementation

> > > > Patch#14-#15: utilities

> > > > Patch#16-#17: pytests

> > > > Patch#18: for sandbox test

> > > >

> > > > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> > > >

> > > > Prerequisite patches

> > > > ====================

> > > > None

> > > >

> > > > Test

> > > > ====

> > > > * passed all the pytests which are included in this patch series

> > > >   on sandbox build locally.

> > > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> > > >   "virt-make-fs" cannot be executed.

> > > 

> > > Dear Takahiro,

> > > 

> > > please, rebase your series on origin/master. A prior version of the

> > > first patches is already applied.

> > 

> > You can simply pick up and apply non-merged patches without problems

> > except a function prototype change of efi_create_indexed_name() you made,

> > which is not trivial to me.

> > 

> > But anyway, I will post a clean patch set soon.

> > 

> > > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> > > the remaining patches.

> > 

> > Where can we find the results?

> > I don't think there is no official information on those CI's.

> 

> If you submit a pull request against https://github.com/u-boot/u-boot

> Azure will run automatically and show the results.  We don't have


We can get a free account for Azure, but yet it requires us to give
our credit card information to Azure. I don't want to accept it.
So I believe requiring Azure test results is just inappropriate.

-Takahiro Akashi


> anything similar setup for GitLab, but since it uses the same Docker

> images as Azure, so long as the syntax is right (and there are linters

> if you're unsure of a change), it would be expected to work too.

> 

> -- 

> Tom
Tom Rini Nov. 17, 2020, 12:36 a.m. UTC | #5
On Tue, Nov 17, 2020 at 09:16:26AM +0900, AKASHI Takahiro wrote:
> On Mon, Nov 16, 2020 at 11:10:12AM -0500, Tom Rini wrote:

> > On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:

> > > Heinrich,

> > > 

> > > On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:

> > > > On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > > > > Summary

> > > > > =======

> > > > > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > > > > and its aim is to allow a caller (OS) to pass information to the firmware,

> > > > > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > > > > instructions from OS.

> > > > >

> > > > > While 'UpdateCapsule' is a runtime services function, it is, at least

> > > > > initially, supported only before exiting boot services alike other runtime

> > > > > functions, [Get/]SetVariable. This is because modifying storage which may

> > > > > be shared with OS must be carefully designed and there is no general

> > > > > assumption that we can do it.

> > > > >

> > > > > Therefore, we practically support only "capsule on disk"; any capsule can

> > > > > be handed over to UEFI subsystem as a file on a specific file system.

> > > > >

> > > > > In this patch series, all the related definitions and structures are given

> > > > > as UEFI specification describes, and basic framework for capsule support

> > > > > is provided. Currently supported is

> > > > >  * firmware update (Firmware Management Protocol or simply FMP)

> > > > >

> > > > > Most of functionality of firmware update is provided by FMP driver and

> > > > > it can be, by nature, system/platform-specific. So you can and should

> > > > > implement your own FMP driver(s) based on your system requirements.

> > > > > Under the current implementation, we provide two basic but generic

> > > > > drivers with two formats:

> > > > >   * FIT image format (as used in TFTP update and dfu)

> > > > >   * raw image format

> > > > >

> > > > > It's totally up to users which one, or both, should be used on users'

> > > > > system depending on user requirements.

> > > > >

> > > > > Quick usage

> > > > > ===========

> > > > > 1. You can create a capsule file with the following host command:

> > > > >

> > > > >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> > > > >

> > > > > 2. Put the file under:

> > > > >

> > > > >   /EFI/UpdateCapsule of UEFI system partition

> > > > >

> > > > > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> > > > >    (Please follow README.dfu for details.)

> > > > >

> > > > >   ==> env set dfu_alt_info '...'

> > > > >

> > > > > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> > > > >

> > > > >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> > > > >

> > > > > Patch structure

> > > > > ===============

> > > > > Patch#1-#4,#12: preparatory patches

> > > > > Patch#5-#11,#13: main part of implementation

> > > > > Patch#14-#15: utilities

> > > > > Patch#16-#17: pytests

> > > > > Patch#18: for sandbox test

> > > > >

> > > > > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> > > > >

> > > > > Prerequisite patches

> > > > > ====================

> > > > > None

> > > > >

> > > > > Test

> > > > > ====

> > > > > * passed all the pytests which are included in this patch series

> > > > >   on sandbox build locally.

> > > > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> > > > >   "virt-make-fs" cannot be executed.

> > > > 

> > > > Dear Takahiro,

> > > > 

> > > > please, rebase your series on origin/master. A prior version of the

> > > > first patches is already applied.

> > > 

> > > You can simply pick up and apply non-merged patches without problems

> > > except a function prototype change of efi_create_indexed_name() you made,

> > > which is not trivial to me.

> > > 

> > > But anyway, I will post a clean patch set soon.

> > > 

> > > > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> > > > the remaining patches.

> > > 

> > > Where can we find the results?

> > > I don't think there is no official information on those CI's.

> > 

> > If you submit a pull request against https://github.com/u-boot/u-boot

> > Azure will run automatically and show the results.  We don't have

> 

> We can get a free account for Azure, but yet it requires us to give

> our credit card information to Azure. I don't want to accept it.

> So I believe requiring Azure test results is just inappropriate.


That's not correct, as far as I can tell.  You just need to submit a
pull request against the repository I mentioned above and that triggers
one.  For example: https://github.com/u-boot/u-boot/pull/35/checks is
the result of one such PR.

-- 
Tom
AKASHI Takahiro Nov. 17, 2020, 12:44 a.m. UTC | #6
Hi Tom,

On Mon, Nov 16, 2020 at 07:36:26PM -0500, Tom Rini wrote:
> On Tue, Nov 17, 2020 at 09:16:26AM +0900, AKASHI Takahiro wrote:

> > On Mon, Nov 16, 2020 at 11:10:12AM -0500, Tom Rini wrote:

> > > On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:

> > > > Heinrich,

> > > > 

> > > > On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:

> > > > > On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > > > > > Summary

> > > > > > =======

> > > > > > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > > > > > and its aim is to allow a caller (OS) to pass information to the firmware,

> > > > > > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > > > > > instructions from OS.

> > > > > >

> > > > > > While 'UpdateCapsule' is a runtime services function, it is, at least

> > > > > > initially, supported only before exiting boot services alike other runtime

> > > > > > functions, [Get/]SetVariable. This is because modifying storage which may

> > > > > > be shared with OS must be carefully designed and there is no general

> > > > > > assumption that we can do it.

> > > > > >

> > > > > > Therefore, we practically support only "capsule on disk"; any capsule can

> > > > > > be handed over to UEFI subsystem as a file on a specific file system.

> > > > > >

> > > > > > In this patch series, all the related definitions and structures are given

> > > > > > as UEFI specification describes, and basic framework for capsule support

> > > > > > is provided. Currently supported is

> > > > > >  * firmware update (Firmware Management Protocol or simply FMP)

> > > > > >

> > > > > > Most of functionality of firmware update is provided by FMP driver and

> > > > > > it can be, by nature, system/platform-specific. So you can and should

> > > > > > implement your own FMP driver(s) based on your system requirements.

> > > > > > Under the current implementation, we provide two basic but generic

> > > > > > drivers with two formats:

> > > > > >   * FIT image format (as used in TFTP update and dfu)

> > > > > >   * raw image format

> > > > > >

> > > > > > It's totally up to users which one, or both, should be used on users'

> > > > > > system depending on user requirements.

> > > > > >

> > > > > > Quick usage

> > > > > > ===========

> > > > > > 1. You can create a capsule file with the following host command:

> > > > > >

> > > > > >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> > > > > >

> > > > > > 2. Put the file under:

> > > > > >

> > > > > >   /EFI/UpdateCapsule of UEFI system partition

> > > > > >

> > > > > > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> > > > > >    (Please follow README.dfu for details.)

> > > > > >

> > > > > >   ==> env set dfu_alt_info '...'

> > > > > >

> > > > > > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> > > > > >

> > > > > >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> > > > > >

> > > > > > Patch structure

> > > > > > ===============

> > > > > > Patch#1-#4,#12: preparatory patches

> > > > > > Patch#5-#11,#13: main part of implementation

> > > > > > Patch#14-#15: utilities

> > > > > > Patch#16-#17: pytests

> > > > > > Patch#18: for sandbox test

> > > > > >

> > > > > > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> > > > > >

> > > > > > Prerequisite patches

> > > > > > ====================

> > > > > > None

> > > > > >

> > > > > > Test

> > > > > > ====

> > > > > > * passed all the pytests which are included in this patch series

> > > > > >   on sandbox build locally.

> > > > > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> > > > > >   "virt-make-fs" cannot be executed.

> > > > > 

> > > > > Dear Takahiro,

> > > > > 

> > > > > please, rebase your series on origin/master. A prior version of the

> > > > > first patches is already applied.

> > > > 

> > > > You can simply pick up and apply non-merged patches without problems

> > > > except a function prototype change of efi_create_indexed_name() you made,

> > > > which is not trivial to me.

> > > > 

> > > > But anyway, I will post a clean patch set soon.

> > > > 

> > > > > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> > > > > the remaining patches.

> > > > 

> > > > Where can we find the results?

> > > > I don't think there is no official information on those CI's.

> > > 

> > > If you submit a pull request against https://github.com/u-boot/u-boot

> > > Azure will run automatically and show the results.  We don't have

> > 

> > We can get a free account for Azure, but yet it requires us to give

> > our credit card information to Azure. I don't want to accept it.

> > So I believe requiring Azure test results is just inappropriate.

> 

> That's not correct, as far as I can tell.  You just need to submit a

> pull request against the repository I mentioned above and that triggers

> one.  For example: https://github.com/u-boot/u-boot/pull/35/checks is

> the result of one such PR.


Okay, I will check and try.
But I hope that you should have, on the home page?, more documents
on those CI loops and requirements of test results as part of
submission process.

Thanks,
-Takahiro Akashi

> -- 

> Tom
Alper Nebi Yasak Nov. 17, 2020, 12:43 p.m. UTC | #7
On 17/11/2020 04:49, Jean Lucas wrote:
> Hello all,

> 

> On Pine64 RockPro64 and Pinebook Pro (both RK3399), flashing U-Boot 

> v2021.01-rc2-47-g9324c9a823 defconfig and mainline ATF 

> v2.4-rc0-2-gd01f31c03 to SPI flash of both devices results in a hang 

> shortly after loading the appropriate FDT when booting.

> 

> On a Pinebook Pro:

> 

> => load mmc 0:1 ${kernel_addr_r} Image

> 32418304 bytes read in 1448 ms (21.4 MiB/s)

> => load mmc 0:1 ${fdt_addr_r} dtbs/rockchip/rk3399-pinebook-pro.dtb

> 80831 bytes read in 27 ms (2.9 MiB/s)

> => load mmc 0:1 ${ramdisk_addr_r} initramfs-linux.img

> 29698825 bytes read in 1291 ms (21.9 MiB/s)

> => setenv bootargs console=ttyS2,1500000 console=tty0 

> root=/dev/ghost/root audit=0

> => booti ${kernel_addr_r} ${ramdisk_addr_r}:${filesize} ${fdt_addr_r}

> Moving Image from 0x2080000 to 0x2200000, end=41c0000

> ## Flattened Device Tree blob at 01f00000

>     Booting using the

> 

> Behavior is the same on RockPro64.

> 

> Worth mentioning is that U-Boot from about a week ago (I think 

> rc2-4-gf36603c7a8) with same mainline ATF version written to eMMC 

> results in a working boot on Pinebook Pro, so bug seems to be when 

> booting from SPI.

> 

> To further the hypothesis, on RockPro64, the latest U-Boot I can use 

> from SPI (defconfig) is release 2020.04, since later releases also hang 

> on loading the appropriate FDT when booting as described above.

> 

> Any ideas as to what could be causing the hangs?

> 


My gru-kevin was hanging at that same place when I tried to boot from
USB, try running "usb stop" to see if that hangs. If so, try disabling
CONFIG_USB_OHCI_HCD and CONFIG_USB_OHCI_GENERIC, which fixed the hang in
my case.
Peter Robinson Nov. 17, 2020, 2:11 p.m. UTC | #8
On Tue, Nov 17, 2020 at 12:43 PM Alper Nebi Yasak
<alpernebiyasak@gmail.com> wrote:
>

> On 17/11/2020 04:49, Jean Lucas wrote:

> > Hello all,

> >

> > On Pine64 RockPro64 and Pinebook Pro (both RK3399), flashing U-Boot

> > v2021.01-rc2-47-g9324c9a823 defconfig and mainline ATF

> > v2.4-rc0-2-gd01f31c03 to SPI flash of both devices results in a hang

> > shortly after loading the appropriate FDT when booting.

> >

> > On a Pinebook Pro:

> >

> > => load mmc 0:1 ${kernel_addr_r} Image

> > 32418304 bytes read in 1448 ms (21.4 MiB/s)

> > => load mmc 0:1 ${fdt_addr_r} dtbs/rockchip/rk3399-pinebook-pro.dtb

> > 80831 bytes read in 27 ms (2.9 MiB/s)

> > => load mmc 0:1 ${ramdisk_addr_r} initramfs-linux.img

> > 29698825 bytes read in 1291 ms (21.9 MiB/s)

> > => setenv bootargs console=ttyS2,1500000 console=tty0

> > root=/dev/ghost/root audit=0

> > => booti ${kernel_addr_r} ${ramdisk_addr_r}:${filesize} ${fdt_addr_r}

> > Moving Image from 0x2080000 to 0x2200000, end=41c0000

> > ## Flattened Device Tree blob at 01f00000

> >     Booting using the

> >

> > Behavior is the same on RockPro64.

> >

> > Worth mentioning is that U-Boot from about a week ago (I think

> > rc2-4-gf36603c7a8) with same mainline ATF version written to eMMC

> > results in a working boot on Pinebook Pro, so bug seems to be when

> > booting from SPI.

> >

> > To further the hypothesis, on RockPro64, the latest U-Boot I can use

> > from SPI (defconfig) is release 2020.04, since later releases also hang

> > on loading the appropriate FDT when booting as described above.

> >

> > Any ideas as to what could be causing the hangs?

> >

>

> My gru-kevin was hanging at that same place when I tried to boot from

> USB, try running "usb stop" to see if that hangs. If so, try disabling

> CONFIG_USB_OHCI_HCD and CONFIG_USB_OHCI_GENERIC, which fixed the hang in

> my case.


Which probably breaks USB keyboards?
Alper Nebi Yasak Nov. 17, 2020, 2:42 p.m. UTC | #9
On 17/11/2020 17:11, Peter Robinson wrote:
> On Tue, Nov 17, 2020 at 12:43 PM Alper Nebi Yasak

> <alpernebiyasak@gmail.com> wrote:

>>

>> On 17/11/2020 04:49, Jean Lucas wrote:

>>> Hello all,

>>>

>>> On Pine64 RockPro64 and Pinebook Pro (both RK3399), flashing U-Boot

>>> v2021.01-rc2-47-g9324c9a823 defconfig and mainline ATF

>>> v2.4-rc0-2-gd01f31c03 to SPI flash of both devices results in a hang

>>> shortly after loading the appropriate FDT when booting.

>>>

>>> On a Pinebook Pro:

>>>

>>> => load mmc 0:1 ${kernel_addr_r} Image

>>> 32418304 bytes read in 1448 ms (21.4 MiB/s)

>>> => load mmc 0:1 ${fdt_addr_r} dtbs/rockchip/rk3399-pinebook-pro.dtb

>>> 80831 bytes read in 27 ms (2.9 MiB/s)

>>> => load mmc 0:1 ${ramdisk_addr_r} initramfs-linux.img

>>> 29698825 bytes read in 1291 ms (21.9 MiB/s)

>>> => setenv bootargs console=ttyS2,1500000 console=tty0

>>> root=/dev/ghost/root audit=0

>>> => booti ${kernel_addr_r} ${ramdisk_addr_r}:${filesize} ${fdt_addr_r}

>>> Moving Image from 0x2080000 to 0x2200000, end=41c0000

>>> ## Flattened Device Tree blob at 01f00000

>>>     Booting using the

>>>

>>> Behavior is the same on RockPro64.

>>>

>>> Worth mentioning is that U-Boot from about a week ago (I think

>>> rc2-4-gf36603c7a8) with same mainline ATF version written to eMMC

>>> results in a working boot on Pinebook Pro, so bug seems to be when

>>> booting from SPI.

>>>

>>> To further the hypothesis, on RockPro64, the latest U-Boot I can use

>>> from SPI (defconfig) is release 2020.04, since later releases also hang

>>> on loading the appropriate FDT when booting as described above.

>>>

>>> Any ideas as to what could be causing the hangs?

>>>

>>

>> My gru-kevin was hanging at that same place when I tried to boot from

>> USB, try running "usb stop" to see if that hangs. If so, try disabling

>> CONFIG_USB_OHCI_HCD and CONFIG_USB_OHCI_GENERIC, which fixed the hang in

>> my case.

> 

> Which probably breaks USB keyboards?


I wouldn't know, gru-kevin uses cros-ec-keyb so I never needed to try a
USB keyboard. I meant those as steps to narrow things down.
Jean Lucas Nov. 17, 2020, 6:28 p.m. UTC | #10
On 11/17/20 9:42 AM, Alper Nebi Yasak wrote:
> On 17/11/2020 17:11, Peter Robinson wrote:

>> On Tue, Nov 17, 2020 at 12:43 PM Alper Nebi Yasak

>> <alpernebiyasak@gmail.com> wrote:

>>> On 17/11/2020 04:49, Jean Lucas wrote:

>>>> Hello all,

>>>>

>>>> On Pine64 RockPro64 and Pinebook Pro (both RK3399), flashing U-Boot

>>>> v2021.01-rc2-47-g9324c9a823 defconfig and mainline ATF

>>>> v2.4-rc0-2-gd01f31c03 to SPI flash of both devices results in a hang

>>>> shortly after loading the appropriate FDT when booting.

>>>>

>>>> On a Pinebook Pro:

>>>>

>>>> => load mmc 0:1 ${kernel_addr_r} Image

>>>> 32418304 bytes read in 1448 ms (21.4 MiB/s)

>>>> => load mmc 0:1 ${fdt_addr_r} dtbs/rockchip/rk3399-pinebook-pro.dtb

>>>> 80831 bytes read in 27 ms (2.9 MiB/s)

>>>> => load mmc 0:1 ${ramdisk_addr_r} initramfs-linux.img

>>>> 29698825 bytes read in 1291 ms (21.9 MiB/s)

>>>> => setenv bootargs console=ttyS2,1500000 console=tty0

>>>> root=/dev/ghost/root audit=0

>>>> => booti ${kernel_addr_r} ${ramdisk_addr_r}:${filesize} ${fdt_addr_r}

>>>> Moving Image from 0x2080000 to 0x2200000, end=41c0000

>>>> ## Flattened Device Tree blob at 01f00000

>>>>      Booting using the

>>>>

>>>> Behavior is the same on RockPro64.

>>>>

>>>> Worth mentioning is that U-Boot from about a week ago (I think

>>>> rc2-4-gf36603c7a8) with same mainline ATF version written to eMMC

>>>> results in a working boot on Pinebook Pro, so bug seems to be when

>>>> booting from SPI.

>>>>

>>>> To further the hypothesis, on RockPro64, the latest U-Boot I can use

>>>> from SPI (defconfig) is release 2020.04, since later releases also hang

>>>> on loading the appropriate FDT when booting as described above.

>>>>

>>>> Any ideas as to what could be causing the hangs?

>>>>

>>> My gru-kevin was hanging at that same place when I tried to boot from

>>> USB, try running "usb stop" to see if that hangs. If so, try disabling

>>> CONFIG_USB_OHCI_HCD and CONFIG_USB_OHCI_GENERIC, which fixed the hang in

>>> my case.

>> Which probably breaks USB keyboards?

> I wouldn't know, gru-kevin uses cros-ec-keyb so I never needed to try a

> USB keyboard. I meant those as steps to narrow things down.


So, running "usb stop" resulted in a hang.

I will test a build with the OHCI options Alper mentioned disabled, this 
evening.

As far as testing goes, disabling USB is fine since I can access the 
machines over serial.
AKASHI Takahiro Nov. 17, 2020, 11:50 p.m. UTC | #11
Tom, Heinrich,

On Tue, Nov 17, 2020 at 09:44:36AM +0900, AKASHI Takahiro wrote:
> Hi Tom,

> 

> On Mon, Nov 16, 2020 at 07:36:26PM -0500, Tom Rini wrote:

> > On Tue, Nov 17, 2020 at 09:16:26AM +0900, AKASHI Takahiro wrote:

> > > On Mon, Nov 16, 2020 at 11:10:12AM -0500, Tom Rini wrote:

> > > > On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:

> > > > > Heinrich,

> > > > > 

> > > > > On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:

> > > > > > On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > > > > > > Summary

> > > > > > > =======

> > > > > > > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > > > > > > and its aim is to allow a caller (OS) to pass information to the firmware,

> > > > > > > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > > > > > > instructions from OS.

> > > > > > >

> > > > > > > While 'UpdateCapsule' is a runtime services function, it is, at least

> > > > > > > initially, supported only before exiting boot services alike other runtime

> > > > > > > functions, [Get/]SetVariable. This is because modifying storage which may

> > > > > > > be shared with OS must be carefully designed and there is no general

> > > > > > > assumption that we can do it.

> > > > > > >

> > > > > > > Therefore, we practically support only "capsule on disk"; any capsule can

> > > > > > > be handed over to UEFI subsystem as a file on a specific file system.

> > > > > > >

> > > > > > > In this patch series, all the related definitions and structures are given

> > > > > > > as UEFI specification describes, and basic framework for capsule support

> > > > > > > is provided. Currently supported is

> > > > > > >  * firmware update (Firmware Management Protocol or simply FMP)

> > > > > > >

> > > > > > > Most of functionality of firmware update is provided by FMP driver and

> > > > > > > it can be, by nature, system/platform-specific. So you can and should

> > > > > > > implement your own FMP driver(s) based on your system requirements.

> > > > > > > Under the current implementation, we provide two basic but generic

> > > > > > > drivers with two formats:

> > > > > > >   * FIT image format (as used in TFTP update and dfu)

> > > > > > >   * raw image format

> > > > > > >

> > > > > > > It's totally up to users which one, or both, should be used on users'

> > > > > > > system depending on user requirements.

> > > > > > >

> > > > > > > Quick usage

> > > > > > > ===========

> > > > > > > 1. You can create a capsule file with the following host command:

> > > > > > >

> > > > > > >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> > > > > > >

> > > > > > > 2. Put the file under:

> > > > > > >

> > > > > > >   /EFI/UpdateCapsule of UEFI system partition

> > > > > > >

> > > > > > > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> > > > > > >    (Please follow README.dfu for details.)

> > > > > > >

> > > > > > >   ==> env set dfu_alt_info '...'

> > > > > > >

> > > > > > > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> > > > > > >

> > > > > > >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> > > > > > >

> > > > > > > Patch structure

> > > > > > > ===============

> > > > > > > Patch#1-#4,#12: preparatory patches

> > > > > > > Patch#5-#11,#13: main part of implementation

> > > > > > > Patch#14-#15: utilities

> > > > > > > Patch#16-#17: pytests

> > > > > > > Patch#18: for sandbox test

> > > > > > >

> > > > > > > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> > > > > > >

> > > > > > > Prerequisite patches

> > > > > > > ====================

> > > > > > > None

> > > > > > >

> > > > > > > Test

> > > > > > > ====

> > > > > > > * passed all the pytests which are included in this patch series

> > > > > > >   on sandbox build locally.

> > > > > > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> > > > > > >   "virt-make-fs" cannot be executed.

> > > > > > 

> > > > > > Dear Takahiro,

> > > > > > 

> > > > > > please, rebase your series on origin/master. A prior version of the

> > > > > > first patches is already applied.

> > > > > 

> > > > > You can simply pick up and apply non-merged patches without problems

> > > > > except a function prototype change of efi_create_indexed_name() you made,

> > > > > which is not trivial to me.

> > > > > 

> > > > > But anyway, I will post a clean patch set soon.

> > > > > 

> > > > > > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> > > > > > the remaining patches.


@Heinrich,

https://travis-ci.org/github/t-akashi/u-boot-for-travis/builds/743837907
https://github.com/u-boot/u-boot/pull/39

Do those results satisfy your request?
I don't know how I can deal with gitlab and amazon CI.

> > > > > Where can we find the results?

> > > > > I don't think there is no official information on those CI's.

> > > > 

> > > > If you submit a pull request against https://github.com/u-boot/u-boot

> > > > Azure will run automatically and show the results.  We don't have

> > > 

> > > We can get a free account for Azure, but yet it requires us to give

> > > our credit card information to Azure. I don't want to accept it.

> > > So I believe requiring Azure test results is just inappropriate.

> > 

> > That's not correct, as far as I can tell.  You just need to submit a

> > pull request against the repository I mentioned above and that triggers

> > one.  For example: https://github.com/u-boot/u-boot/pull/35/checks is

> > the result of one such PR.

> 

> Okay, I will check and try.

> But I hope that you should have, on the home page?, more documents

> on those CI loops and requirements of test results as part of

> submission process.


@Tom,

Again, I'd like you to add a rule and a related document with regard to
criteria for "testing before submission". Otherwise, it's confusing.

@Heinrich,

As I requested several times, please clarify your view on virt-make-fs/sudo
issue and have consensus on it with Tom.

-Takahiro Akashi

> 

> Thanks,

> -Takahiro Akashi

> 

> > -- 

> > Tom

> 

>
Tom Rini Nov. 17, 2020, 11:59 p.m. UTC | #12
On Wed, Nov 18, 2020 at 08:50:08AM +0900, AKASHI Takahiro wrote:
> Tom, Heinrich,

> 

> On Tue, Nov 17, 2020 at 09:44:36AM +0900, AKASHI Takahiro wrote:

> > Hi Tom,

> > 

> > On Mon, Nov 16, 2020 at 07:36:26PM -0500, Tom Rini wrote:

> > > On Tue, Nov 17, 2020 at 09:16:26AM +0900, AKASHI Takahiro wrote:

> > > > On Mon, Nov 16, 2020 at 11:10:12AM -0500, Tom Rini wrote:

> > > > > On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:

> > > > > > Heinrich,

> > > > > > 

> > > > > > On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:

> > > > > > > On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > > > > > > > Summary

> > > > > > > > =======

> > > > > > > > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > > > > > > > and its aim is to allow a caller (OS) to pass information to the firmware,

> > > > > > > > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > > > > > > > instructions from OS.

> > > > > > > >

> > > > > > > > While 'UpdateCapsule' is a runtime services function, it is, at least

> > > > > > > > initially, supported only before exiting boot services alike other runtime

> > > > > > > > functions, [Get/]SetVariable. This is because modifying storage which may

> > > > > > > > be shared with OS must be carefully designed and there is no general

> > > > > > > > assumption that we can do it.

> > > > > > > >

> > > > > > > > Therefore, we practically support only "capsule on disk"; any capsule can

> > > > > > > > be handed over to UEFI subsystem as a file on a specific file system.

> > > > > > > >

> > > > > > > > In this patch series, all the related definitions and structures are given

> > > > > > > > as UEFI specification describes, and basic framework for capsule support

> > > > > > > > is provided. Currently supported is

> > > > > > > >  * firmware update (Firmware Management Protocol or simply FMP)

> > > > > > > >

> > > > > > > > Most of functionality of firmware update is provided by FMP driver and

> > > > > > > > it can be, by nature, system/platform-specific. So you can and should

> > > > > > > > implement your own FMP driver(s) based on your system requirements.

> > > > > > > > Under the current implementation, we provide two basic but generic

> > > > > > > > drivers with two formats:

> > > > > > > >   * FIT image format (as used in TFTP update and dfu)

> > > > > > > >   * raw image format

> > > > > > > >

> > > > > > > > It's totally up to users which one, or both, should be used on users'

> > > > > > > > system depending on user requirements.

> > > > > > > >

> > > > > > > > Quick usage

> > > > > > > > ===========

> > > > > > > > 1. You can create a capsule file with the following host command:

> > > > > > > >

> > > > > > > >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> > > > > > > >

> > > > > > > > 2. Put the file under:

> > > > > > > >

> > > > > > > >   /EFI/UpdateCapsule of UEFI system partition

> > > > > > > >

> > > > > > > > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> > > > > > > >    (Please follow README.dfu for details.)

> > > > > > > >

> > > > > > > >   ==> env set dfu_alt_info '...'

> > > > > > > >

> > > > > > > > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> > > > > > > >

> > > > > > > >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> > > > > > > >

> > > > > > > > Patch structure

> > > > > > > > ===============

> > > > > > > > Patch#1-#4,#12: preparatory patches

> > > > > > > > Patch#5-#11,#13: main part of implementation

> > > > > > > > Patch#14-#15: utilities

> > > > > > > > Patch#16-#17: pytests

> > > > > > > > Patch#18: for sandbox test

> > > > > > > >

> > > > > > > > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> > > > > > > >

> > > > > > > > Prerequisite patches

> > > > > > > > ====================

> > > > > > > > None

> > > > > > > >

> > > > > > > > Test

> > > > > > > > ====

> > > > > > > > * passed all the pytests which are included in this patch series

> > > > > > > >   on sandbox build locally.

> > > > > > > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> > > > > > > >   "virt-make-fs" cannot be executed.

> > > > > > > 

> > > > > > > Dear Takahiro,

> > > > > > > 

> > > > > > > please, rebase your series on origin/master. A prior version of the

> > > > > > > first patches is already applied.

> > > > > > 

> > > > > > You can simply pick up and apply non-merged patches without problems

> > > > > > except a function prototype change of efi_create_indexed_name() you made,

> > > > > > which is not trivial to me.

> > > > > > 

> > > > > > But anyway, I will post a clean patch set soon.

> > > > > > 

> > > > > > > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> > > > > > > the remaining patches.

> 

> @Heinrich,

> 

> https://travis-ci.org/github/t-akashi/u-boot-for-travis/builds/743837907

> https://github.com/u-boot/u-boot/pull/39

> 

> Do those results satisfy your request?

> I don't know how I can deal with gitlab and amazon CI.


I believe that was a typo of "Azure", which you have above.  And since
https://github.com/u-boot/u-boot/pull/39/checks shows green, I would
expect that so long as you linted your .gitlab-ci.yml changes, if any,
then it too should pass.

> > > > > > Where can we find the results?

> > > > > > I don't think there is no official information on those CI's.

> > > > > 

> > > > > If you submit a pull request against https://github.com/u-boot/u-boot

> > > > > Azure will run automatically and show the results.  We don't have

> > > > 

> > > > We can get a free account for Azure, but yet it requires us to give

> > > > our credit card information to Azure. I don't want to accept it.

> > > > So I believe requiring Azure test results is just inappropriate.

> > > 

> > > That's not correct, as far as I can tell.  You just need to submit a

> > > pull request against the repository I mentioned above and that triggers

> > > one.  For example: https://github.com/u-boot/u-boot/pull/35/checks is

> > > the result of one such PR.

> > 

> > Okay, I will check and try.

> > But I hope that you should have, on the home page?, more documents

> > on those CI loops and requirements of test results as part of

> > submission process.

> 

> @Tom,

> 

> Again, I'd like you to add a rule and a related document with regard to

> criteria for "testing before submission". Otherwise, it's confusing.


I'm sorry you found it confusing.  It has been stated many times in
these threads that since you're adding tests, they're expected to not
break CI, and should even run when possible.  Updating the rST with a CI
section would be good, yes.

> @Heinrich,

> 

> As I requested several times, please clarify your view on virt-make-fs/sudo

> issue and have consensus on it with Tom.


Yes, please.

-- 
Tom
AKASHI Takahiro Nov. 18, 2020, 12:26 a.m. UTC | #13
On Tue, Nov 17, 2020 at 06:59:52PM -0500, Tom Rini wrote:
> On Wed, Nov 18, 2020 at 08:50:08AM +0900, AKASHI Takahiro wrote:

> > Tom, Heinrich,

> > 

> > On Tue, Nov 17, 2020 at 09:44:36AM +0900, AKASHI Takahiro wrote:

> > > Hi Tom,

> > > 

> > > On Mon, Nov 16, 2020 at 07:36:26PM -0500, Tom Rini wrote:

> > > > On Tue, Nov 17, 2020 at 09:16:26AM +0900, AKASHI Takahiro wrote:

> > > > > On Mon, Nov 16, 2020 at 11:10:12AM -0500, Tom Rini wrote:

> > > > > > On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:

> > > > > > > Heinrich,

> > > > > > > 

> > > > > > > On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:

> > > > > > > > On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > > > > > > > > Summary

> > > > > > > > > =======

> > > > > > > > > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > > > > > > > > and its aim is to allow a caller (OS) to pass information to the firmware,

> > > > > > > > > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > > > > > > > > instructions from OS.

> > > > > > > > >

> > > > > > > > > While 'UpdateCapsule' is a runtime services function, it is, at least

> > > > > > > > > initially, supported only before exiting boot services alike other runtime

> > > > > > > > > functions, [Get/]SetVariable. This is because modifying storage which may

> > > > > > > > > be shared with OS must be carefully designed and there is no general

> > > > > > > > > assumption that we can do it.

> > > > > > > > >

> > > > > > > > > Therefore, we practically support only "capsule on disk"; any capsule can

> > > > > > > > > be handed over to UEFI subsystem as a file on a specific file system.

> > > > > > > > >

> > > > > > > > > In this patch series, all the related definitions and structures are given

> > > > > > > > > as UEFI specification describes, and basic framework for capsule support

> > > > > > > > > is provided. Currently supported is

> > > > > > > > >  * firmware update (Firmware Management Protocol or simply FMP)

> > > > > > > > >

> > > > > > > > > Most of functionality of firmware update is provided by FMP driver and

> > > > > > > > > it can be, by nature, system/platform-specific. So you can and should

> > > > > > > > > implement your own FMP driver(s) based on your system requirements.

> > > > > > > > > Under the current implementation, we provide two basic but generic

> > > > > > > > > drivers with two formats:

> > > > > > > > >   * FIT image format (as used in TFTP update and dfu)

> > > > > > > > >   * raw image format

> > > > > > > > >

> > > > > > > > > It's totally up to users which one, or both, should be used on users'

> > > > > > > > > system depending on user requirements.

> > > > > > > > >

> > > > > > > > > Quick usage

> > > > > > > > > ===========

> > > > > > > > > 1. You can create a capsule file with the following host command:

> > > > > > > > >

> > > > > > > > >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> > > > > > > > >

> > > > > > > > > 2. Put the file under:

> > > > > > > > >

> > > > > > > > >   /EFI/UpdateCapsule of UEFI system partition

> > > > > > > > >

> > > > > > > > > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> > > > > > > > >    (Please follow README.dfu for details.)

> > > > > > > > >

> > > > > > > > >   ==> env set dfu_alt_info '...'

> > > > > > > > >

> > > > > > > > > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> > > > > > > > >

> > > > > > > > >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> > > > > > > > >

> > > > > > > > > Patch structure

> > > > > > > > > ===============

> > > > > > > > > Patch#1-#4,#12: preparatory patches

> > > > > > > > > Patch#5-#11,#13: main part of implementation

> > > > > > > > > Patch#14-#15: utilities

> > > > > > > > > Patch#16-#17: pytests

> > > > > > > > > Patch#18: for sandbox test

> > > > > > > > >

> > > > > > > > > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> > > > > > > > >

> > > > > > > > > Prerequisite patches

> > > > > > > > > ====================

> > > > > > > > > None

> > > > > > > > >

> > > > > > > > > Test

> > > > > > > > > ====

> > > > > > > > > * passed all the pytests which are included in this patch series

> > > > > > > > >   on sandbox build locally.

> > > > > > > > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> > > > > > > > >   "virt-make-fs" cannot be executed.

> > > > > > > > 

> > > > > > > > Dear Takahiro,

> > > > > > > > 

> > > > > > > > please, rebase your series on origin/master. A prior version of the

> > > > > > > > first patches is already applied.

> > > > > > > 

> > > > > > > You can simply pick up and apply non-merged patches without problems

> > > > > > > except a function prototype change of efi_create_indexed_name() you made,

> > > > > > > which is not trivial to me.

> > > > > > > 

> > > > > > > But anyway, I will post a clean patch set soon.

> > > > > > > 

> > > > > > > > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> > > > > > > > the remaining patches.

> > 

> > @Heinrich,

> > 

> > https://travis-ci.org/github/t-akashi/u-boot-for-travis/builds/743837907

> > https://github.com/u-boot/u-boot/pull/39

> > 

> > Do those results satisfy your request?

> > I don't know how I can deal with gitlab and amazon CI.

> 

> I believe that was a typo of "Azure", which you have above.  And since

> https://github.com/u-boot/u-boot/pull/39/checks shows green, I would

> expect that so long as you linted your .gitlab-ci.yml changes, if any,

> then it too should pass.

> 

> > > > > > > Where can we find the results?

> > > > > > > I don't think there is no official information on those CI's.

> > > > > > 

> > > > > > If you submit a pull request against https://github.com/u-boot/u-boot

> > > > > > Azure will run automatically and show the results.  We don't have

> > > > > 

> > > > > We can get a free account for Azure, but yet it requires us to give

> > > > > our credit card information to Azure. I don't want to accept it.

> > > > > So I believe requiring Azure test results is just inappropriate.

> > > > 

> > > > That's not correct, as far as I can tell.  You just need to submit a

> > > > pull request against the repository I mentioned above and that triggers

> > > > one.  For example: https://github.com/u-boot/u-boot/pull/35/checks is

> > > > the result of one such PR.

> > > 

> > > Okay, I will check and try.

> > > But I hope that you should have, on the home page?, more documents

> > > on those CI loops and requirements of test results as part of

> > > submission process.

> > 

> > @Tom,

> > 

> > Again, I'd like you to add a rule and a related document with regard to

> > criteria for "testing before submission". Otherwise, it's confusing.

> 

> I'm sorry you found it confusing.  It has been stated many times in

> these threads that since you're adding tests, they're expected to not

> break CI, and should even run when possible.  Updating the rST with a CI

> section would be good, yes.


For clarity, my claim above is not only for me, but also for "all"
contributors.
When I looked at
https://github.com/u-boot/u-boot/pulls
I was a bit disappointed as there are so few people who have actually
tried to invoke Azure CI (prior to their submissions?).
Heinrich requires CI results, but many custodians not.
Why this inconsistency?

-Takahiro Akashi


> > @Heinrich,

> > 

> > As I requested several times, please clarify your view on virt-make-fs/sudo

> > issue and have consensus on it with Tom.

> 

> Yes, please.

> 

> -- 

> Tom
Tom Rini Nov. 18, 2020, 3:02 a.m. UTC | #14
On Wed, Nov 18, 2020 at 09:26:38AM +0900, AKASHI Takahiro wrote:
> On Tue, Nov 17, 2020 at 06:59:52PM -0500, Tom Rini wrote:

> > On Wed, Nov 18, 2020 at 08:50:08AM +0900, AKASHI Takahiro wrote:

> > > Tom, Heinrich,

> > > 

> > > On Tue, Nov 17, 2020 at 09:44:36AM +0900, AKASHI Takahiro wrote:

> > > > Hi Tom,

> > > > 

> > > > On Mon, Nov 16, 2020 at 07:36:26PM -0500, Tom Rini wrote:

> > > > > On Tue, Nov 17, 2020 at 09:16:26AM +0900, AKASHI Takahiro wrote:

> > > > > > On Mon, Nov 16, 2020 at 11:10:12AM -0500, Tom Rini wrote:

> > > > > > > On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:

> > > > > > > > Heinrich,

> > > > > > > > 

> > > > > > > > On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:

> > > > > > > > > On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > > > > > > > > > Summary

> > > > > > > > > > =======

> > > > > > > > > > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > > > > > > > > > and its aim is to allow a caller (OS) to pass information to the firmware,

> > > > > > > > > > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > > > > > > > > > instructions from OS.

> > > > > > > > > >

> > > > > > > > > > While 'UpdateCapsule' is a runtime services function, it is, at least

> > > > > > > > > > initially, supported only before exiting boot services alike other runtime

> > > > > > > > > > functions, [Get/]SetVariable. This is because modifying storage which may

> > > > > > > > > > be shared with OS must be carefully designed and there is no general

> > > > > > > > > > assumption that we can do it.

> > > > > > > > > >

> > > > > > > > > > Therefore, we practically support only "capsule on disk"; any capsule can

> > > > > > > > > > be handed over to UEFI subsystem as a file on a specific file system.

> > > > > > > > > >

> > > > > > > > > > In this patch series, all the related definitions and structures are given

> > > > > > > > > > as UEFI specification describes, and basic framework for capsule support

> > > > > > > > > > is provided. Currently supported is

> > > > > > > > > >  * firmware update (Firmware Management Protocol or simply FMP)

> > > > > > > > > >

> > > > > > > > > > Most of functionality of firmware update is provided by FMP driver and

> > > > > > > > > > it can be, by nature, system/platform-specific. So you can and should

> > > > > > > > > > implement your own FMP driver(s) based on your system requirements.

> > > > > > > > > > Under the current implementation, we provide two basic but generic

> > > > > > > > > > drivers with two formats:

> > > > > > > > > >   * FIT image format (as used in TFTP update and dfu)

> > > > > > > > > >   * raw image format

> > > > > > > > > >

> > > > > > > > > > It's totally up to users which one, or both, should be used on users'

> > > > > > > > > > system depending on user requirements.

> > > > > > > > > >

> > > > > > > > > > Quick usage

> > > > > > > > > > ===========

> > > > > > > > > > 1. You can create a capsule file with the following host command:

> > > > > > > > > >

> > > > > > > > > >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> > > > > > > > > >

> > > > > > > > > > 2. Put the file under:

> > > > > > > > > >

> > > > > > > > > >   /EFI/UpdateCapsule of UEFI system partition

> > > > > > > > > >

> > > > > > > > > > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> > > > > > > > > >    (Please follow README.dfu for details.)

> > > > > > > > > >

> > > > > > > > > >   ==> env set dfu_alt_info '...'

> > > > > > > > > >

> > > > > > > > > > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> > > > > > > > > >

> > > > > > > > > >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> > > > > > > > > >

> > > > > > > > > > Patch structure

> > > > > > > > > > ===============

> > > > > > > > > > Patch#1-#4,#12: preparatory patches

> > > > > > > > > > Patch#5-#11,#13: main part of implementation

> > > > > > > > > > Patch#14-#15: utilities

> > > > > > > > > > Patch#16-#17: pytests

> > > > > > > > > > Patch#18: for sandbox test

> > > > > > > > > >

> > > > > > > > > > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> > > > > > > > > >

> > > > > > > > > > Prerequisite patches

> > > > > > > > > > ====================

> > > > > > > > > > None

> > > > > > > > > >

> > > > > > > > > > Test

> > > > > > > > > > ====

> > > > > > > > > > * passed all the pytests which are included in this patch series

> > > > > > > > > >   on sandbox build locally.

> > > > > > > > > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> > > > > > > > > >   "virt-make-fs" cannot be executed.

> > > > > > > > > 

> > > > > > > > > Dear Takahiro,

> > > > > > > > > 

> > > > > > > > > please, rebase your series on origin/master. A prior version of the

> > > > > > > > > first patches is already applied.

> > > > > > > > 

> > > > > > > > You can simply pick up and apply non-merged patches without problems

> > > > > > > > except a function prototype change of efi_create_indexed_name() you made,

> > > > > > > > which is not trivial to me.

> > > > > > > > 

> > > > > > > > But anyway, I will post a clean patch set soon.

> > > > > > > > 

> > > > > > > > > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> > > > > > > > > the remaining patches.

> > > 

> > > @Heinrich,

> > > 

> > > https://travis-ci.org/github/t-akashi/u-boot-for-travis/builds/743837907

> > > https://github.com/u-boot/u-boot/pull/39

> > > 

> > > Do those results satisfy your request?

> > > I don't know how I can deal with gitlab and amazon CI.

> > 

> > I believe that was a typo of "Azure", which you have above.  And since

> > https://github.com/u-boot/u-boot/pull/39/checks shows green, I would

> > expect that so long as you linted your .gitlab-ci.yml changes, if any,

> > then it too should pass.

> > 

> > > > > > > > Where can we find the results?

> > > > > > > > I don't think there is no official information on those CI's.

> > > > > > > 

> > > > > > > If you submit a pull request against https://github.com/u-boot/u-boot

> > > > > > > Azure will run automatically and show the results.  We don't have

> > > > > > 

> > > > > > We can get a free account for Azure, but yet it requires us to give

> > > > > > our credit card information to Azure. I don't want to accept it.

> > > > > > So I believe requiring Azure test results is just inappropriate.

> > > > > 

> > > > > That's not correct, as far as I can tell.  You just need to submit a

> > > > > pull request against the repository I mentioned above and that triggers

> > > > > one.  For example: https://github.com/u-boot/u-boot/pull/35/checks is

> > > > > the result of one such PR.

> > > > 

> > > > Okay, I will check and try.

> > > > But I hope that you should have, on the home page?, more documents

> > > > on those CI loops and requirements of test results as part of

> > > > submission process.

> > > 

> > > @Tom,

> > > 

> > > Again, I'd like you to add a rule and a related document with regard to

> > > criteria for "testing before submission". Otherwise, it's confusing.

> > 

> > I'm sorry you found it confusing.  It has been stated many times in

> > these threads that since you're adding tests, they're expected to not

> > break CI, and should even run when possible.  Updating the rST with a CI

> > section would be good, yes.

> 

> For clarity, my claim above is not only for me, but also for "all"

> contributors.

> When I looked at

> https://github.com/u-boot/u-boot/pulls

> I was a bit disappointed as there are so few people who have actually

> tried to invoke Azure CI (prior to their submissions?).

> Heinrich requires CI results, but many custodians not.

> Why this inconsistency?


Given that it takes Travis-CI several hours to complete a run, and Azure
around 2 hours, most of the time it's fine the CI runs to get put off
until the PR is being made.  In _this_ case you're adding tests, and
they had previously caused CI to fail, so you needed to get CI to work.

-- 
Tom
AKASHI Takahiro Nov. 18, 2020, 3:11 a.m. UTC | #15
On Tue, Nov 17, 2020 at 10:02:41PM -0500, Tom Rini wrote:
> On Wed, Nov 18, 2020 at 09:26:38AM +0900, AKASHI Takahiro wrote:

> > On Tue, Nov 17, 2020 at 06:59:52PM -0500, Tom Rini wrote:

> > > On Wed, Nov 18, 2020 at 08:50:08AM +0900, AKASHI Takahiro wrote:

> > > > Tom, Heinrich,

> > > > 

> > > > On Tue, Nov 17, 2020 at 09:44:36AM +0900, AKASHI Takahiro wrote:

> > > > > Hi Tom,

> > > > > 

> > > > > On Mon, Nov 16, 2020 at 07:36:26PM -0500, Tom Rini wrote:

> > > > > > On Tue, Nov 17, 2020 at 09:16:26AM +0900, AKASHI Takahiro wrote:

> > > > > > > On Mon, Nov 16, 2020 at 11:10:12AM -0500, Tom Rini wrote:

> > > > > > > > On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:

> > > > > > > > > Heinrich,

> > > > > > > > > 

> > > > > > > > > On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:

> > > > > > > > > > On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > > > > > > > > > > Summary

> > > > > > > > > > > =======

> > > > > > > > > > > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > > > > > > > > > > and its aim is to allow a caller (OS) to pass information to the firmware,

> > > > > > > > > > > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > > > > > > > > > > instructions from OS.

> > > > > > > > > > >

> > > > > > > > > > > While 'UpdateCapsule' is a runtime services function, it is, at least

> > > > > > > > > > > initially, supported only before exiting boot services alike other runtime

> > > > > > > > > > > functions, [Get/]SetVariable. This is because modifying storage which may

> > > > > > > > > > > be shared with OS must be carefully designed and there is no general

> > > > > > > > > > > assumption that we can do it.

> > > > > > > > > > >

> > > > > > > > > > > Therefore, we practically support only "capsule on disk"; any capsule can

> > > > > > > > > > > be handed over to UEFI subsystem as a file on a specific file system.

> > > > > > > > > > >

> > > > > > > > > > > In this patch series, all the related definitions and structures are given

> > > > > > > > > > > as UEFI specification describes, and basic framework for capsule support

> > > > > > > > > > > is provided. Currently supported is

> > > > > > > > > > >  * firmware update (Firmware Management Protocol or simply FMP)

> > > > > > > > > > >

> > > > > > > > > > > Most of functionality of firmware update is provided by FMP driver and

> > > > > > > > > > > it can be, by nature, system/platform-specific. So you can and should

> > > > > > > > > > > implement your own FMP driver(s) based on your system requirements.

> > > > > > > > > > > Under the current implementation, we provide two basic but generic

> > > > > > > > > > > drivers with two formats:

> > > > > > > > > > >   * FIT image format (as used in TFTP update and dfu)

> > > > > > > > > > >   * raw image format

> > > > > > > > > > >

> > > > > > > > > > > It's totally up to users which one, or both, should be used on users'

> > > > > > > > > > > system depending on user requirements.

> > > > > > > > > > >

> > > > > > > > > > > Quick usage

> > > > > > > > > > > ===========

> > > > > > > > > > > 1. You can create a capsule file with the following host command:

> > > > > > > > > > >

> > > > > > > > > > >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> > > > > > > > > > >

> > > > > > > > > > > 2. Put the file under:

> > > > > > > > > > >

> > > > > > > > > > >   /EFI/UpdateCapsule of UEFI system partition

> > > > > > > > > > >

> > > > > > > > > > > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> > > > > > > > > > >    (Please follow README.dfu for details.)

> > > > > > > > > > >

> > > > > > > > > > >   ==> env set dfu_alt_info '...'

> > > > > > > > > > >

> > > > > > > > > > > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> > > > > > > > > > >

> > > > > > > > > > >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> > > > > > > > > > >

> > > > > > > > > > > Patch structure

> > > > > > > > > > > ===============

> > > > > > > > > > > Patch#1-#4,#12: preparatory patches

> > > > > > > > > > > Patch#5-#11,#13: main part of implementation

> > > > > > > > > > > Patch#14-#15: utilities

> > > > > > > > > > > Patch#16-#17: pytests

> > > > > > > > > > > Patch#18: for sandbox test

> > > > > > > > > > >

> > > > > > > > > > > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> > > > > > > > > > >

> > > > > > > > > > > Prerequisite patches

> > > > > > > > > > > ====================

> > > > > > > > > > > None

> > > > > > > > > > >

> > > > > > > > > > > Test

> > > > > > > > > > > ====

> > > > > > > > > > > * passed all the pytests which are included in this patch series

> > > > > > > > > > >   on sandbox build locally.

> > > > > > > > > > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> > > > > > > > > > >   "virt-make-fs" cannot be executed.

> > > > > > > > > > 

> > > > > > > > > > Dear Takahiro,

> > > > > > > > > > 

> > > > > > > > > > please, rebase your series on origin/master. A prior version of the

> > > > > > > > > > first patches is already applied.

> > > > > > > > > 

> > > > > > > > > You can simply pick up and apply non-merged patches without problems

> > > > > > > > > except a function prototype change of efi_create_indexed_name() you made,

> > > > > > > > > which is not trivial to me.

> > > > > > > > > 

> > > > > > > > > But anyway, I will post a clean patch set soon.

> > > > > > > > > 

> > > > > > > > > > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> > > > > > > > > > the remaining patches.

> > > > 

> > > > @Heinrich,

> > > > 

> > > > https://travis-ci.org/github/t-akashi/u-boot-for-travis/builds/743837907

> > > > https://github.com/u-boot/u-boot/pull/39

> > > > 

> > > > Do those results satisfy your request?

> > > > I don't know how I can deal with gitlab and amazon CI.

> > > 

> > > I believe that was a typo of "Azure", which you have above.  And since

> > > https://github.com/u-boot/u-boot/pull/39/checks shows green, I would

> > > expect that so long as you linted your .gitlab-ci.yml changes, if any,

> > > then it too should pass.

> > > 

> > > > > > > > > Where can we find the results?

> > > > > > > > > I don't think there is no official information on those CI's.

> > > > > > > > 

> > > > > > > > If you submit a pull request against https://github.com/u-boot/u-boot

> > > > > > > > Azure will run automatically and show the results.  We don't have

> > > > > > > 

> > > > > > > We can get a free account for Azure, but yet it requires us to give

> > > > > > > our credit card information to Azure. I don't want to accept it.

> > > > > > > So I believe requiring Azure test results is just inappropriate.

> > > > > > 

> > > > > > That's not correct, as far as I can tell.  You just need to submit a

> > > > > > pull request against the repository I mentioned above and that triggers

> > > > > > one.  For example: https://github.com/u-boot/u-boot/pull/35/checks is

> > > > > > the result of one such PR.

> > > > > 

> > > > > Okay, I will check and try.

> > > > > But I hope that you should have, on the home page?, more documents

> > > > > on those CI loops and requirements of test results as part of

> > > > > submission process.

> > > > 

> > > > @Tom,

> > > > 

> > > > Again, I'd like you to add a rule and a related document with regard to

> > > > criteria for "testing before submission". Otherwise, it's confusing.

> > > 

> > > I'm sorry you found it confusing.  It has been stated many times in

> > > these threads that since you're adding tests, they're expected to not

> > > break CI, and should even run when possible.  Updating the rST with a CI

> > > section would be good, yes.

> > 

> > For clarity, my claim above is not only for me, but also for "all"

> > contributors.

> > When I looked at

> > https://github.com/u-boot/u-boot/pulls

> > I was a bit disappointed as there are so few people who have actually

> > tried to invoke Azure CI (prior to their submissions?).

> > Heinrich requires CI results, but many custodians not.

> > Why this inconsistency?

> 

> Given that it takes Travis-CI several hours to complete a run, and Azure

> around 2 hours, most of the time it's fine the CI runs to get put off

> until the PR is being made.  In _this_ case you're adding tests, and

> they had previously caused CI to fail, so you needed to get CI to work.


Even if we don't know how we can kick off CI jobs and see the results?
That's why I request you to make information/documents publicly available.

> > Heinrich requires CI results, but many custodians not.

> > Why this inconsistency?


You didn't give me the answer for this.
Moreover, it's not very clear in which case you demand test cases being added.
Again, that's why I request you to add information/documents about
the criteria for "testing before submission".

-Takahiro Akashi

> -- 

> Tom
Tom Rini Nov. 18, 2020, 2:49 p.m. UTC | #16
On Wed, Nov 18, 2020 at 12:11:29PM +0900, AKASHI Takahiro wrote:
> On Tue, Nov 17, 2020 at 10:02:41PM -0500, Tom Rini wrote:

> > On Wed, Nov 18, 2020 at 09:26:38AM +0900, AKASHI Takahiro wrote:

> > > On Tue, Nov 17, 2020 at 06:59:52PM -0500, Tom Rini wrote:

> > > > On Wed, Nov 18, 2020 at 08:50:08AM +0900, AKASHI Takahiro wrote:

> > > > > Tom, Heinrich,

> > > > > 

> > > > > On Tue, Nov 17, 2020 at 09:44:36AM +0900, AKASHI Takahiro wrote:

> > > > > > Hi Tom,

> > > > > > 

> > > > > > On Mon, Nov 16, 2020 at 07:36:26PM -0500, Tom Rini wrote:

> > > > > > > On Tue, Nov 17, 2020 at 09:16:26AM +0900, AKASHI Takahiro wrote:

> > > > > > > > On Mon, Nov 16, 2020 at 11:10:12AM -0500, Tom Rini wrote:

> > > > > > > > > On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:

> > > > > > > > > > Heinrich,

> > > > > > > > > > 

> > > > > > > > > > On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:

> > > > > > > > > > > On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > > > > > > > > > > > Summary

> > > > > > > > > > > > =======

> > > > > > > > > > > > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > > > > > > > > > > > and its aim is to allow a caller (OS) to pass information to the firmware,

> > > > > > > > > > > > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > > > > > > > > > > > instructions from OS.

> > > > > > > > > > > >

> > > > > > > > > > > > While 'UpdateCapsule' is a runtime services function, it is, at least

> > > > > > > > > > > > initially, supported only before exiting boot services alike other runtime

> > > > > > > > > > > > functions, [Get/]SetVariable. This is because modifying storage which may

> > > > > > > > > > > > be shared with OS must be carefully designed and there is no general

> > > > > > > > > > > > assumption that we can do it.

> > > > > > > > > > > >

> > > > > > > > > > > > Therefore, we practically support only "capsule on disk"; any capsule can

> > > > > > > > > > > > be handed over to UEFI subsystem as a file on a specific file system.

> > > > > > > > > > > >

> > > > > > > > > > > > In this patch series, all the related definitions and structures are given

> > > > > > > > > > > > as UEFI specification describes, and basic framework for capsule support

> > > > > > > > > > > > is provided. Currently supported is

> > > > > > > > > > > >  * firmware update (Firmware Management Protocol or simply FMP)

> > > > > > > > > > > >

> > > > > > > > > > > > Most of functionality of firmware update is provided by FMP driver and

> > > > > > > > > > > > it can be, by nature, system/platform-specific. So you can and should

> > > > > > > > > > > > implement your own FMP driver(s) based on your system requirements.

> > > > > > > > > > > > Under the current implementation, we provide two basic but generic

> > > > > > > > > > > > drivers with two formats:

> > > > > > > > > > > >   * FIT image format (as used in TFTP update and dfu)

> > > > > > > > > > > >   * raw image format

> > > > > > > > > > > >

> > > > > > > > > > > > It's totally up to users which one, or both, should be used on users'

> > > > > > > > > > > > system depending on user requirements.

> > > > > > > > > > > >

> > > > > > > > > > > > Quick usage

> > > > > > > > > > > > ===========

> > > > > > > > > > > > 1. You can create a capsule file with the following host command:

> > > > > > > > > > > >

> > > > > > > > > > > >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> > > > > > > > > > > >

> > > > > > > > > > > > 2. Put the file under:

> > > > > > > > > > > >

> > > > > > > > > > > >   /EFI/UpdateCapsule of UEFI system partition

> > > > > > > > > > > >

> > > > > > > > > > > > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> > > > > > > > > > > >    (Please follow README.dfu for details.)

> > > > > > > > > > > >

> > > > > > > > > > > >   ==> env set dfu_alt_info '...'

> > > > > > > > > > > >

> > > > > > > > > > > > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> > > > > > > > > > > >

> > > > > > > > > > > >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> > > > > > > > > > > >

> > > > > > > > > > > > Patch structure

> > > > > > > > > > > > ===============

> > > > > > > > > > > > Patch#1-#4,#12: preparatory patches

> > > > > > > > > > > > Patch#5-#11,#13: main part of implementation

> > > > > > > > > > > > Patch#14-#15: utilities

> > > > > > > > > > > > Patch#16-#17: pytests

> > > > > > > > > > > > Patch#18: for sandbox test

> > > > > > > > > > > >

> > > > > > > > > > > > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> > > > > > > > > > > >

> > > > > > > > > > > > Prerequisite patches

> > > > > > > > > > > > ====================

> > > > > > > > > > > > None

> > > > > > > > > > > >

> > > > > > > > > > > > Test

> > > > > > > > > > > > ====

> > > > > > > > > > > > * passed all the pytests which are included in this patch series

> > > > > > > > > > > >   on sandbox build locally.

> > > > > > > > > > > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> > > > > > > > > > > >   "virt-make-fs" cannot be executed.

> > > > > > > > > > > 

> > > > > > > > > > > Dear Takahiro,

> > > > > > > > > > > 

> > > > > > > > > > > please, rebase your series on origin/master. A prior version of the

> > > > > > > > > > > first patches is already applied.

> > > > > > > > > > 

> > > > > > > > > > You can simply pick up and apply non-merged patches without problems

> > > > > > > > > > except a function prototype change of efi_create_indexed_name() you made,

> > > > > > > > > > which is not trivial to me.

> > > > > > > > > > 

> > > > > > > > > > But anyway, I will post a clean patch set soon.

> > > > > > > > > > 

> > > > > > > > > > > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> > > > > > > > > > > the remaining patches.

> > > > > 

> > > > > @Heinrich,

> > > > > 

> > > > > https://travis-ci.org/github/t-akashi/u-boot-for-travis/builds/743837907

> > > > > https://github.com/u-boot/u-boot/pull/39

> > > > > 

> > > > > Do those results satisfy your request?

> > > > > I don't know how I can deal with gitlab and amazon CI.

> > > > 

> > > > I believe that was a typo of "Azure", which you have above.  And since

> > > > https://github.com/u-boot/u-boot/pull/39/checks shows green, I would

> > > > expect that so long as you linted your .gitlab-ci.yml changes, if any,

> > > > then it too should pass.

> > > > 

> > > > > > > > > > Where can we find the results?

> > > > > > > > > > I don't think there is no official information on those CI's.

> > > > > > > > > 

> > > > > > > > > If you submit a pull request against https://github.com/u-boot/u-boot

> > > > > > > > > Azure will run automatically and show the results.  We don't have

> > > > > > > > 

> > > > > > > > We can get a free account for Azure, but yet it requires us to give

> > > > > > > > our credit card information to Azure. I don't want to accept it.

> > > > > > > > So I believe requiring Azure test results is just inappropriate.

> > > > > > > 

> > > > > > > That's not correct, as far as I can tell.  You just need to submit a

> > > > > > > pull request against the repository I mentioned above and that triggers

> > > > > > > one.  For example: https://github.com/u-boot/u-boot/pull/35/checks is

> > > > > > > the result of one such PR.

> > > > > > 

> > > > > > Okay, I will check and try.

> > > > > > But I hope that you should have, on the home page?, more documents

> > > > > > on those CI loops and requirements of test results as part of

> > > > > > submission process.

> > > > > 

> > > > > @Tom,

> > > > > 

> > > > > Again, I'd like you to add a rule and a related document with regard to

> > > > > criteria for "testing before submission". Otherwise, it's confusing.

> > > > 

> > > > I'm sorry you found it confusing.  It has been stated many times in

> > > > these threads that since you're adding tests, they're expected to not

> > > > break CI, and should even run when possible.  Updating the rST with a CI

> > > > section would be good, yes.

> > > 

> > > For clarity, my claim above is not only for me, but also for "all"

> > > contributors.

> > > When I looked at

> > > https://github.com/u-boot/u-boot/pulls

> > > I was a bit disappointed as there are so few people who have actually

> > > tried to invoke Azure CI (prior to their submissions?).

> > > Heinrich requires CI results, but many custodians not.

> > > Why this inconsistency?

> > 

> > Given that it takes Travis-CI several hours to complete a run, and Azure

> > around 2 hours, most of the time it's fine the CI runs to get put off

> > until the PR is being made.  In _this_ case you're adding tests, and

> > they had previously caused CI to fail, so you needed to get CI to work.

> 

> Even if we don't know how we can kick off CI jobs and see the results?

> That's why I request you to make information/documents publicly available.


Yes, it would be good to add a section to the rST about CI.  I've noted
how to trigger Azure in a few threads a few times, outside of this
thread.

> > > Heinrich requires CI results, but many custodians not.

> > > Why this inconsistency?

> 

> You didn't give me the answer for this.


Yes, I did.  You, right here, are adding tests.  That's why you need to
run CI.  Custodians who don't run CI before sending me a PR, and who
then have a problem CI catches, get the PR nak'd.  Most changes are
localized and don't break builds outside of the SoC being changed.

> Moreover, it's not very clear in which case you demand test cases being added.

> Again, that's why I request you to add information/documents about

> the criteria for "testing before submission".


Well, how important is it to YOU that the EFI capsule code doesn't get
broken?  Code that we can run tests on in CI should get tests run in CI.

-- 
Tom
AKASHI Takahiro Nov. 19, 2020, 12:51 a.m. UTC | #17
On Wed, Nov 18, 2020 at 09:49:22AM -0500, Tom Rini wrote:
> On Wed, Nov 18, 2020 at 12:11:29PM +0900, AKASHI Takahiro wrote:

> > On Tue, Nov 17, 2020 at 10:02:41PM -0500, Tom Rini wrote:

> > > On Wed, Nov 18, 2020 at 09:26:38AM +0900, AKASHI Takahiro wrote:

> > > > On Tue, Nov 17, 2020 at 06:59:52PM -0500, Tom Rini wrote:

> > > > > On Wed, Nov 18, 2020 at 08:50:08AM +0900, AKASHI Takahiro wrote:

> > > > > > Tom, Heinrich,

> > > > > > 

> > > > > > On Tue, Nov 17, 2020 at 09:44:36AM +0900, AKASHI Takahiro wrote:

> > > > > > > Hi Tom,

> > > > > > > 

> > > > > > > On Mon, Nov 16, 2020 at 07:36:26PM -0500, Tom Rini wrote:

> > > > > > > > On Tue, Nov 17, 2020 at 09:16:26AM +0900, AKASHI Takahiro wrote:

> > > > > > > > > On Mon, Nov 16, 2020 at 11:10:12AM -0500, Tom Rini wrote:

> > > > > > > > > > On Mon, Nov 16, 2020 at 09:37:23AM +0900, AKASHI Takahiro wrote:

> > > > > > > > > > > Heinrich,

> > > > > > > > > > > 

> > > > > > > > > > > On Fri, Nov 13, 2020 at 08:18:58AM +0100, Heinrich Schuchardt wrote:

> > > > > > > > > > > > On 11/13/20 5:14 AM, AKASHI Takahiro wrote:

> > > > > > > > > > > > > Summary

> > > > > > > > > > > > > =======

> > > > > > > > > > > > > 'UpdateCapsule' is one of runtime services defined in UEFI specification

> > > > > > > > > > > > > and its aim is to allow a caller (OS) to pass information to the firmware,

> > > > > > > > > > > > > i.e. U-Boot. This is mostly used to update firmware binary on devices by

> > > > > > > > > > > > > instructions from OS.

> > > > > > > > > > > > >

> > > > > > > > > > > > > While 'UpdateCapsule' is a runtime services function, it is, at least

> > > > > > > > > > > > > initially, supported only before exiting boot services alike other runtime

> > > > > > > > > > > > > functions, [Get/]SetVariable. This is because modifying storage which may

> > > > > > > > > > > > > be shared with OS must be carefully designed and there is no general

> > > > > > > > > > > > > assumption that we can do it.

> > > > > > > > > > > > >

> > > > > > > > > > > > > Therefore, we practically support only "capsule on disk"; any capsule can

> > > > > > > > > > > > > be handed over to UEFI subsystem as a file on a specific file system.

> > > > > > > > > > > > >

> > > > > > > > > > > > > In this patch series, all the related definitions and structures are given

> > > > > > > > > > > > > as UEFI specification describes, and basic framework for capsule support

> > > > > > > > > > > > > is provided. Currently supported is

> > > > > > > > > > > > >  * firmware update (Firmware Management Protocol or simply FMP)

> > > > > > > > > > > > >

> > > > > > > > > > > > > Most of functionality of firmware update is provided by FMP driver and

> > > > > > > > > > > > > it can be, by nature, system/platform-specific. So you can and should

> > > > > > > > > > > > > implement your own FMP driver(s) based on your system requirements.

> > > > > > > > > > > > > Under the current implementation, we provide two basic but generic

> > > > > > > > > > > > > drivers with two formats:

> > > > > > > > > > > > >   * FIT image format (as used in TFTP update and dfu)

> > > > > > > > > > > > >   * raw image format

> > > > > > > > > > > > >

> > > > > > > > > > > > > It's totally up to users which one, or both, should be used on users'

> > > > > > > > > > > > > system depending on user requirements.

> > > > > > > > > > > > >

> > > > > > > > > > > > > Quick usage

> > > > > > > > > > > > > ===========

> > > > > > > > > > > > > 1. You can create a capsule file with the following host command:

> > > > > > > > > > > > >

> > > > > > > > > > > > >   $ mkeficapsule [--fit <fit image> | --raw <raw image>] <output file>

> > > > > > > > > > > > >

> > > > > > > > > > > > > 2. Put the file under:

> > > > > > > > > > > > >

> > > > > > > > > > > > >   /EFI/UpdateCapsule of UEFI system partition

> > > > > > > > > > > > >

> > > > > > > > > > > > > 3. Specify firmware storage to be updated in "dfu_alt_info" variable

> > > > > > > > > > > > >    (Please follow README.dfu for details.)

> > > > > > > > > > > > >

> > > > > > > > > > > > >   ==> env set dfu_alt_info '...'

> > > > > > > > > > > > >

> > > > > > > > > > > > > 4. After setting up UEFI's OsIndications variable, reboot U-Boot:

> > > > > > > > > > > > >

> > > > > > > > > > > > >   OsIndications <= EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED

> > > > > > > > > > > > >

> > > > > > > > > > > > > Patch structure

> > > > > > > > > > > > > ===============

> > > > > > > > > > > > > Patch#1-#4,#12: preparatory patches

> > > > > > > > > > > > > Patch#5-#11,#13: main part of implementation

> > > > > > > > > > > > > Patch#14-#15: utilities

> > > > > > > > > > > > > Patch#16-#17: pytests

> > > > > > > > > > > > > Patch#18: for sandbox test

> > > > > > > > > > > > >

> > > > > > > > > > > > > [1] https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/capsule

> > > > > > > > > > > > >

> > > > > > > > > > > > > Prerequisite patches

> > > > > > > > > > > > > ====================

> > > > > > > > > > > > > None

> > > > > > > > > > > > >

> > > > > > > > > > > > > Test

> > > > > > > > > > > > > ====

> > > > > > > > > > > > > * passed all the pytests which are included in this patch series

> > > > > > > > > > > > >   on sandbox build locally.

> > > > > > > > > > > > > * skipped (or 'S', but it's not a failure, 'F') in Travis CI because

> > > > > > > > > > > > >   "virt-make-fs" cannot be executed.

> > > > > > > > > > > > 

> > > > > > > > > > > > Dear Takahiro,

> > > > > > > > > > > > 

> > > > > > > > > > > > please, rebase your series on origin/master. A prior version of the

> > > > > > > > > > > > first patches is already applied.

> > > > > > > > > > > 

> > > > > > > > > > > You can simply pick up and apply non-merged patches without problems

> > > > > > > > > > > except a function prototype change of efi_create_indexed_name() you made,

> > > > > > > > > > > which is not trivial to me.

> > > > > > > > > > > 

> > > > > > > > > > > But anyway, I will post a clean patch set soon.

> > > > > > > > > > > 

> > > > > > > > > > > > Testing on Gitlab CI, Travis CI, Amazon CI must be addressed for merging

> > > > > > > > > > > > the remaining patches.

> > > > > > 

> > > > > > @Heinrich,

> > > > > > 

> > > > > > https://travis-ci.org/github/t-akashi/u-boot-for-travis/builds/743837907

> > > > > > https://github.com/u-boot/u-boot/pull/39

> > > > > > 

> > > > > > Do those results satisfy your request?

> > > > > > I don't know how I can deal with gitlab and amazon CI.

> > > > > 

> > > > > I believe that was a typo of "Azure", which you have above.  And since

> > > > > https://github.com/u-boot/u-boot/pull/39/checks shows green, I would

> > > > > expect that so long as you linted your .gitlab-ci.yml changes, if any,

> > > > > then it too should pass.

> > > > > 

> > > > > > > > > > > Where can we find the results?

> > > > > > > > > > > I don't think there is no official information on those CI's.

> > > > > > > > > > 

> > > > > > > > > > If you submit a pull request against https://github.com/u-boot/u-boot

> > > > > > > > > > Azure will run automatically and show the results.  We don't have

> > > > > > > > > 

> > > > > > > > > We can get a free account for Azure, but yet it requires us to give

> > > > > > > > > our credit card information to Azure. I don't want to accept it.

> > > > > > > > > So I believe requiring Azure test results is just inappropriate.

> > > > > > > > 

> > > > > > > > That's not correct, as far as I can tell.  You just need to submit a

> > > > > > > > pull request against the repository I mentioned above and that triggers

> > > > > > > > one.  For example: https://github.com/u-boot/u-boot/pull/35/checks is

> > > > > > > > the result of one such PR.

> > > > > > > 

> > > > > > > Okay, I will check and try.

> > > > > > > But I hope that you should have, on the home page?, more documents

> > > > > > > on those CI loops and requirements of test results as part of

> > > > > > > submission process.

> > > > > > 

> > > > > > @Tom,

> > > > > > 

> > > > > > Again, I'd like you to add a rule and a related document with regard to

> > > > > > criteria for "testing before submission". Otherwise, it's confusing.

> > > > > 

> > > > > I'm sorry you found it confusing.  It has been stated many times in

> > > > > these threads that since you're adding tests, they're expected to not

> > > > > break CI, and should even run when possible.  Updating the rST with a CI

> > > > > section would be good, yes.

> > > > 

> > > > For clarity, my claim above is not only for me, but also for "all"

> > > > contributors.

> > > > When I looked at

> > > > https://github.com/u-boot/u-boot/pulls

> > > > I was a bit disappointed as there are so few people who have actually

> > > > tried to invoke Azure CI (prior to their submissions?).

> > > > Heinrich requires CI results, but many custodians not.

> > > > Why this inconsistency?

> > > 

> > > Given that it takes Travis-CI several hours to complete a run, and Azure

> > > around 2 hours, most of the time it's fine the CI runs to get put off

> > > until the PR is being made.  In _this_ case you're adding tests, and

> > > they had previously caused CI to fail, so you needed to get CI to work.

> > 

> > Even if we don't know how we can kick off CI jobs and see the results?

> > That's why I request you to make information/documents publicly available.

> 

> Yes, it would be good to add a section to the rST about CI.  I've noted

> how to trigger Azure in a few threads a few times, outside of this

> thread.


Again, my point is to put such information in public space to make
it easy to find them and understand clearly what you "demand".

> > > > Heinrich requires CI results, but many custodians not.

> > > > Why this inconsistency?

> > 

> > You didn't give me the answer for this.

> 

> Yes, I did.  You, right here, are adding tests.  That's why you need to

> run CI.  Custodians who don't run CI before sending me a PR, and who

> then have a problem CI catches, get the PR nak'd.  Most changes are

> localized and don't break builds outside of the SoC being changed.


I see lots of "new features" coming in even in these days.
But I only see a very few "pull requests" on your github repository.
That is why I'm asking you for criteria for "testing".

> > Moreover, it's not very clear in which case you demand test cases being added.

> > Again, that's why I request you to add information/documents about

> > the criteria for "testing before submission".

> 

> Well, how important is it to YOU that the EFI capsule code doesn't get

> broken?  Code that we can run tests on in CI should get tests run in CI.


I totally agreed, and did submit the test results in the previous
message. But remember Heinrich demanded that even Gitlab and Amazon CI pass.
How can we do that without knowing how to run them?
What is your expectation?

That is why ... (I don't want to repeat my point again here.)

-Takahiro Akashi


> -- 

> Tom
Jean Lucas Nov. 19, 2020, 5:45 a.m. UTC | #18
On 11/17/20 1:28 PM, Jean Lucas wrote:
> On 11/17/20 9:42 AM, Alper Nebi Yasak wrote:

>> On 17/11/2020 17:11, Peter Robinson wrote:

>>> On Tue, Nov 17, 2020 at 12:43 PM Alper Nebi Yasak

>>> <alpernebiyasak@gmail.com> wrote:

>>>> On 17/11/2020 04:49, Jean Lucas wrote:

>>>>> Hello all,

>>>>>

>>>>> On Pine64 RockPro64 and Pinebook Pro (both RK3399), flashing U-Boot

>>>>> v2021.01-rc2-47-g9324c9a823 defconfig and mainline ATF

>>>>> v2.4-rc0-2-gd01f31c03 to SPI flash of both devices results in a hang

>>>>> shortly after loading the appropriate FDT when booting.

>>>>>

>>>>> On a Pinebook Pro:

>>>>>

>>>>> => load mmc 0:1 ${kernel_addr_r} Image

>>>>> 32418304 bytes read in 1448 ms (21.4 MiB/s)

>>>>> => load mmc 0:1 ${fdt_addr_r} dtbs/rockchip/rk3399-pinebook-pro.dtb

>>>>> 80831 bytes read in 27 ms (2.9 MiB/s)

>>>>> => load mmc 0:1 ${ramdisk_addr_r} initramfs-linux.img

>>>>> 29698825 bytes read in 1291 ms (21.9 MiB/s)

>>>>> => setenv bootargs console=ttyS2,1500000 console=tty0

>>>>> root=/dev/ghost/root audit=0

>>>>> => booti ${kernel_addr_r} ${ramdisk_addr_r}:${filesize} ${fdt_addr_r}

>>>>> Moving Image from 0x2080000 to 0x2200000, end=41c0000

>>>>> ## Flattened Device Tree blob at 01f00000

>>>>>      Booting using the

>>>>>

>>>>> Behavior is the same on RockPro64.

>>>>>

>>>>> Worth mentioning is that U-Boot from about a week ago (I think

>>>>> rc2-4-gf36603c7a8) with same mainline ATF version written to eMMC

>>>>> results in a working boot on Pinebook Pro, so bug seems to be when

>>>>> booting from SPI.

>>>>>

>>>>> To further the hypothesis, on RockPro64, the latest U-Boot I can use

>>>>> from SPI (defconfig) is release 2020.04, since later releases also 

>>>>> hang

>>>>> on loading the appropriate FDT when booting as described above.

>>>>>

>>>>> Any ideas as to what could be causing the hangs?

>>>>>

>>>> My gru-kevin was hanging at that same place when I tried to boot from

>>>> USB, try running "usb stop" to see if that hangs. If so, try disabling

>>>> CONFIG_USB_OHCI_HCD and CONFIG_USB_OHCI_GENERIC, which fixed the 

>>>> hang in

>>>> my case.

>>> Which probably breaks USB keyboards?

>> I wouldn't know, gru-kevin uses cros-ec-keyb so I never needed to try a

>> USB keyboard. I meant those as steps to narrow things down.

> 

> So, running "usb stop" resulted in a hang.

> 

> I will test a build with the OHCI options Alper mentioned disabled, this 

> evening.

> 

> As far as testing goes, disabling USB is fine since I can access the 

> machines over serial.

> 


Hi,

Interestingly, disabling OHCI did the trick, and the Pinebook Pro and 
RockPro64 both boot normally. Unfortunately, Pinebook Pro's keyboard is 
connected to the mainboard via USB, and RockPro64 is USB-only as well, 
so OHCI support will have to be reviewed for these boards at least.

Interestingly, flashing the same U-Boot revision (except with OHCI 
enabled) to eMMC on Pinebook Pro - which results in SPL loading U-Boot 
from the eMMC by default, even when booted from SPI - results in working 
USB support. The boot hang only becomes an issue when U-Boot is loaded 
purely from SPI.

Regards,
Jean
Jean Lucas Nov. 19, 2020, 6:08 a.m. UTC | #19
On 11/19/20 12:45 AM, Jean Lucas wrote:
> On 11/17/20 1:28 PM, Jean Lucas wrote:

>> On 11/17/20 9:42 AM, Alper Nebi Yasak wrote:

>>> On 17/11/2020 17:11, Peter Robinson wrote:

>>>> On Tue, Nov 17, 2020 at 12:43 PM Alper Nebi Yasak

>>>> <alpernebiyasak@gmail.com> wrote:

>>>>> On 17/11/2020 04:49, Jean Lucas wrote:

>>>>>> Hello all,

>>>>>>

>>>>>> On Pine64 RockPro64 and Pinebook Pro (both RK3399), flashing U-Boot

>>>>>> v2021.01-rc2-47-g9324c9a823 defconfig and mainline ATF

>>>>>> v2.4-rc0-2-gd01f31c03 to SPI flash of both devices results in a hang

>>>>>> shortly after loading the appropriate FDT when booting.

>>>>>>

>>>>>> On a Pinebook Pro:

>>>>>>

>>>>>> => load mmc 0:1 ${kernel_addr_r} Image

>>>>>> 32418304 bytes read in 1448 ms (21.4 MiB/s)

>>>>>> => load mmc 0:1 ${fdt_addr_r} dtbs/rockchip/rk3399-pinebook-pro.dtb

>>>>>> 80831 bytes read in 27 ms (2.9 MiB/s)

>>>>>> => load mmc 0:1 ${ramdisk_addr_r} initramfs-linux.img

>>>>>> 29698825 bytes read in 1291 ms (21.9 MiB/s)

>>>>>> => setenv bootargs console=ttyS2,1500000 console=tty0

>>>>>> root=/dev/ghost/root audit=0

>>>>>> => booti ${kernel_addr_r} ${ramdisk_addr_r}:${filesize} ${fdt_addr_r}

>>>>>> Moving Image from 0x2080000 to 0x2200000, end=41c0000

>>>>>> ## Flattened Device Tree blob at 01f00000

>>>>>>      Booting using the

>>>>>>

>>>>>> Behavior is the same on RockPro64.

>>>>>>

>>>>>> Worth mentioning is that U-Boot from about a week ago (I think

>>>>>> rc2-4-gf36603c7a8) with same mainline ATF version written to eMMC

>>>>>> results in a working boot on Pinebook Pro, so bug seems to be when

>>>>>> booting from SPI.

>>>>>>

>>>>>> To further the hypothesis, on RockPro64, the latest U-Boot I can use

>>>>>> from SPI (defconfig) is release 2020.04, since later releases also 

>>>>>> hang

>>>>>> on loading the appropriate FDT when booting as described above.

>>>>>>

>>>>>> Any ideas as to what could be causing the hangs?

>>>>>>

>>>>> My gru-kevin was hanging at that same place when I tried to boot from

>>>>> USB, try running "usb stop" to see if that hangs. If so, try disabling

>>>>> CONFIG_USB_OHCI_HCD and CONFIG_USB_OHCI_GENERIC, which fixed the 

>>>>> hang in

>>>>> my case.

>>>> Which probably breaks USB keyboards?

>>> I wouldn't know, gru-kevin uses cros-ec-keyb so I never needed to try a

>>> USB keyboard. I meant those as steps to narrow things down.

>>

>> So, running "usb stop" resulted in a hang.

>>

>> I will test a build with the OHCI options Alper mentioned disabled, 

>> this evening.

>>

>> As far as testing goes, disabling USB is fine since I can access the 

>> machines over serial.

>>

> 

> Hi,

> 

> Interestingly, disabling OHCI did the trick, and the Pinebook Pro and 

> RockPro64 both boot normally. Unfortunately, Pinebook Pro's keyboard is 

> connected to the mainboard via USB, and RockPro64 is USB-only as well, 

> so OHCI support will have to be reviewed for these boards at least.

> 

> Interestingly, flashing the same U-Boot revision (except with OHCI 

> enabled) to eMMC on Pinebook Pro - which results in SPL loading U-Boot 

> from the eMMC by default, even when booted from SPI - results in working 

> USB support. The boot hang only becomes an issue when U-Boot is loaded 

> purely from SPI.

> 

> Regards,

> Jean


Ah, I spoke too soon regarding U-Boot flashed to eMMC with OHCI support 
resulting in working boot, and the boot hang only being when U-Boot with 
OHCI support is loaded purely from SPI.

The hang indeed also occurs as of v2021.01-rc2-67-ge800d715e0, even when 
loaded from eMMC.

Regards
Jan Palus Jan. 28, 2021, 10:28 p.m. UTC | #20
FWIW I figured it hangs in ohci-hcd.c at:

static int hc_reset(ohci_t *ohci)
...
        if (ohci_readl(&ohci->regs->control) & OHCI_CTRL_IR) {
Jan Palus Jan. 29, 2021, 12:56 a.m. UTC | #21
On 28.01.2021 23:28, Jan Palus wrote:
> FWIW I figured it hangs in ohci-hcd.c at:

> 

> static int hc_reset(ohci_t *ohci)

> ...

>         if (ohci_readl(&ohci->regs->control) & OHCI_CTRL_IR) {


This appears to be caused by following events:

- usb_stop() (usb-class.c) iterates to remove every bus
- starts with ehci and goes all the way to ehci_shutdown_phy()
- there it calls generic_phy_exit() ending in rockchip_usb2phy_exit()
- where at last it disables clock with clk_disable() (rk3399_clk_disable())

After disabling clock ohci_readl(&ohci->regs->control) starts to hang so perhaps it
shouldn't be disabled until ohci is removed.
Jan Palus Jan. 29, 2021, 6:47 p.m. UTC | #22
On 29.01.2021 01:56, Jan Palus wrote:
> On 28.01.2021 23:28, Jan Palus wrote:

> > FWIW I figured it hangs in ohci-hcd.c at:

> > 

> > static int hc_reset(ohci_t *ohci)

> > ...

> >         if (ohci_readl(&ohci->regs->control) & OHCI_CTRL_IR) {

> 

> This appears to be caused by following events:

> 

> - usb_stop() (usb-class.c) iterates to remove every bus

> - starts with ehci and goes all the way to ehci_shutdown_phy()

> - there it calls generic_phy_exit() ending in rockchip_usb2phy_exit()

> - where at last it disables clock with clk_disable() (rk3399_clk_disable())

> 

> After disabling clock ohci_readl(&ohci->regs->control) starts to hang so perhaps it

> shouldn't be disabled until ohci is removed.


Looks like that's as far as I can go by myself. Perhaps the solution
could be CCF which appears to enable usage count tracking and prevents
disabling clock if it still has more users. Or perhaps EHCI/OHCI should
not be removed independently but instead one single remove should be
issued for usb2phy. Can't really tell.

In the meantime on Pinebook Pro more convenient workaround than
suggested disabling OHCI, is to rather disable EHCI. This way internal
keyboard works and boot process is still fine. I suppose external
keyboard wouldn't work then though.