Re: [PATCH v2 3/7] power: regulator-uclass: perform regulator setup inside uclass
5 серпня 2023 р. 23:46:27 GMT+03:00, Jonas Karlman
написав(-ла):
>On 2023-08-05 21:58, Svyatoslav Ryhel wrote:
>>
>>
>> 5 серпня 2023 р. 15:49:32 GMT+03:00, Jonas Karlman
>> написав(-ла):
>>> Hi,
>>>
>>> On 2023-07-21 07:34, Svyatoslav Ryhel wrote:
чт, 20 лип. 2023 р. о 22:43 Simon Glass пише:
>
> Hi Svyatoslav,
>
> On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel wrote:
>>
>> Regulators initial setup was previously dependent on board call.
>> To move from this behaviour were introduced two steps. First is
>> that all individual regulators will be probed just after binding
>
> We must not probe devices automatically when bound. The i2c interface
> may not be available, etc. Do a probe afterwards.
>
> Perhaps I am misunderstanding this, iwc please reword this commit message.
After bound. If the regulator is a self-sufficient i2c device then it
will be bound
after i2c is available by code design so i2c interface should be
available at that
moment. At least led and gpio uclasses perform this for initial setup
of devices.
Platform regulators (aka fixed/gpio regulators) work perfectly fine. I
have no i2c
regulators to test deeply.
As for now only one uclass is not compatible with this system - PMIC which
has
strong dependency between regulator and main mfd. This is why probing after
bind for pmic regulators is disabled and pmic regulators are probed on
pmic core
post_probe.
>>>
>>> This sounds more like a possible bug of some dependency not being
>>> probed in correct order or not leaving the device in a ready state
>>> after probe.
>>>
>>> If pmic regulators are bind in pmic bind with the pmic as parent, then
>>> at regulator probe the driver core ensure that pmic has probed before
>>> regulator use.
>>>
>>> This works perfect fine with the RK8xx I2C PMIC and its regulators.
>>> Wich a call to device_get_supply_regulator(dev, "test-supply", )
>>> probe happens in i2c, pmic and regulator order.
>>>
>>
>> I will check and re-test drivers I have ASAP.
>>
>> which ensures that regulator pdata is filled and second is setting
>> up regulator in post probe which enseres that correct regulator
>> state according to pdata is reached.
>>>
>>> I think that the approach in this patch is trying to change too much all
>>> at once.
>>>
>>> Have made some adjustments that I think should help with transition.
>>> - Added a flag to protect regulator_autoset from being called more then
>>> once for each regulator, in a separate patch.
>>
>> It is not a good decision in the long run and you should keep in mind that
>> there is nothing more constant than a temporary solution.
>
>Nor do I propose this to be a long-term solution, only that it is more
>reviewable to see changes in non-breaking smaller steps. In current
>state this patch breaks building U-Boot and requires the last patch to
>fix build again.
>
>Anyway, I will be posting a similar patch for autoset as linked below as
>part of a series to add a Rockchip IO-domain driver shortly. In current
>state autoset cannot safely be called multiple times, and such patch
>should not have an impact on the direction of this series.
>
>>
>>> - Changed to only probe-after-bind on regulators marked as
>>> always-on/boot-on.
>>>
>>> And it works something like this:
>>>
>>> static int regulator_post_bind(struct udevice *dev)
>>> {
>>> [...]
>>>
>>> if (uc_pdata->always_on || uc_pdata->boot_on)
>>> dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
>>> }
>>>
>>> static int regulator_post_probe(struct udevice *dev)
>>> {
>>> ret = regulator_autoset(dev);
>>> }
>>>
>>> With that any always-on/boot-on regulator would automatically probe and
>>> autoset after bind, remaining would only probe and autoset if they are
>>> used.
>>
>> uc_pdata is filled in pre_probe, while you are runing check in post_bind.
>
>Please look closer at the code and not the snippet above, they are
>filled in post_bind or the code would not have worked :-)
So you have moved that, alright. Well, I am fine with less invasive patch as
long as my devices continue to work as intended. I will test your suggestions
and reload patchset. Thanks.
Best regards,
Svyatoslav R.
>Regards,
>Jonas
>
>>
>>> This approach should also prevent having to change existing code that
>>> may call autoset, and cleanup can be done in follow-up patches/series.
>>>
>>> Probe-after-bind and call to autoset could also be protected with a new
>>> Kconfig to help with transition.
>>>
>>> See following for a working example using a new driver that depends
>>> on that regulators have been autoset prior to regulator_get_value.
>>> https://github.com/Kwiboo/u-boot-rockchip/compare/master...rk3568-io-domain
>>>
>>> Or the two commits separate:
>>>
>>> power: regulator: Only run autoset once for each regulator
>>>
cli: Add explicit kconfig dependency for CONFIG_IS_ENABLED macro
From fe2ceb7c4365112055ecfc3bbf68ad47330b744d Mon Sep 17 00:00:00 2001 From: Pavel Korotkevich Date: Sun, 6 Aug 2023 02:00:41 +0300 Subject: [PATCH] cli: Add explicit kconfig dependency for CONFIG_IS_ENABLED macro --- include/cli.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/cli.h b/include/cli.h index 094a6602d7..d6dbbde070 100644 --- a/include/cli.h +++ b/include/cli.h @@ -9,6 +9,8 @@ #include +#include + /** * struct cli_ch_state - state information for reading cmdline characters * -- 2.39.0
Re: [PATCH v7 06/11] sandbox: Build the mkeficapsule tool for the sandbox variants
Hi Tom, On Sat, 5 Aug 2023 at 16:15, Tom Rini wrote: > > On Sat, Aug 05, 2023 at 09:03:49AM -0600, Simon Glass wrote: > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote: > > > > > > Build the mkeficapsule tool for all the sandbox variants. This tool > > > will be used subsequently for testing capsule generation in binman. > > > > > > Signed-off-by: Sughosh Ganu > > > --- > > > Changes since V6: > > > * New patch which has been split up from the binman capsule entry > > > support patch from earlier version, as suggested by Simon Glass. > > > * Enables mkeficapsule tool for all sandbox variants, instead of only > > > for sandbox_spl variant. > > > > > > tools/Kconfig | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > Reviewed-by: Simon Glass > > > > At some point after this series is in, please take a look at why this > > tool isn't enabled for all builds. > > The tool requires a bunch of stuff (see the rules in tools/Makefile). Oh, I see. It requires uuid and gnutils Even so, would it be too bad to require those packages for building U-Boot? Regards, Simon
Please pull u-boot-dm
Hi Tom,
https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/17224
https://dev.azure.com/simon0972/u-boot/_build/results?buildId=45=results
The following changes since commit 9787da0d32c2d58bae790a16ded0fe0c150c3280:
Merge branch '2023-08-04-toradex-platform-updates' (2023-08-04 16:04:11 -0400)
are available in the Git repository at:
git://git.denx.de/u-boot-dm.git tags/dm-pull-5aug23
for you to fetch changes up to 48d4c0a85d6f7fdb46dddcaa8a5f5b5c4290819f:
buildman: Drop warning about orphaned defconfigs (2023-08-05 11:39:23 -0600)
binman support for Xilinx signing
buildman minor fixes
Jaehoon Chung (1):
event: Fix a wrong type_name from dm_post_init to dm_post_init_f
Lukas Funke (3):
binman: btool: Add Xilinx Bootgen btool
binman: etype: Add xilinx-bootgen etype
binman: ftest: Add test for xilinx-bootgen etype
Simon Glass (3):
binman: Renumber 291 and 292 test files
buildman: Exit after reading toolchain
buildman: Drop warning about orphaned defconfigs
common/event.c | 2 +-
tools/binman/bintools.rst | 2 +-
tools/binman/btool/bootgen.py | 137
tools/binman/entries.rst | 75 +
tools/binman/etype/xilinx_bootgen.py | 225
+++
tools/binman/ftest.py | 81 +-
tools/binman/test/307_xilinx_bootgen_sign.dts | 22 +++
tools/binman/test/308_xilinx_bootgen_sign_enc.dts | 24 +++
.../{291_template_phandle.dts => 309_template_phandle.dts} | 0
...2_template_phandle_dup.dts => 310_template_phandle_dup.dts} | 0
tools/buildman/boards.py | 7 -
tools/buildman/control.py | 3 +
tools/buildman/func_test.py| 9 +-
13 files changed, 569 insertions(+), 18 deletions(-)
create mode 100644 tools/binman/btool/bootgen.py
create mode 100644 tools/binman/etype/xilinx_bootgen.py
create mode 100644 tools/binman/test/307_xilinx_bootgen_sign.dts
create mode 100644 tools/binman/test/308_xilinx_bootgen_sign_enc.dts
rename tools/binman/test/{291_template_phandle.dts =>
309_template_phandle.dts} (100%)
rename tools/binman/test/{292_template_phandle_dup.dts =>
310_template_phandle_dup.dts} (100%)
Regards,
Simon
Re: [PATCH v7 09/11] sandbox: capsule: Generate capsule related files through binman
On Sat, Aug 05, 2023 at 09:04:00AM -0600, Simon Glass wrote:
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> >
> > The EFI capsule files can now be generated as part of u-boot
> > build through binman. Add capsule entry nodes in the u-boot.dtsi for
> > the sandbox architecture for generating the capsules. These capsules
> > are then used for testing the EFI capsule update functionality on the
> > sandbox platforms. Also add binman nodes for generating the input
> > files used for these capsules.
> >
> > Remove the corresponding logic which was used for generation of these
> > input files which is now superfluous.
> >
> > Signed-off-by: Sughosh Ganu
> > ---
> > Changes since V6:
> > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule
> > input filenames.
> > * Generate the capsule input files through binman text entries.
> >
> > arch/sandbox/dts/u-boot.dtsi | 363 ++
> > include/sandbox_efi_capsule.h | 11 +
> > test/py/tests/test_efi_capsule/conftest.py| 168 +---
> > test/py/tests/test_efi_capsule/signature.dts | 10 -
> > .../tests/test_efi_capsule/uboot_bin_env.its | 36 --
> > 5 files changed, 385 insertions(+), 203 deletions(-)
> > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts
> > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
>
> I think you are still getting confused with using filenames when you
> don't need to. See below...
>
>
> >
> > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > index 60bd004937..f1b16b41c2 100644
> > --- a/arch/sandbox/dts/u-boot.dtsi
> > +++ b/arch/sandbox/dts/u-boot.dtsi
> > @@ -7,11 +7,374 @@
> > */
> >
> > #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > +
> > +#include
> > +
> > / {
> > #ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > signature {
> > capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > };
> > #endif
> > +
> > + binman: binman {
> > + multiple-images;
> > + };
> > +};
> > +
> > + {
> > + input1 {
> > + filename = UBOOT_BIN_IMAGE_OLD;
> > + text {
> > + text = "u-boot:Old";
> > + };
> > + };
> > +
> > + input2 {
> > + filename = UBOOT_BIN_IMAGE_NEW;
> > + text {
> > + text = "u-boot:New";
> > + };
> > + };
> > +
> > + input3 {
> > + filename = UBOOT_ENV_IMAGE_OLD;
> > + text {
> > + text = "u-boot-env:Old";
> > + };
> > + };
> > +
> > + input4 {
> > + filename = UBOOT_ENV_IMAGE_NEW;
> > + text {
> > + text = "u-boot-env:New";
> > + };
> > + };
>
> All of those nodes can be removed
>
> > +
> > + itb {
> > + filename = UBOOT_FIT_IMAGE;
> > +
> > + fit {
> > + description = "Automatic U-Boot environment update";
> > + #address-cells = <2>;
> > +
> > + images {
> > + u-boot-bin {
> > + description = "U-Boot binary on SPI
> > Flash";
> > + compression = "none";
> > + type = "firmware";
> > + arch = "sandbox";
> > + load = <0>;
> > + blob {
> > + filename =
> > UBOOT_BIN_IMAGE_NEW;
> > + };
>
> instead of this blob node, you should be able to write:
>
> text {
> text = "u-boot:Old";
> };
>
> There is no need to provide filenames in every node.
>
> > +
> > + hash-1 {
> > + algo = "sha1";
> > + };
> > + };
> > + u-boot-env {
> > + description = "U-Boot environment
> > on SPI Flash";
> > + compression = "none";
> > + type = "firmware";
> > + arch = "sandbox";
> > + load = <0>;
> > + blob {
> > + filename =
> > UBOOT_ENV_IMAGE_NEW;
> > + };
>
> same here and below
>
> > +
> > + hash-1 {
> > + algo = "sha1";
> > + };
> > + };
> > + };
Re: [PATCH v7 06/11] sandbox: Build the mkeficapsule tool for the sandbox variants
On Sat, Aug 05, 2023 at 09:03:49AM -0600, Simon Glass wrote: > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote: > > > > Build the mkeficapsule tool for all the sandbox variants. This tool > > will be used subsequently for testing capsule generation in binman. > > > > Signed-off-by: Sughosh Ganu > > --- > > Changes since V6: > > * New patch which has been split up from the binman capsule entry > > support patch from earlier version, as suggested by Simon Glass. > > * Enables mkeficapsule tool for all sandbox variants, instead of only > > for sandbox_spl variant. > > > > tools/Kconfig | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > Reviewed-by: Simon Glass > > At some point after this series is in, please take a look at why this > tool isn't enabled for all builds. The tool requires a bunch of stuff (see the rules in tools/Makefile). -- Tom signature.asc Description: PGP signature
Re: [PATCH v7 04/11] capsule: authenticate: Add capsule public key in platform's dtb
On Sat, Aug 05, 2023 at 09:03:53AM -0600, Simon Glass wrote:
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> >
> > The EFI capsule authentication logic in u-boot expects the public key
> > in the form of an EFI Signature List(ESL) to be provided as part of
> > the platform's dtb. Currently, the embedding of the ESL file into the
> > dtb needs to be done manually.
> >
> > Add a signature node in the u-boot dtsi file and include the public
> > key through the capsule-key property. This file is per architecture,
> > and is currently being added for sandbox and arm architectures. It
> > will have to be added for other architectures which need to enable
> > capsule authentication support.
> >
> > The path to the ESL file is specified through the
> > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> >
> > Signed-off-by: Sughosh Ganu
> > ---
> > Changes since V6:
> > * Populate the CONFIG_EFI_CAPSULE_ESL_FILE symbol for sandbox and
> > sandbox_flattree which enable capsule authentication.
> >
> > Note:
> > Simon Glass had asked me to rid of the CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > ifdef used in the sandbox' u-boot.dtsi file. However, that results in
> > the sandbox_vpl test failing in CI. Hence that check has been kept.
> >
> >
> > arch/arm/dts/u-boot.dtsi | 14 ++
> > arch/sandbox/dts/u-boot.dtsi | 17 +
We've already had some go-round as to why this basically identical file
can't be in like lib/efi_loader/ or something, yes?
> > configs/sandbox_defconfig | 1 +
> > configs/sandbox_flattree_defconfig | 1 +
> > lib/efi_loader/Kconfig | 9 +
> > 5 files changed, 42 insertions(+)
> > create mode 100644 arch/arm/dts/u-boot.dtsi
> > create mode 100644 arch/sandbox/dts/u-boot.dtsi
> >
> > diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> > new file mode 100644
> > index 00..4f31da4521
> > --- /dev/null
> > +++ b/arch/arm/dts/u-boot.dtsi
> > @@ -0,0 +1,14 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/**
> > + * Devicetree file with miscellaneous nodes that will be included
> > + * at build time into the DTB. Currently being used for including
> > + * capsule related information.
> > + */
> > +
> > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > +/ {
> > + signature {
> > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > + };
> > +};
> > +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > new file mode 100644
> > index 00..60bd004937
> > --- /dev/null
> > +++ b/arch/sandbox/dts/u-boot.dtsi
> > @@ -0,0 +1,17 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Devicetree file with miscellaneous nodes that will be included
> > + * at build time into the DTB. Currently being used for including
> > + * capsule related information.
> > + *
> > + */
> > +
> > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > +/ {
> > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > + signature {
> > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > + };
> > +#endif
> > +};
> > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
And why are these two different?
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index a22e47616f..0d559ff3a1 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -235,6 +235,15 @@ config EFI_CAPSULE_MAX
> > Select the max capsule index value used for capsule report
> > variables. This value is used to create CapsuleMax variable.
> >
> > +config EFI_CAPSULE_ESL_FILE
> > + string "Path to the EFI Signature List File"
> > + default ""
No default.
> > + depends on EFI_CAPSULE_AUTHENTICATE
> > + help
> > + Provides the absolute path to the EFI Signature List file which
>
> It isn't really an absolute path as it doesn't start with /
>
> You really can't/shouldn't use absolute paths in a U-Boot build.
You can't as it will break reproducible builds (or make them harder than
required).
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 3/7] power: regulator-uclass: perform regulator setup inside uclass
On 2023-08-05 21:58, Svyatoslav Ryhel wrote:
>
>
> 5 серпня 2023 р. 15:49:32 GMT+03:00, Jonas Karlman
> написав(-ла):
>> Hi,
>>
>> On 2023-07-21 07:34, Svyatoslav Ryhel wrote:
>>> чт, 20 лип. 2023 р. о 22:43 Simon Glass пише:
Hi Svyatoslav,
On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel wrote:
>
> Regulators initial setup was previously dependent on board call.
> To move from this behaviour were introduced two steps. First is
> that all individual regulators will be probed just after binding
We must not probe devices automatically when bound. The i2c interface
may not be available, etc. Do a probe afterwards.
Perhaps I am misunderstanding this, iwc please reword this commit message.
>>>
>>> After bound. If the regulator is a self-sufficient i2c device then it
>>> will be bound
>>> after i2c is available by code design so i2c interface should be
>>> available at that
>>> moment. At least led and gpio uclasses perform this for initial setup
>>> of devices.
>>>
>>> Platform regulators (aka fixed/gpio regulators) work perfectly fine. I
>>> have no i2c
>>> regulators to test deeply.
>>>
>>> As for now only one uclass is not compatible with this system - PMIC which
>>> has
>>> strong dependency between regulator and main mfd. This is why probing after
>>> bind for pmic regulators is disabled and pmic regulators are probed on pmic
>>> core
>>> post_probe.
>>
>> This sounds more like a possible bug of some dependency not being
>> probed in correct order or not leaving the device in a ready state
>> after probe.
>>
>> If pmic regulators are bind in pmic bind with the pmic as parent, then
>> at regulator probe the driver core ensure that pmic has probed before
>> regulator use.
>>
>> This works perfect fine with the RK8xx I2C PMIC and its regulators.
>> Wich a call to device_get_supply_regulator(dev, "test-supply", )
>> probe happens in i2c, pmic and regulator order.
>>
>
> I will check and re-test drivers I have ASAP.
>
>>>
> which ensures that regulator pdata is filled and second is setting
> up regulator in post probe which enseres that correct regulator
> state according to pdata is reached.
>>
>> I think that the approach in this patch is trying to change too much all
>> at once.
>>
>> Have made some adjustments that I think should help with transition.
>> - Added a flag to protect regulator_autoset from being called more then
>> once for each regulator, in a separate patch.
>
> It is not a good decision in the long run and you should keep in mind that
> there is nothing more constant than a temporary solution.
Nor do I propose this to be a long-term solution, only that it is more
reviewable to see changes in non-breaking smaller steps. In current
state this patch breaks building U-Boot and requires the last patch to
fix build again.
Anyway, I will be posting a similar patch for autoset as linked below as
part of a series to add a Rockchip IO-domain driver shortly. In current
state autoset cannot safely be called multiple times, and such patch
should not have an impact on the direction of this series.
>
>> - Changed to only probe-after-bind on regulators marked as
>> always-on/boot-on.
>>
>> And it works something like this:
>>
>> static int regulator_post_bind(struct udevice *dev)
>> {
>> [...]
>>
>> if (uc_pdata->always_on || uc_pdata->boot_on)
>> dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
>> }
>>
>> static int regulator_post_probe(struct udevice *dev)
>> {
>> ret = regulator_autoset(dev);
>> }
>>
>> With that any always-on/boot-on regulator would automatically probe and
>> autoset after bind, remaining would only probe and autoset if they are
>> used.
>
> uc_pdata is filled in pre_probe, while you are runing check in post_bind.
Please look closer at the code and not the snippet above, they are
filled in post_bind or the code would not have worked :-)
Regards,
Jonas
>
>> This approach should also prevent having to change existing code that
>> may call autoset, and cleanup can be done in follow-up patches/series.
>>
>> Probe-after-bind and call to autoset could also be protected with a new
>> Kconfig to help with transition.
>>
>> See following for a working example using a new driver that depends
>> on that regulators have been autoset prior to regulator_get_value.
>> https://github.com/Kwiboo/u-boot-rockchip/compare/master...rk3568-io-domain
>>
>> Or the two commits separate:
>>
>> power: regulator: Only run autoset once for each regulator
>> https://github.com/Kwiboo/u-boot-rockchip/commit/05db4dbcb8f908b417ed5cae8a7a325c82112d75
>>
>> power: regulator: Perform regulator setup inside uclass
>> https://github.com/Kwiboo/u-boot-rockchip/commit/489bd5d2c1a2a33824eac4f2d54399ef5dff4fdf
>>
>> Regards,
>> Jonas
>>
>
> Signed-off-by: Svyatoslav Ryhel
> ---
> drivers/power/regulator/regulator-uclass.c | 212 ++---
> include/power/regulator.h
Re: [PATCH v2 3/7] power: regulator-uclass: perform regulator setup inside uclass
5 серпня 2023 р. 15:49:32 GMT+03:00, Jonas Karlman
написав(-ла):
>Hi,
>
>On 2023-07-21 07:34, Svyatoslav Ryhel wrote:
>> чт, 20 лип. 2023 р. о 22:43 Simon Glass пише:
>>>
>>> Hi Svyatoslav,
>>>
>>> On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel wrote:
Regulators initial setup was previously dependent on board call.
To move from this behaviour were introduced two steps. First is
that all individual regulators will be probed just after binding
>>>
>>> We must not probe devices automatically when bound. The i2c interface
>>> may not be available, etc. Do a probe afterwards.
>>>
>>> Perhaps I am misunderstanding this, iwc please reword this commit message.
>>
>> After bound. If the regulator is a self-sufficient i2c device then it
>> will be bound
>> after i2c is available by code design so i2c interface should be
>> available at that
>> moment. At least led and gpio uclasses perform this for initial setup
>> of devices.
>>
>> Platform regulators (aka fixed/gpio regulators) work perfectly fine. I
>> have no i2c
>> regulators to test deeply.
>>
>> As for now only one uclass is not compatible with this system - PMIC which
>> has
>> strong dependency between regulator and main mfd. This is why probing after
>> bind for pmic regulators is disabled and pmic regulators are probed on pmic
>> core
>> post_probe.
>
>This sounds more like a possible bug of some dependency not being
>probed in correct order or not leaving the device in a ready state
>after probe.
>
>If pmic regulators are bind in pmic bind with the pmic as parent, then
>at regulator probe the driver core ensure that pmic has probed before
>regulator use.
>
>This works perfect fine with the RK8xx I2C PMIC and its regulators.
>Wich a call to device_get_supply_regulator(dev, "test-supply", )
>probe happens in i2c, pmic and regulator order.
>
I will check and re-test drivers I have ASAP.
>>
which ensures that regulator pdata is filled and second is setting
up regulator in post probe which enseres that correct regulator
state according to pdata is reached.
>
>I think that the approach in this patch is trying to change too much all
>at once.
>
>Have made some adjustments that I think should help with transition.
>- Added a flag to protect regulator_autoset from being called more then
> once for each regulator, in a separate patch.
It is not a good decision in the long run and you should keep in mind that
there is nothing more constant than a temporary solution.
>- Changed to only probe-after-bind on regulators marked as
> always-on/boot-on.
>
>And it works something like this:
>
>static int regulator_post_bind(struct udevice *dev)
>{
> [...]
>
> if (uc_pdata->always_on || uc_pdata->boot_on)
> dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
>}
>
>static int regulator_post_probe(struct udevice *dev)
>{
> ret = regulator_autoset(dev);
>}
>
>With that any always-on/boot-on regulator would automatically probe and
>autoset after bind, remaining would only probe and autoset if they are
>used.
uc_pdata is filled in pre_probe, while you are runing check in post_bind.
>This approach should also prevent having to change existing code that
>may call autoset, and cleanup can be done in follow-up patches/series.
>
>Probe-after-bind and call to autoset could also be protected with a new
>Kconfig to help with transition.
>
>See following for a working example using a new driver that depends
>on that regulators have been autoset prior to regulator_get_value.
>https://github.com/Kwiboo/u-boot-rockchip/compare/master...rk3568-io-domain
>
>Or the two commits separate:
>
>power: regulator: Only run autoset once for each regulator
>https://github.com/Kwiboo/u-boot-rockchip/commit/05db4dbcb8f908b417ed5cae8a7a325c82112d75
>
>power: regulator: Perform regulator setup inside uclass
>https://github.com/Kwiboo/u-boot-rockchip/commit/489bd5d2c1a2a33824eac4f2d54399ef5dff4fdf
>
>Regards,
>Jonas
>
Signed-off-by: Svyatoslav Ryhel
---
drivers/power/regulator/regulator-uclass.c | 212 ++---
include/power/regulator.h | 119
2 files changed, 62 insertions(+), 269 deletions(-)
>>>
>>> Regards,
>>> SImon
>
Re: [PATCH v7 09/11] sandbox: capsule: Generate capsule related files through binman
Hi Sughosh, On Sat, 5 Aug 2023 at 13:12, Sughosh Ganu wrote: > > hi Simon, > > On Sun, 6 Aug 2023 at 00:06, Simon Glass wrote: > > > > Hi Sughosh, > > > > On Sat, 5 Aug 2023 at 12:01, Sughosh Ganu wrote: > > > > > > hi Simon, > > > > > > On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu > > > > wrote: > > > > > > > > > > The EFI capsule files can now be generated as part of u-boot > > > > > build through binman. Add capsule entry nodes in the u-boot.dtsi for > > > > > the sandbox architecture for generating the capsules. These capsules > > > > > are then used for testing the EFI capsule update functionality on the > > > > > sandbox platforms. Also add binman nodes for generating the input > > > > > files used for these capsules. > > > > > > > > > > Remove the corresponding logic which was used for generation of these > > > > > input files which is now superfluous. > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > --- > > > > > Changes since V6: > > > > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > > > > > input filenames. > > > > > * Generate the capsule input files through binman text entries. > > > > > > > > > > arch/sandbox/dts/u-boot.dtsi | 363 > > > > > ++ > > > > > include/sandbox_efi_capsule.h | 11 + > > > > > test/py/tests/test_efi_capsule/conftest.py| 168 +--- > > > > > test/py/tests/test_efi_capsule/signature.dts | 10 - > > > > > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > > > > > 5 files changed, 385 insertions(+), 203 deletions(-) > > > > > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > > > > > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its > > > > > > > > I think you are still getting confused with using filenames when you > > > > don't need to. See below... > > > > > > No, I don't think so. This is being done on purpose, since I want > > > these files to be created. These are then copied to the efi capsule > > > update tests' data directory, and are then used in testing the > > > feature. Maybe if you want, I can put a comment to that effect. > > > > I just traced through this code. I really think this needs to be > > simplified. You can do it in a patch at the end if you like. > > > > But you have the 'u-boot.bin.old' and 'Old' strings in > > test_efi_capsule_auth2, for example. > > > > In the binman world you define UBOOT_BIN_IMAGE_OLD as > > "u-boot.bin.old", then use that in the sandbox.dtsi > > > > Then binman creates the u-boot.bin.old file (unnecessarily in my view) > > Then in efi_capsule_data() you copy the file to the test directory. > > > > So for the last step, you could just create the file again, rather > > than copying it from the U-Boot build directory. After all, you know > > the contents. If you like you could put the different contents as > > binary strings in capsule_defs.py > > > > Then you don't need to create the files. > > Okay. TBH, I thought you would ask me to reuse the files created in > binman for the tests as well, which is why I put this logic. I will > create these files as part of the tests then. > > > > > There is a lot more I can say about the EFI capsule tests. For now I > > think we'll have to live with it creating 19 different binman images > > on sandbox. I think we could put them in an efi_capsules subdir, but > > that would need to be created somewhere, since binman doesn't do it. I > > suppose we could make binman automatically create a directory if an > > entry filename needs it? > > I think this can be taken up as a follow-up. I also was thinking of > adding a flag/property to not generate all the map files. I don't > think such a property exists currently. The map files really are not > needed for the capsules. Sure, but if you implemented SetImagePos() then it would work. Something to think about when you create the tool to dump capsules. > > > > > Anyway, for tests, primarily we need to split things up, so we have, > > for example: > > > > process_capsule_file() > > > > which processes the capsule in U-Boot, e.g. using an 'efi' command, > > then outputs what it did. Then the test can plant the capsule, run > > that function and check that the output is as expected. This can > > actually be a unit test, i.e. written in C. > > > > Most of the tests can do this. Then we can have one test that checks > > the whole flow, but it doesn't need to be done for every case, as now. > > I believe even Ilias thinks that these tests should be in C. But this > is a separate effort, not related to this series. I also have doubts > about your observation on not using so many capsule files for tests, > but that can be investigated separately. For now, I want to focus on > getting these changes in, followed by the generation of capsules > through the config file. And FWIW, I am able to use relative paths in > the binman
Re: [PATCH v7 08/11] binman: capsule: Add support for generating EFI capsules
Hi Sughosh, On Sat, 5 Aug 2023 at 13:35, Sughosh Ganu wrote: > > hi Simon, > > On Sun, 6 Aug 2023 at 00:35, Simon Glass wrote: > > > > Hi Sughosh, > > > > On Sat, 5 Aug 2023 at 12:42, Sughosh Ganu wrote: > > > > > > hi Simon, > > > > > > On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu > > > > wrote: > > > > > > > > > > Add support in binman for generating EFI capsules. The capsule > > > > > parameters can be specified through the capsule binman entry. Also add > > > > > test cases in binman for testing capsule generation. > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > --- > > > > > Changes since V6: > > > > > * Add macros for the GUID strings in sandbox_efi_capsule.h > > > > > * Highlight that the private and public keys are mandatory for capsule > > > > > signing. > > > > > * Add a URL link to the UEFI spec, as used in the rst files. > > > > > * Use local vars for private and public keys in BuildSectionData() > > > > > * Use local vars for input payload and capsule filenames in > > > > > BuildSectionData(). > > > > > * Drop the ProcessContents() and SetImagePos() as the superclass > > > > > functions suffice. > > > > > * Use GUID macro names in the capsule test dts files. > > > > > * Rename efi_capsule_payload.bin to capsule_input.bin. > > > > > > > > > > > > > > > include/sandbox_efi_capsule.h | 14 ++ > > > > > > > > Please move this file to a later patch - see below. > > > > > > The idea was to also be able to run the binman capsule tests once this > > > patch was applied. If we are to move this to a separate patch, it > > > should be the one before this patch. But I guess based on your other > > > reply, this might not be needed after all. > > > > Yes, it should not be needed if we name the test GUIDs. Remember that > > binman is a standalone tool so cannot reference files outside > > tools/...although there is no test for that so some things may have > > crept in. > > > > > > > > > > > > > Could we have a single header file with all the GUIDs, i.e. sandbox, > > > > ARM, etc. > > > > > > Umm, I am not too sure. Maybe we can take a call at a later point if > > > there are too many files that start cropping up. > > > > OK > > > > > > > > > > > > > > tools/binman/entries.rst | 62 > > > > > tools/binman/etype/efi_capsule.py | 143 > > > > > ++ > > > > > tools/binman/ftest.py | 121 +++ > > > > > tools/binman/test/307_capsule.dts | 23 +++ > > > > > tools/binman/test/308_capsule_signed.dts | 25 +++ > > > > > tools/binman/test/309_capsule_version.dts | 24 +++ > > > > > tools/binman/test/310_capsule_signed_ver.dts | 26 > > > > > tools/binman/test/311_capsule_oemflags.dts| 24 +++ > > > > > tools/binman/test/312_capsule_missing_key.dts | 24 +++ > > > > > .../binman/test/313_capsule_missing_index.dts | 22 +++ > > > > > .../binman/test/314_capsule_missing_guid.dts | 19 +++ > > > > > .../test/315_capsule_missing_payload.dts | 19 +++ > > > > > 13 files changed, 546 insertions(+) > > > > > create mode 100644 include/sandbox_efi_capsule.h > > > > > create mode 100644 tools/binman/etype/efi_capsule.py > > > > > create mode 100644 tools/binman/test/307_capsule.dts > > > > > create mode 100644 tools/binman/test/308_capsule_signed.dts > > > > > create mode 100644 tools/binman/test/309_capsule_version.dts > > > > > create mode 100644 tools/binman/test/310_capsule_signed_ver.dts > > > > > create mode 100644 tools/binman/test/311_capsule_oemflags.dts > > > > > create mode 100644 tools/binman/test/312_capsule_missing_key.dts > > > > > create mode 100644 tools/binman/test/313_capsule_missing_index.dts > > > > > create mode 100644 tools/binman/test/314_capsule_missing_guid.dts > > > > > create mode 100644 tools/binman/test/315_capsule_missing_payload.dts > > > > > > > > > [..] > > > > > > > + > > > > > +def ReadNode(self): > > > > > +self.ReadEntries() > > > > > +super().ReadNode() > > > > > > > > I believe those two lines should be swapped. > > > > > > Again, like my earlier code for ProcessContents() and SetImagePos(), > > > which was taken from mkimage.py as reference, this code is on similar > > > lines to what is in intel_ifwi.py. Both these files are authored by > > > you, so I took this as reference, especially mkimage.py. > > > > OK, then take a look at mkimage.py and follow that. Yes intel_ifwi is > > around the wrong way. Although these days ReadEntries() is called > > automatically from entry_Section so you don't need to call it here. > > > > > > > > > > > > > > + > > > > > +self.image_index = fdt_util.GetInt(self._node, 'image-index') > > > > > +self.image_guid = fdt_util.GetString(self._node, > > > > > 'image-type-id') > > > > > +self.fw_version = fdt_util.GetInt(self._node,
Re: [PATCH] event: Fix an wrong type_name from dm_post_init to dm_post_init_f
On Tue, 1 Aug 2023 at 04:17, Jaehoon Chung wrote: > > DM_POST_INIT was changed to DM_POST_INIT_F. > To debug correct message, change type_name from dm_post_init to > dm_post_init_f. > > Signed-off-by: Jaehoon Chung > --- > common/event.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Simon Glass Applied to u-boot-dm, thanks!
Re: [PATCH v4 1/3] binman: btool: Add Xilinx Bootgen btool
On Thu, 3 Aug 2023 at 09:22, wrote: > > From: Lukas Funke > > Add the Xilinx Bootgen as bintool. Xilinx Bootgen is used to create > bootable SPL (FSBL in Xilinx terms) images for Zynq/ZynqMP devices. The > btool creates a signed version of the SPL. Additionally to signing the > key source for the decryption engine can be passend to the boot image. > > Signed-off-by: Lukas Funke > > --- > > Changes in v4: > - Fixed some typos > > Changes in v3: > - Fixed an issue where the build result was not found > - Fixed an issue where the version string was not reported correctly > > Changes in v2: > - Pass additional 'keysrc_enc' parameter to Bootgen > - Added more information and terms to documentation > > tools/binman/bintools.rst | 2 +- > tools/binman/btool/bootgen.py | 137 ++ > 2 files changed, 138 insertions(+), 1 deletion(-) > create mode 100644 tools/binman/btool/bootgen.py > Reviewed-by: Simon Glass Applied to u-boot-dm, thanks!
Re: [PATCH v4 2/3] binman: ftest: Add test for xilinx-bootgen etype
From: Lukas Funke Add test for the 'xilinx-bootgen' etype Signed-off-by: Lukas Funke Reviewed-by: Simon Glass --- Changes in v4: - Add test to check for missing bootgen tool Changes in v3: - Improved test coverage for xilinx-fsbl-auth etype Changes in v2: - Fixed typo in dts name tools/binman/ftest.py | 75 +++ tools/binman/test/307_xilinx_bootgen_sign.dts | 22 ++ .../test/308_xilinx_bootgen_sign_enc.dts | 24 ++ 3 files changed, 121 insertions(+) create mode 100644 tools/binman/test/307_xilinx_bootgen_sign.dts create mode 100644 tools/binman/test/308_xilinx_bootgen_sign_enc.dts Applied to u-boot-dm, thanks!
Re: [PATCH v4 3/3] binman: etype: Add xilinx-bootgen etype
From: Lukas Funke
This adds a new etype 'xilinx-bootgen'. By using this etype it is
possible to created an signed SPL (FSBL in Xilinx terms) for
ZynqMP boards.
The etype uses Xilinx Bootgen tools in order to transform the SPL into
a bootable image and sign the image with a given primary and secondary
public key. For more information to signing the FSBL please refer to the
Xilinx Bootgen documentation.
Here is an example of the etype in use:
spl {
filename = "boot.signed.bin";
xilinx-bootgen {
pmufw-filename = "pmu-firmware.elf";
psk-key-name-hint = "psk0";
ssk-key-name-hint = "ssk0";
auth-params = "ppk_select=0", "spk_id=0x";
u-boot-spl-nodtb {
};
u-boot-spl-dtb {
};
};
};
For this to work the hash of the primary public key has to be fused
into the ZynqMP device and authentication (RSA_EN) has to be set.
For testing purposes: if ppk hash check should be skipped one can add
the property 'fsbl_config = "bh_auth_enable";' to the etype. However,
this should only be used for testing(!).
Signed-off-by: Lukas Funke
Reviewed-by: Simon Glass
---
Changes in v4:
- Renamed etype from "xilinx-fsbl-auth" to "xilinx-bootgen"
- Add detection of missing bintool
- Promote 'pmufw-filename' to required property
Changes in v3:
- Changed etype from entry to section
- Changed property name "psk-filename" to "psk-key-name-hint"
- Changed property name "ssk-filename" to "ssk-key-name-hint"
- Decode spl elf file instead of reading start symbol
- Improved test coverage
- Improved documentation
Changes in v2:
- Add 'keysrc-enc' property to pass down to Bootgen
- Improved documentation
- Use predictable output names for intermediated results
tools/binman/entries.rst | 75 +
tools/binman/etype/xilinx_bootgen.py | 225 +++
2 files changed, 300 insertions(+)
create mode 100644 tools/binman/etype/xilinx_bootgen.py
Applied to u-boot-dm, thanks!
Re: [PATCH 1/2] buildman: Exit after reading toolchain
Recent refactoring changed buildman to continue operation after fetching
a toolchain. Fix this.
Fixes: b8680646521 ("bulidman: Move toolchain handling to a function")
Signed-off-by: Simon Glass
---
tools/buildman/control.py | 3 +++
1 file changed, 3 insertions(+)
Applied to u-boot-dm, thanks!
Re: [PATCH 2/2] buildman: Drop warning about orphaned defconfigs
Some boards use a MAINTAINERS entry to specify common files without referencing any defconfigs. This is allowed and should not result in a warning. Drop the warning in this case. Signed-off-by: Simon Glass --- tools/buildman/boards.py| 7 --- tools/buildman/func_test.py | 9 ++--- 2 files changed, 2 insertions(+), 14 deletions(-) Applied to u-boot-dm, thanks!
Re: [PATCH] binman: Renumber 291 and 292 test files
These have ended up with the same numbers as earlier files. Fix them.
Signed-off-by: Simon Glass
---
tools/binman/ftest.py | 4 ++--
.../{291_template_phandle.dts => 309_template_phandle.dts}| 0
..._template_phandle_dup.dts => 310_template_phandle_dup.dts} | 0
3 files changed, 2 insertions(+), 2 deletions(-)
rename tools/binman/test/{291_template_phandle.dts =>
309_template_phandle.dts} (100%)
rename tools/binman/test/{292_template_phandle_dup.dts =>
310_template_phandle_dup.dts} (100%)
Applied to u-boot-dm, thanks!
Re: [PATCH v7 08/11] binman: capsule: Add support for generating EFI capsules
hi Simon, On Sun, 6 Aug 2023 at 00:35, Simon Glass wrote: > > Hi Sughosh, > > On Sat, 5 Aug 2023 at 12:42, Sughosh Ganu wrote: > > > > hi Simon, > > > > On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote: > > > > > > Hi Sughosh, > > > > > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote: > > > > > > > > Add support in binman for generating EFI capsules. The capsule > > > > parameters can be specified through the capsule binman entry. Also add > > > > test cases in binman for testing capsule generation. > > > > > > > > Signed-off-by: Sughosh Ganu > > > > --- > > > > Changes since V6: > > > > * Add macros for the GUID strings in sandbox_efi_capsule.h > > > > * Highlight that the private and public keys are mandatory for capsule > > > > signing. > > > > * Add a URL link to the UEFI spec, as used in the rst files. > > > > * Use local vars for private and public keys in BuildSectionData() > > > > * Use local vars for input payload and capsule filenames in > > > > BuildSectionData(). > > > > * Drop the ProcessContents() and SetImagePos() as the superclass > > > > functions suffice. > > > > * Use GUID macro names in the capsule test dts files. > > > > * Rename efi_capsule_payload.bin to capsule_input.bin. > > > > > > > > > > > > include/sandbox_efi_capsule.h | 14 ++ > > > > > > Please move this file to a later patch - see below. > > > > The idea was to also be able to run the binman capsule tests once this > > patch was applied. If we are to move this to a separate patch, it > > should be the one before this patch. But I guess based on your other > > reply, this might not be needed after all. > > Yes, it should not be needed if we name the test GUIDs. Remember that > binman is a standalone tool so cannot reference files outside > tools/...although there is no test for that so some things may have > crept in. > > > > > > > > > Could we have a single header file with all the GUIDs, i.e. sandbox, ARM, > > > etc. > > > > Umm, I am not too sure. Maybe we can take a call at a later point if > > there are too many files that start cropping up. > > OK > > > > > > > > > > tools/binman/entries.rst | 62 > > > > tools/binman/etype/efi_capsule.py | 143 ++ > > > > tools/binman/ftest.py | 121 +++ > > > > tools/binman/test/307_capsule.dts | 23 +++ > > > > tools/binman/test/308_capsule_signed.dts | 25 +++ > > > > tools/binman/test/309_capsule_version.dts | 24 +++ > > > > tools/binman/test/310_capsule_signed_ver.dts | 26 > > > > tools/binman/test/311_capsule_oemflags.dts| 24 +++ > > > > tools/binman/test/312_capsule_missing_key.dts | 24 +++ > > > > .../binman/test/313_capsule_missing_index.dts | 22 +++ > > > > .../binman/test/314_capsule_missing_guid.dts | 19 +++ > > > > .../test/315_capsule_missing_payload.dts | 19 +++ > > > > 13 files changed, 546 insertions(+) > > > > create mode 100644 include/sandbox_efi_capsule.h > > > > create mode 100644 tools/binman/etype/efi_capsule.py > > > > create mode 100644 tools/binman/test/307_capsule.dts > > > > create mode 100644 tools/binman/test/308_capsule_signed.dts > > > > create mode 100644 tools/binman/test/309_capsule_version.dts > > > > create mode 100644 tools/binman/test/310_capsule_signed_ver.dts > > > > create mode 100644 tools/binman/test/311_capsule_oemflags.dts > > > > create mode 100644 tools/binman/test/312_capsule_missing_key.dts > > > > create mode 100644 tools/binman/test/313_capsule_missing_index.dts > > > > create mode 100644 tools/binman/test/314_capsule_missing_guid.dts > > > > create mode 100644 tools/binman/test/315_capsule_missing_payload.dts > > > > > > [..] > > > > > + > > > > +def ReadNode(self): > > > > +self.ReadEntries() > > > > +super().ReadNode() > > > > > > I believe those two lines should be swapped. > > > > Again, like my earlier code for ProcessContents() and SetImagePos(), > > which was taken from mkimage.py as reference, this code is on similar > > lines to what is in intel_ifwi.py. Both these files are authored by > > you, so I took this as reference, especially mkimage.py. > > OK, then take a look at mkimage.py and follow that. Yes intel_ifwi is > around the wrong way. Although these days ReadEntries() is called > automatically from entry_Section so you don't need to call it here. > > > > > > > > > > + > > > > +self.image_index = fdt_util.GetInt(self._node, 'image-index') > > > > +self.image_guid = fdt_util.GetString(self._node, > > > > 'image-type-id') > > > > +self.fw_version = fdt_util.GetInt(self._node, 'fw-version') > > > > +self.hardware_instance = fdt_util.GetInt(self._node, > > > > 'hardware-instance') > > > > +self.monotonic_count = fdt_util.GetInt(self._node, > > > > 'monotonic-count') > > > > +self.oem_flags = fdt_util.GetInt(self._node, 'oem-flags') > > > > + > > > > +
Re: [PATCH v7 04/11] capsule: authenticate: Add capsule public key in platform's dtb
hi Simon,
On Sun, 6 Aug 2023 at 00:35, Simon Glass wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 12:47, Sughosh Ganu wrote:
> >
> > hi Simon,
> >
> > On Sun, 6 Aug 2023 at 00:06, Simon Glass wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Sat, 5 Aug 2023 at 11:54, Sughosh Ganu wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu
> > > > > wrote:
> > > > > >
> > > > > > The EFI capsule authentication logic in u-boot expects the public
> > > > > > key
> > > > > > in the form of an EFI Signature List(ESL) to be provided as part of
> > > > > > the platform's dtb. Currently, the embedding of the ESL file into
> > > > > > the
> > > > > > dtb needs to be done manually.
> > > > > >
> > > > > > Add a signature node in the u-boot dtsi file and include the public
> > > > > > key through the capsule-key property. This file is per architecture,
> > > > > > and is currently being added for sandbox and arm architectures. It
> > > > > > will have to be added for other architectures which need to enable
> > > > > > capsule authentication support.
> > > > > >
> > > > > > The path to the ESL file is specified through the
> > > > > > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> > > > > >
> > > > > > Signed-off-by: Sughosh Ganu
> > > > > > ---
> > > > > > Changes since V6:
> > > > > > * Populate the CONFIG_EFI_CAPSULE_ESL_FILE symbol for sandbox and
> > > > > > sandbox_flattree which enable capsule authentication.
> > > > > >
> > > > > > Note:
> > > > > > Simon Glass had asked me to rid of the
> > > > > > CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > > > > ifdef used in the sandbox' u-boot.dtsi file. However, that results
> > > > > > in
> > > > > > the sandbox_vpl test failing in CI. Hence that check has been kept.
> > > > > >
> > > > > >
> > > > > > arch/arm/dts/u-boot.dtsi | 14 ++
> > > > > > arch/sandbox/dts/u-boot.dtsi | 17 +
> > > > > > configs/sandbox_defconfig | 1 +
> > > > > > configs/sandbox_flattree_defconfig | 1 +
> > > > > > lib/efi_loader/Kconfig | 9 +
> > > > > > 5 files changed, 42 insertions(+)
> > > > > > create mode 100644 arch/arm/dts/u-boot.dtsi
> > > > > > create mode 100644 arch/sandbox/dts/u-boot.dtsi
> > > > > >
> > > > > > diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> > > > > > new file mode 100644
> > > > > > index 00..4f31da4521
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm/dts/u-boot.dtsi
> > > > > > @@ -0,0 +1,14 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > +/**
> > > > > > + * Devicetree file with miscellaneous nodes that will be included
> > > > > > + * at build time into the DTB. Currently being used for including
> > > > > > + * capsule related information.
> > > > > > + */
> > > > > > +
> > > > > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > > > > +/ {
> > > > > > + signature {
> > > > > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > > > > + };
> > > > > > +};
> > > > > > +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> > > > > > diff --git a/arch/sandbox/dts/u-boot.dtsi
> > > > > > b/arch/sandbox/dts/u-boot.dtsi
> > > > > > new file mode 100644
> > > > > > index 00..60bd004937
> > > > > > --- /dev/null
> > > > > > +++ b/arch/sandbox/dts/u-boot.dtsi
> > > > > > @@ -0,0 +1,17 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > +/*
> > > > > > + * Devicetree file with miscellaneous nodes that will be included
> > > > > > + * at build time into the DTB. Currently being used for including
> > > > > > + * capsule related information.
> > > > > > + *
> > > > > > + */
> > > > > > +
> > > > > > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > > > > +/ {
> > > > > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > > > > + signature {
> > > > > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > > > > + };
> > > > > > +#endif
> > > > > > +};
> > > > > > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > > > > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > > > > > index b6c4f735f2..779af4abc8 100644
> > > > > > --- a/configs/sandbox_defconfig
> > > > > > +++ b/configs/sandbox_defconfig
> > > > > > @@ -341,6 +341,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> > > > > > CONFIG_EFI_CAPSULE_ON_DISK=y
> > > > > > CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > > > > > CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> > > > > > +CONFIG_EFI_CAPSULE_ESL_FILE="../../../board/sandbox/SIGNER.esl"
> > > > >
> > > > > Can we avoid the path here, and just use e.g. good.esl ?
> > > >
> > > > Unfortunately no. I believe the way incbin works in dts is that it
> > > > looks for the binary to be included in the same directory as the dts.
> > > > Which is why I have to point it to the location of the esl relative to
> > > > the dts.
> > > >
> > > > >
> >
Re: [PATCH v7 09/11] sandbox: capsule: Generate capsule related files through binman
hi Simon, On Sun, 6 Aug 2023 at 00:06, Simon Glass wrote: > > Hi Sughosh, > > On Sat, 5 Aug 2023 at 12:01, Sughosh Ganu wrote: > > > > hi Simon, > > > > On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote: > > > > > > Hi Sughosh, > > > > > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote: > > > > > > > > The EFI capsule files can now be generated as part of u-boot > > > > build through binman. Add capsule entry nodes in the u-boot.dtsi for > > > > the sandbox architecture for generating the capsules. These capsules > > > > are then used for testing the EFI capsule update functionality on the > > > > sandbox platforms. Also add binman nodes for generating the input > > > > files used for these capsules. > > > > > > > > Remove the corresponding logic which was used for generation of these > > > > input files which is now superfluous. > > > > > > > > Signed-off-by: Sughosh Ganu > > > > --- > > > > Changes since V6: > > > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > > > > input filenames. > > > > * Generate the capsule input files through binman text entries. > > > > > > > > arch/sandbox/dts/u-boot.dtsi | 363 ++ > > > > include/sandbox_efi_capsule.h | 11 + > > > > test/py/tests/test_efi_capsule/conftest.py| 168 +--- > > > > test/py/tests/test_efi_capsule/signature.dts | 10 - > > > > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > > > > 5 files changed, 385 insertions(+), 203 deletions(-) > > > > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > > > > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its > > > > > > I think you are still getting confused with using filenames when you > > > don't need to. See below... > > > > No, I don't think so. This is being done on purpose, since I want > > these files to be created. These are then copied to the efi capsule > > update tests' data directory, and are then used in testing the > > feature. Maybe if you want, I can put a comment to that effect. > > I just traced through this code. I really think this needs to be > simplified. You can do it in a patch at the end if you like. > > But you have the 'u-boot.bin.old' and 'Old' strings in > test_efi_capsule_auth2, for example. > > In the binman world you define UBOOT_BIN_IMAGE_OLD as > "u-boot.bin.old", then use that in the sandbox.dtsi > > Then binman creates the u-boot.bin.old file (unnecessarily in my view) > Then in efi_capsule_data() you copy the file to the test directory. > > So for the last step, you could just create the file again, rather > than copying it from the U-Boot build directory. After all, you know > the contents. If you like you could put the different contents as > binary strings in capsule_defs.py > > Then you don't need to create the files. Okay. TBH, I thought you would ask me to reuse the files created in binman for the tests as well, which is why I put this logic. I will create these files as part of the tests then. > > There is a lot more I can say about the EFI capsule tests. For now I > think we'll have to live with it creating 19 different binman images > on sandbox. I think we could put them in an efi_capsules subdir, but > that would need to be created somewhere, since binman doesn't do it. I > suppose we could make binman automatically create a directory if an > entry filename needs it? I think this can be taken up as a follow-up. I also was thinking of adding a flag/property to not generate all the map files. I don't think such a property exists currently. The map files really are not needed for the capsules. > > Anyway, for tests, primarily we need to split things up, so we have, > for example: > > process_capsule_file() > > which processes the capsule in U-Boot, e.g. using an 'efi' command, > then outputs what it did. Then the test can plant the capsule, run > that function and check that the output is as expected. This can > actually be a unit test, i.e. written in C. > > Most of the tests can do this. Then we can have one test that checks > the whole flow, but it doesn't need to be done for every case, as now. I believe even Ilias thinks that these tests should be in C. But this is a separate effort, not related to this series. I also have doubts about your observation on not using so many capsule files for tests, but that can be investigated separately. For now, I want to focus on getting these changes in, followed by the generation of capsules through the config file. And FWIW, I am able to use relative paths in the binman tests for generating and testing the capsules generated through the config file -- so that takes care of your objection to using absolute paths. I will send that series once this gets merged. -sughosh > > Regards, > Simon
Re: [PATCH v7 03/11] sandbox: capsule: Add keys and certificates needed for capsule update testing
Hi Sughosh,
On Sat, 5 Aug 2023 at 12:50, Sughosh Ganu wrote:
>
> hi Simon,
>
> On Sun, 6 Aug 2023 at 00:06, Simon Glass wrote:
> >
> > Hi Sughosh,
> >
> > On Sat, 5 Aug 2023 at 11:50, Sughosh Ganu wrote:
> > >
> > > hi Simon,
> > >
> > > On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu
> > > > wrote:
> > > > >
> > > > > Add the private keys and public key certificates which are to be used
> > > > > for capsule authentication while testing the EFI capsule update
> > > > > functionality. There are two pairs of private and public keys. The
> > > > > SIGNER.{key,crt} pair will be used for signing capsules, whilst the
> > > > > SIGNER2.{key,crt} pair is to be used as malicious keys for testing
> > > > > authentication failure cases. The SIGNER.crt is also converted to an
> > > > > EFI Signature List(ESL) file, SIGNER.esl, which is embedded in the
> > > > > platform's device-tree for capsule authentication.
> > > > >
> > > > > Signed-off-by: Sughosh Ganu
> > > > > ---
> > > > > Changes since V6:
> > > > > * New patch that puts the keys and cert files under board/sandbox/
> > > > > directory as suggested Simon Glass.
> > > > >
> > > > > board/sandbox/SIGNER.crt | 19 +++
> > > > > board/sandbox/SIGNER.esl | Bin 0 -> 831 bytes
> > > > > board/sandbox/SIGNER.key | 28
> > > > > board/sandbox/SIGNER2.crt | 19 +++
> > > > > board/sandbox/SIGNER2.key | 28
> > > > > 5 files changed, 94 insertions(+)
> > > > > create mode 100644 board/sandbox/SIGNER.crt
> > > > > create mode 100644 board/sandbox/SIGNER.esl
> > > > > create mode 100644 board/sandbox/SIGNER.key
> > > > > create mode 100644 board/sandbox/SIGNER2.crt
> > > > > create mode 100644 board/sandbox/SIGNER2.key
> > > >
> > > > Can we call these good.* and bad.* so it is clear what they are for?
> > > > Also, please avoid capital letters in filenames.
> > >
> > > I was using the same nomenclature that was being used currently by the
> > > efi capsule update tests. But I guess I can change this.
> >
> > Yes please. You could use a patch at the start of your series, perhaps?
>
> Er, this is actually at the start of the series, isn't it. Well, at
> least before we start adding relevant stuff like the ESL file incbin
> logic in the u-boot.dtsi file -- this patch precedes patch 4 which is
> adding the incbin logic to u-boot.dtsi.
OK, well anyway if you can rename them to be more meaningful that would help.
Regards,
Simon
Re: [PATCH v7 04/11] capsule: authenticate: Add capsule public key in platform's dtb
Hi Sughosh,
On Sat, 5 Aug 2023 at 12:47, Sughosh Ganu wrote:
>
> hi Simon,
>
> On Sun, 6 Aug 2023 at 00:06, Simon Glass wrote:
> >
> > Hi Sughosh,
> >
> > On Sat, 5 Aug 2023 at 11:54, Sughosh Ganu wrote:
> > >
> > > hi Simon,
> > >
> > > On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu
> > > > wrote:
> > > > >
> > > > > The EFI capsule authentication logic in u-boot expects the public key
> > > > > in the form of an EFI Signature List(ESL) to be provided as part of
> > > > > the platform's dtb. Currently, the embedding of the ESL file into the
> > > > > dtb needs to be done manually.
> > > > >
> > > > > Add a signature node in the u-boot dtsi file and include the public
> > > > > key through the capsule-key property. This file is per architecture,
> > > > > and is currently being added for sandbox and arm architectures. It
> > > > > will have to be added for other architectures which need to enable
> > > > > capsule authentication support.
> > > > >
> > > > > The path to the ESL file is specified through the
> > > > > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> > > > >
> > > > > Signed-off-by: Sughosh Ganu
> > > > > ---
> > > > > Changes since V6:
> > > > > * Populate the CONFIG_EFI_CAPSULE_ESL_FILE symbol for sandbox and
> > > > > sandbox_flattree which enable capsule authentication.
> > > > >
> > > > > Note:
> > > > > Simon Glass had asked me to rid of the CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > > > ifdef used in the sandbox' u-boot.dtsi file. However, that results in
> > > > > the sandbox_vpl test failing in CI. Hence that check has been kept.
> > > > >
> > > > >
> > > > > arch/arm/dts/u-boot.dtsi | 14 ++
> > > > > arch/sandbox/dts/u-boot.dtsi | 17 +
> > > > > configs/sandbox_defconfig | 1 +
> > > > > configs/sandbox_flattree_defconfig | 1 +
> > > > > lib/efi_loader/Kconfig | 9 +
> > > > > 5 files changed, 42 insertions(+)
> > > > > create mode 100644 arch/arm/dts/u-boot.dtsi
> > > > > create mode 100644 arch/sandbox/dts/u-boot.dtsi
> > > > >
> > > > > diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> > > > > new file mode 100644
> > > > > index 00..4f31da4521
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/dts/u-boot.dtsi
> > > > > @@ -0,0 +1,14 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/**
> > > > > + * Devicetree file with miscellaneous nodes that will be included
> > > > > + * at build time into the DTB. Currently being used for including
> > > > > + * capsule related information.
> > > > > + */
> > > > > +
> > > > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > > > +/ {
> > > > > + signature {
> > > > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > > > + };
> > > > > +};
> > > > > +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> > > > > diff --git a/arch/sandbox/dts/u-boot.dtsi
> > > > > b/arch/sandbox/dts/u-boot.dtsi
> > > > > new file mode 100644
> > > > > index 00..60bd004937
> > > > > --- /dev/null
> > > > > +++ b/arch/sandbox/dts/u-boot.dtsi
> > > > > @@ -0,0 +1,17 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > +/*
> > > > > + * Devicetree file with miscellaneous nodes that will be included
> > > > > + * at build time into the DTB. Currently being used for including
> > > > > + * capsule related information.
> > > > > + *
> > > > > + */
> > > > > +
> > > > > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > > > +/ {
> > > > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > > > + signature {
> > > > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > > > + };
> > > > > +#endif
> > > > > +};
> > > > > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > > > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > > > > index b6c4f735f2..779af4abc8 100644
> > > > > --- a/configs/sandbox_defconfig
> > > > > +++ b/configs/sandbox_defconfig
> > > > > @@ -341,6 +341,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> > > > > CONFIG_EFI_CAPSULE_ON_DISK=y
> > > > > CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > > > > CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> > > > > +CONFIG_EFI_CAPSULE_ESL_FILE="../../../board/sandbox/SIGNER.esl"
> > > >
> > > > Can we avoid the path here, and just use e.g. good.esl ?
> > >
> > > Unfortunately no. I believe the way incbin works in dts is that it
> > > looks for the binary to be included in the same directory as the dts.
> > > Which is why I have to point it to the location of the esl relative to
> > > the dts.
> > >
> > > >
> > > > Perhaps this could be fixed up later, e.g. by adding the board
> > > > directory as an include dir when building the DT?
> > >
> > > Again, this is not how incbin seems to work in dts. I tried putting
> > > the esl in one of the existing include directory locations, but it
> > > does not pick the file from those dirs. It works with the
Re: [PATCH v7 08/11] binman: capsule: Add support for generating EFI capsules
Hi Sughosh, On Sat, 5 Aug 2023 at 12:42, Sughosh Ganu wrote: > > hi Simon, > > On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote: > > > > Hi Sughosh, > > > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote: > > > > > > Add support in binman for generating EFI capsules. The capsule > > > parameters can be specified through the capsule binman entry. Also add > > > test cases in binman for testing capsule generation. > > > > > > Signed-off-by: Sughosh Ganu > > > --- > > > Changes since V6: > > > * Add macros for the GUID strings in sandbox_efi_capsule.h > > > * Highlight that the private and public keys are mandatory for capsule > > > signing. > > > * Add a URL link to the UEFI spec, as used in the rst files. > > > * Use local vars for private and public keys in BuildSectionData() > > > * Use local vars for input payload and capsule filenames in > > > BuildSectionData(). > > > * Drop the ProcessContents() and SetImagePos() as the superclass > > > functions suffice. > > > * Use GUID macro names in the capsule test dts files. > > > * Rename efi_capsule_payload.bin to capsule_input.bin. > > > > > > > > > include/sandbox_efi_capsule.h | 14 ++ > > > > Please move this file to a later patch - see below. > > The idea was to also be able to run the binman capsule tests once this > patch was applied. If we are to move this to a separate patch, it > should be the one before this patch. But I guess based on your other > reply, this might not be needed after all. Yes, it should not be needed if we name the test GUIDs. Remember that binman is a standalone tool so cannot reference files outside tools/...although there is no test for that so some things may have crept in. > > > > > Could we have a single header file with all the GUIDs, i.e. sandbox, ARM, > > etc. > > Umm, I am not too sure. Maybe we can take a call at a later point if > there are too many files that start cropping up. OK > > > > > > tools/binman/entries.rst | 62 > > > tools/binman/etype/efi_capsule.py | 143 ++ > > > tools/binman/ftest.py | 121 +++ > > > tools/binman/test/307_capsule.dts | 23 +++ > > > tools/binman/test/308_capsule_signed.dts | 25 +++ > > > tools/binman/test/309_capsule_version.dts | 24 +++ > > > tools/binman/test/310_capsule_signed_ver.dts | 26 > > > tools/binman/test/311_capsule_oemflags.dts| 24 +++ > > > tools/binman/test/312_capsule_missing_key.dts | 24 +++ > > > .../binman/test/313_capsule_missing_index.dts | 22 +++ > > > .../binman/test/314_capsule_missing_guid.dts | 19 +++ > > > .../test/315_capsule_missing_payload.dts | 19 +++ > > > 13 files changed, 546 insertions(+) > > > create mode 100644 include/sandbox_efi_capsule.h > > > create mode 100644 tools/binman/etype/efi_capsule.py > > > create mode 100644 tools/binman/test/307_capsule.dts > > > create mode 100644 tools/binman/test/308_capsule_signed.dts > > > create mode 100644 tools/binman/test/309_capsule_version.dts > > > create mode 100644 tools/binman/test/310_capsule_signed_ver.dts > > > create mode 100644 tools/binman/test/311_capsule_oemflags.dts > > > create mode 100644 tools/binman/test/312_capsule_missing_key.dts > > > create mode 100644 tools/binman/test/313_capsule_missing_index.dts > > > create mode 100644 tools/binman/test/314_capsule_missing_guid.dts > > > create mode 100644 tools/binman/test/315_capsule_missing_payload.dts > > > [..] > > > + > > > +def ReadNode(self): > > > +self.ReadEntries() > > > +super().ReadNode() > > > > I believe those two lines should be swapped. > > Again, like my earlier code for ProcessContents() and SetImagePos(), > which was taken from mkimage.py as reference, this code is on similar > lines to what is in intel_ifwi.py. Both these files are authored by > you, so I took this as reference, especially mkimage.py. OK, then take a look at mkimage.py and follow that. Yes intel_ifwi is around the wrong way. Although these days ReadEntries() is called automatically from entry_Section so you don't need to call it here. > > > > > > + > > > +self.image_index = fdt_util.GetInt(self._node, 'image-index') > > > +self.image_guid = fdt_util.GetString(self._node, 'image-type-id') > > > +self.fw_version = fdt_util.GetInt(self._node, 'fw-version') > > > +self.hardware_instance = fdt_util.GetInt(self._node, > > > 'hardware-instance') > > > +self.monotonic_count = fdt_util.GetInt(self._node, > > > 'monotonic-count') > > > +self.oem_flags = fdt_util.GetInt(self._node, 'oem-flags') > > > + > > > +self.private_key = fdt_util.GetString(self._node, 'private-key') > > > +self.public_key_cert = fdt_util.GetString(self._node, > > > 'public-key-cert') > > > +if ((self.private_key and not self.public_key_cert) or > > > (self.public_key_cert and not self.private_key)): > > > +
Re: [PATCH v7 03/11] sandbox: capsule: Add keys and certificates needed for capsule update testing
hi Simon,
On Sun, 6 Aug 2023 at 00:06, Simon Glass wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 11:50, Sughosh Ganu wrote:
> >
> > hi Simon,
> >
> > On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> > > >
> > > > Add the private keys and public key certificates which are to be used
> > > > for capsule authentication while testing the EFI capsule update
> > > > functionality. There are two pairs of private and public keys. The
> > > > SIGNER.{key,crt} pair will be used for signing capsules, whilst the
> > > > SIGNER2.{key,crt} pair is to be used as malicious keys for testing
> > > > authentication failure cases. The SIGNER.crt is also converted to an
> > > > EFI Signature List(ESL) file, SIGNER.esl, which is embedded in the
> > > > platform's device-tree for capsule authentication.
> > > >
> > > > Signed-off-by: Sughosh Ganu
> > > > ---
> > > > Changes since V6:
> > > > * New patch that puts the keys and cert files under board/sandbox/
> > > > directory as suggested Simon Glass.
> > > >
> > > > board/sandbox/SIGNER.crt | 19 +++
> > > > board/sandbox/SIGNER.esl | Bin 0 -> 831 bytes
> > > > board/sandbox/SIGNER.key | 28
> > > > board/sandbox/SIGNER2.crt | 19 +++
> > > > board/sandbox/SIGNER2.key | 28
> > > > 5 files changed, 94 insertions(+)
> > > > create mode 100644 board/sandbox/SIGNER.crt
> > > > create mode 100644 board/sandbox/SIGNER.esl
> > > > create mode 100644 board/sandbox/SIGNER.key
> > > > create mode 100644 board/sandbox/SIGNER2.crt
> > > > create mode 100644 board/sandbox/SIGNER2.key
> > >
> > > Can we call these good.* and bad.* so it is clear what they are for?
> > > Also, please avoid capital letters in filenames.
> >
> > I was using the same nomenclature that was being used currently by the
> > efi capsule update tests. But I guess I can change this.
>
> Yes please. You could use a patch at the start of your series, perhaps?
Er, this is actually at the start of the series, isn't it. Well, at
least before we start adding relevant stuff like the ESL file incbin
logic in the u-boot.dtsi file -- this patch precedes patch 4 which is
adding the incbin logic to u-boot.dtsi.
-sughosh
Re: [PATCH v7 04/11] capsule: authenticate: Add capsule public key in platform's dtb
hi Simon,
On Sun, 6 Aug 2023 at 00:06, Simon Glass wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 11:54, Sughosh Ganu wrote:
> >
> > hi Simon,
> >
> > On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> > > >
> > > > The EFI capsule authentication logic in u-boot expects the public key
> > > > in the form of an EFI Signature List(ESL) to be provided as part of
> > > > the platform's dtb. Currently, the embedding of the ESL file into the
> > > > dtb needs to be done manually.
> > > >
> > > > Add a signature node in the u-boot dtsi file and include the public
> > > > key through the capsule-key property. This file is per architecture,
> > > > and is currently being added for sandbox and arm architectures. It
> > > > will have to be added for other architectures which need to enable
> > > > capsule authentication support.
> > > >
> > > > The path to the ESL file is specified through the
> > > > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> > > >
> > > > Signed-off-by: Sughosh Ganu
> > > > ---
> > > > Changes since V6:
> > > > * Populate the CONFIG_EFI_CAPSULE_ESL_FILE symbol for sandbox and
> > > > sandbox_flattree which enable capsule authentication.
> > > >
> > > > Note:
> > > > Simon Glass had asked me to rid of the CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > > ifdef used in the sandbox' u-boot.dtsi file. However, that results in
> > > > the sandbox_vpl test failing in CI. Hence that check has been kept.
> > > >
> > > >
> > > > arch/arm/dts/u-boot.dtsi | 14 ++
> > > > arch/sandbox/dts/u-boot.dtsi | 17 +
> > > > configs/sandbox_defconfig | 1 +
> > > > configs/sandbox_flattree_defconfig | 1 +
> > > > lib/efi_loader/Kconfig | 9 +
> > > > 5 files changed, 42 insertions(+)
> > > > create mode 100644 arch/arm/dts/u-boot.dtsi
> > > > create mode 100644 arch/sandbox/dts/u-boot.dtsi
> > > >
> > > > diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> > > > new file mode 100644
> > > > index 00..4f31da4521
> > > > --- /dev/null
> > > > +++ b/arch/arm/dts/u-boot.dtsi
> > > > @@ -0,0 +1,14 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/**
> > > > + * Devicetree file with miscellaneous nodes that will be included
> > > > + * at build time into the DTB. Currently being used for including
> > > > + * capsule related information.
> > > > + */
> > > > +
> > > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > > +/ {
> > > > + signature {
> > > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > > + };
> > > > +};
> > > > +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > > > new file mode 100644
> > > > index 00..60bd004937
> > > > --- /dev/null
> > > > +++ b/arch/sandbox/dts/u-boot.dtsi
> > > > @@ -0,0 +1,17 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * Devicetree file with miscellaneous nodes that will be included
> > > > + * at build time into the DTB. Currently being used for including
> > > > + * capsule related information.
> > > > + *
> > > > + */
> > > > +
> > > > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > > +/ {
> > > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > > + signature {
> > > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > > + };
> > > > +#endif
> > > > +};
> > > > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > > > index b6c4f735f2..779af4abc8 100644
> > > > --- a/configs/sandbox_defconfig
> > > > +++ b/configs/sandbox_defconfig
> > > > @@ -341,6 +341,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> > > > CONFIG_EFI_CAPSULE_ON_DISK=y
> > > > CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > > > CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> > > > +CONFIG_EFI_CAPSULE_ESL_FILE="../../../board/sandbox/SIGNER.esl"
> > >
> > > Can we avoid the path here, and just use e.g. good.esl ?
> >
> > Unfortunately no. I believe the way incbin works in dts is that it
> > looks for the binary to be included in the same directory as the dts.
> > Which is why I have to point it to the location of the esl relative to
> > the dts.
> >
> > >
> > > Perhaps this could be fixed up later, e.g. by adding the board
> > > directory as an include dir when building the DT?
> >
> > Again, this is not how incbin seems to work in dts. I tried putting
> > the esl in one of the existing include directory locations, but it
> > does not pick the file from those dirs. It works with the assumption
> > that the bin file is to be in the same dir as the dts.
>
> Yes but you can change that. Try adding to the cmd_dtc rule in Makefile.lib:
>
>-i $(srctree)/board/$(BOARDDIR) \
We already have the
-I$(srctree)/include
and I had tried putting the esl under the include directory, but it
was not found.
Re: [PATCH v7 08/11] binman: capsule: Add support for generating EFI capsules
hi Simon,
On Sat, 5 Aug 2023 at 22:50, Simon Glass wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 09:03, Simon Glass wrote:
> >
> > Hi Sughosh,
> >
> > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> > >
> > > Add support in binman for generating EFI capsules. The capsule
> > > parameters can be specified through the capsule binman entry. Also add
> > > test cases in binman for testing capsule generation.
> > >
> > > Signed-off-by: Sughosh Ganu
> > > ---
> > > Changes since V6:
> > > * Add macros for the GUID strings in sandbox_efi_capsule.h
> > > * Highlight that the private and public keys are mandatory for capsule
> > > signing.
> > > * Add a URL link to the UEFI spec, as used in the rst files.
> > > * Use local vars for private and public keys in BuildSectionData()
> > > * Use local vars for input payload and capsule filenames in
> > > BuildSectionData().
> > > * Drop the ProcessContents() and SetImagePos() as the superclass
> > > functions suffice.
> > > * Use GUID macro names in the capsule test dts files.
> > > * Rename efi_capsule_payload.bin to capsule_input.bin.
> > >
> > >
> > > include/sandbox_efi_capsule.h | 14 ++
> >
> > Please move this file to a later patch - see below.
> >
> > Could we have a single header file with all the GUIDs, i.e. sandbox, ARM,
> > etc.
> >
> > > tools/binman/entries.rst | 62
> > > tools/binman/etype/efi_capsule.py | 143 ++
> > > tools/binman/ftest.py | 121 +++
> > > tools/binman/test/307_capsule.dts | 23 +++
> > > tools/binman/test/308_capsule_signed.dts | 25 +++
> > > tools/binman/test/309_capsule_version.dts | 24 +++
> > > tools/binman/test/310_capsule_signed_ver.dts | 26
> > > tools/binman/test/311_capsule_oemflags.dts| 24 +++
> > > tools/binman/test/312_capsule_missing_key.dts | 24 +++
> > > .../binman/test/313_capsule_missing_index.dts | 22 +++
> > > .../binman/test/314_capsule_missing_guid.dts | 19 +++
> > > .../test/315_capsule_missing_payload.dts | 19 +++
> > > 13 files changed, 546 insertions(+)
> > > create mode 100644 include/sandbox_efi_capsule.h
> > > create mode 100644 tools/binman/etype/efi_capsule.py
> > > create mode 100644 tools/binman/test/307_capsule.dts
> > > create mode 100644 tools/binman/test/308_capsule_signed.dts
> > > create mode 100644 tools/binman/test/309_capsule_version.dts
> > > create mode 100644 tools/binman/test/310_capsule_signed_ver.dts
> > > create mode 100644 tools/binman/test/311_capsule_oemflags.dts
> > > create mode 100644 tools/binman/test/312_capsule_missing_key.dts
> > > create mode 100644 tools/binman/test/313_capsule_missing_index.dts
> > > create mode 100644 tools/binman/test/314_capsule_missing_guid.dts
> > > create mode 100644 tools/binman/test/315_capsule_missing_payload.dts
> > >
>
> [..]
>
> > > diff --git a/tools/binman/test/307_capsule.dts
> > > b/tools/binman/test/307_capsule.dts
> > > new file mode 100644
> > > index 00..86552ff83f
> > > --- /dev/null
> > > +++ b/tools/binman/test/307_capsule.dts
> > > @@ -0,0 +1,23 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +
> > > +/dts-v1/;
> > > +
> > > +#include
> >
> > We can't include sandbox files in binman! I Does it actually matter
> > what the GUID value is? Can they all be the same? For now I think it
> > is best to have
> >
> > #define BINMAN_TEST_GUID "xxx"
> >
> > repeated at the top of each .dts file. Hopefully at some point we can
> > figure out a sensible way to provide a decoder ring for this mess, so
> > we can use actually names in the binman definition instead of GUIDs.
>
> Oh, having thought about this a bit more, there is a much better way.
>
> In the entry type, allow the image_guid to be a string, like
> 'binman-test' or 'sandbox-test'. Then binman can have a conversion
> function to figure out the GUID to pass to the tool, e.g. in
> efi_capsule.py:
>
> TYPE_TO_GUID = {
>'binman-test': '09123987987f98712',
>'sandbox-test':, '98123987123123987123987',
> }
>
> def get_guid(type_str):
> 'Get the GUID to use for a particular purpose"""
> return TYPE_TO_GUID.get(type_str, type_str)
>
> Then you can use these same strings in the sandbox tests, without
> needing to include anything.
Okay, will try this out. Thanks for the suggestion.
-sughosh
>
> >
> > > +
> > > +/ {
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > +
> > > + binman {
> > > + efi-capsule {
> > > + image-index = <0x1>;
> > > + /* Image GUID for testing capsule update */
> > > + image-type-id = SANDBOX_UBOOT_IMAGE_GUID;
> > > + hardware-instance = <0x0>;
> > > +
> > > + blob {
> > > + filename = "capsule_input.bin";
> > > + };
> > > + };
> >
Re: [PATCH v7 08/11] binman: capsule: Add support for generating EFI capsules
hi Simon, On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote: > > Hi Sughosh, > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote: > > > > Add support in binman for generating EFI capsules. The capsule > > parameters can be specified through the capsule binman entry. Also add > > test cases in binman for testing capsule generation. > > > > Signed-off-by: Sughosh Ganu > > --- > > Changes since V6: > > * Add macros for the GUID strings in sandbox_efi_capsule.h > > * Highlight that the private and public keys are mandatory for capsule > > signing. > > * Add a URL link to the UEFI spec, as used in the rst files. > > * Use local vars for private and public keys in BuildSectionData() > > * Use local vars for input payload and capsule filenames in > > BuildSectionData(). > > * Drop the ProcessContents() and SetImagePos() as the superclass > > functions suffice. > > * Use GUID macro names in the capsule test dts files. > > * Rename efi_capsule_payload.bin to capsule_input.bin. > > > > > > include/sandbox_efi_capsule.h | 14 ++ > > Please move this file to a later patch - see below. The idea was to also be able to run the binman capsule tests once this patch was applied. If we are to move this to a separate patch, it should be the one before this patch. But I guess based on your other reply, this might not be needed after all. > > Could we have a single header file with all the GUIDs, i.e. sandbox, ARM, etc. Umm, I am not too sure. Maybe we can take a call at a later point if there are too many files that start cropping up. > > > tools/binman/entries.rst | 62 > > tools/binman/etype/efi_capsule.py | 143 ++ > > tools/binman/ftest.py | 121 +++ > > tools/binman/test/307_capsule.dts | 23 +++ > > tools/binman/test/308_capsule_signed.dts | 25 +++ > > tools/binman/test/309_capsule_version.dts | 24 +++ > > tools/binman/test/310_capsule_signed_ver.dts | 26 > > tools/binman/test/311_capsule_oemflags.dts| 24 +++ > > tools/binman/test/312_capsule_missing_key.dts | 24 +++ > > .../binman/test/313_capsule_missing_index.dts | 22 +++ > > .../binman/test/314_capsule_missing_guid.dts | 19 +++ > > .../test/315_capsule_missing_payload.dts | 19 +++ > > 13 files changed, 546 insertions(+) > > create mode 100644 include/sandbox_efi_capsule.h > > create mode 100644 tools/binman/etype/efi_capsule.py > > create mode 100644 tools/binman/test/307_capsule.dts > > create mode 100644 tools/binman/test/308_capsule_signed.dts > > create mode 100644 tools/binman/test/309_capsule_version.dts > > create mode 100644 tools/binman/test/310_capsule_signed_ver.dts > > create mode 100644 tools/binman/test/311_capsule_oemflags.dts > > create mode 100644 tools/binman/test/312_capsule_missing_key.dts > > create mode 100644 tools/binman/test/313_capsule_missing_index.dts > > create mode 100644 tools/binman/test/314_capsule_missing_guid.dts > > create mode 100644 tools/binman/test/315_capsule_missing_payload.dts > > > > diff --git a/include/sandbox_efi_capsule.h b/include/sandbox_efi_capsule.h > > new file mode 100644 > > index 00..da71b87a18 > > --- /dev/null > > +++ b/include/sandbox_efi_capsule.h > > @@ -0,0 +1,14 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (c) 2023, Linaro Limited > > + */ > > + > > +#if !defined(_SANDBOX_EFI_CAPSULE_H_) > > +#define _SANDBOX_EFI_CAPSULE_H_ > > + > > +#define SANDBOX_UBOOT_IMAGE_GUID > > "09d7cf52-0720-4710-91d1-08469b7fe9c8" > > +#define SANDBOX_UBOOT_ENV_IMAGE_GUID > > "5a7021f5-fef2-48b4-aaba-832e777418c0" > > +#define SANDBOX_FIT_IMAGE_GUID > > "3673b45d-6a7c-46f3-9e60-adabb03f7937" > > +#define SANDBOX_INCORRECT_GUID > > "058b7d83-50d5-4c47-a195-60d86ad341c4" > > These should not be needed in binman tests. > > + > > +#endif /* _SANDBOX_EFI_CAPSULE_H_ */ > > diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst > > index f2376932be..fc094b9c08 100644 > > --- a/tools/binman/entries.rst > > +++ b/tools/binman/entries.rst > > @@ -468,6 +468,68 @@ updating the EC on startup via software sync. > > > > > > > > +.. _etype_efi_capsule: > > + > > +Entry: capsule: Entry for generating EFI Capsule files > > +-- > > + > > +The parameters needed for generation of the capsules can > > +be provided as properties in the entry. > > + > > +Properties / Entry arguments: > > +- image-index: Unique number for identifying corresponding > > + payload image. Number between 1 and descriptor count, i.e. > > + the total number of firmware images that can be updated. > > +- image-type-id: Image GUID which will be used for identifying the > > + updatable image on the board. > > +- hardware-instance: Optional number for identifying unique > > + hardware instance of a device in the system. Default value of 0
Re: [PATCH v7 03/11] sandbox: capsule: Add keys and certificates needed for capsule update testing
Hi Sughosh,
On Sat, 5 Aug 2023 at 11:50, Sughosh Ganu wrote:
>
> hi Simon,
>
> On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
> >
> > Hi Sughosh,
> >
> > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> > >
> > > Add the private keys and public key certificates which are to be used
> > > for capsule authentication while testing the EFI capsule update
> > > functionality. There are two pairs of private and public keys. The
> > > SIGNER.{key,crt} pair will be used for signing capsules, whilst the
> > > SIGNER2.{key,crt} pair is to be used as malicious keys for testing
> > > authentication failure cases. The SIGNER.crt is also converted to an
> > > EFI Signature List(ESL) file, SIGNER.esl, which is embedded in the
> > > platform's device-tree for capsule authentication.
> > >
> > > Signed-off-by: Sughosh Ganu
> > > ---
> > > Changes since V6:
> > > * New patch that puts the keys and cert files under board/sandbox/
> > > directory as suggested Simon Glass.
> > >
> > > board/sandbox/SIGNER.crt | 19 +++
> > > board/sandbox/SIGNER.esl | Bin 0 -> 831 bytes
> > > board/sandbox/SIGNER.key | 28
> > > board/sandbox/SIGNER2.crt | 19 +++
> > > board/sandbox/SIGNER2.key | 28
> > > 5 files changed, 94 insertions(+)
> > > create mode 100644 board/sandbox/SIGNER.crt
> > > create mode 100644 board/sandbox/SIGNER.esl
> > > create mode 100644 board/sandbox/SIGNER.key
> > > create mode 100644 board/sandbox/SIGNER2.crt
> > > create mode 100644 board/sandbox/SIGNER2.key
> >
> > Can we call these good.* and bad.* so it is clear what they are for?
> > Also, please avoid capital letters in filenames.
>
> I was using the same nomenclature that was being used currently by the
> efi capsule update tests. But I guess I can change this.
Yes please. You could use a patch at the start of your series, perhaps?
Regards,
Simon
Re: [PATCH v7 04/11] capsule: authenticate: Add capsule public key in platform's dtb
Hi Sughosh,
On Sat, 5 Aug 2023 at 11:54, Sughosh Ganu wrote:
>
> hi Simon,
>
> On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
> >
> > Hi Sughosh,
> >
> > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> > >
> > > The EFI capsule authentication logic in u-boot expects the public key
> > > in the form of an EFI Signature List(ESL) to be provided as part of
> > > the platform's dtb. Currently, the embedding of the ESL file into the
> > > dtb needs to be done manually.
> > >
> > > Add a signature node in the u-boot dtsi file and include the public
> > > key through the capsule-key property. This file is per architecture,
> > > and is currently being added for sandbox and arm architectures. It
> > > will have to be added for other architectures which need to enable
> > > capsule authentication support.
> > >
> > > The path to the ESL file is specified through the
> > > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> > >
> > > Signed-off-by: Sughosh Ganu
> > > ---
> > > Changes since V6:
> > > * Populate the CONFIG_EFI_CAPSULE_ESL_FILE symbol for sandbox and
> > > sandbox_flattree which enable capsule authentication.
> > >
> > > Note:
> > > Simon Glass had asked me to rid of the CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > ifdef used in the sandbox' u-boot.dtsi file. However, that results in
> > > the sandbox_vpl test failing in CI. Hence that check has been kept.
> > >
> > >
> > > arch/arm/dts/u-boot.dtsi | 14 ++
> > > arch/sandbox/dts/u-boot.dtsi | 17 +
> > > configs/sandbox_defconfig | 1 +
> > > configs/sandbox_flattree_defconfig | 1 +
> > > lib/efi_loader/Kconfig | 9 +
> > > 5 files changed, 42 insertions(+)
> > > create mode 100644 arch/arm/dts/u-boot.dtsi
> > > create mode 100644 arch/sandbox/dts/u-boot.dtsi
> > >
> > > diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> > > new file mode 100644
> > > index 00..4f31da4521
> > > --- /dev/null
> > > +++ b/arch/arm/dts/u-boot.dtsi
> > > @@ -0,0 +1,14 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/**
> > > + * Devicetree file with miscellaneous nodes that will be included
> > > + * at build time into the DTB. Currently being used for including
> > > + * capsule related information.
> > > + */
> > > +
> > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > +/ {
> > > + signature {
> > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > + };
> > > +};
> > > +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > > new file mode 100644
> > > index 00..60bd004937
> > > --- /dev/null
> > > +++ b/arch/sandbox/dts/u-boot.dtsi
> > > @@ -0,0 +1,17 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Devicetree file with miscellaneous nodes that will be included
> > > + * at build time into the DTB. Currently being used for including
> > > + * capsule related information.
> > > + *
> > > + */
> > > +
> > > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > +/ {
> > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > + signature {
> > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > + };
> > > +#endif
> > > +};
> > > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > > index b6c4f735f2..779af4abc8 100644
> > > --- a/configs/sandbox_defconfig
> > > +++ b/configs/sandbox_defconfig
> > > @@ -341,6 +341,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> > > CONFIG_EFI_CAPSULE_ON_DISK=y
> > > CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > > CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> > > +CONFIG_EFI_CAPSULE_ESL_FILE="../../../board/sandbox/SIGNER.esl"
> >
> > Can we avoid the path here, and just use e.g. good.esl ?
>
> Unfortunately no. I believe the way incbin works in dts is that it
> looks for the binary to be included in the same directory as the dts.
> Which is why I have to point it to the location of the esl relative to
> the dts.
>
> >
> > Perhaps this could be fixed up later, e.g. by adding the board
> > directory as an include dir when building the DT?
>
> Again, this is not how incbin seems to work in dts. I tried putting
> the esl in one of the existing include directory locations, but it
> does not pick the file from those dirs. It works with the assumption
> that the bin file is to be in the same dir as the dts.
Yes but you can change that. Try adding to the cmd_dtc rule in Makefile.lib:
-i $(srctree)/board/$(BOARDDIR) \
[..]
Regards,
Simon
Re: [PATCH] Azure: Squash a number of jobs and re-order slightly
On Sat, 5 Aug 2023 at 10:24, Tom Rini wrote: > > To reduce overall job time, move a number of smaller jobs together. > These should still be safely under 1 hour total time, but reducing the > overall number of jobs should help with the queue slightly. > > Signed-off-by: Tom Rini > --- > .azure-pipelines.yml | 72 ++-- > 1 file changed, 23 insertions(+), 49 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v7 09/11] sandbox: capsule: Generate capsule related files through binman
Hi Sughosh, On Sat, 5 Aug 2023 at 12:01, Sughosh Ganu wrote: > > hi Simon, > > On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote: > > > > Hi Sughosh, > > > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote: > > > > > > The EFI capsule files can now be generated as part of u-boot > > > build through binman. Add capsule entry nodes in the u-boot.dtsi for > > > the sandbox architecture for generating the capsules. These capsules > > > are then used for testing the EFI capsule update functionality on the > > > sandbox platforms. Also add binman nodes for generating the input > > > files used for these capsules. > > > > > > Remove the corresponding logic which was used for generation of these > > > input files which is now superfluous. > > > > > > Signed-off-by: Sughosh Ganu > > > --- > > > Changes since V6: > > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule > > > input filenames. > > > * Generate the capsule input files through binman text entries. > > > > > > arch/sandbox/dts/u-boot.dtsi | 363 ++ > > > include/sandbox_efi_capsule.h | 11 + > > > test/py/tests/test_efi_capsule/conftest.py| 168 +--- > > > test/py/tests/test_efi_capsule/signature.dts | 10 - > > > .../tests/test_efi_capsule/uboot_bin_env.its | 36 -- > > > 5 files changed, 385 insertions(+), 203 deletions(-) > > > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts > > > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its > > > > I think you are still getting confused with using filenames when you > > don't need to. See below... > > No, I don't think so. This is being done on purpose, since I want > these files to be created. These are then copied to the efi capsule > update tests' data directory, and are then used in testing the > feature. Maybe if you want, I can put a comment to that effect. I just traced through this code. I really think this needs to be simplified. You can do it in a patch at the end if you like. But you have the 'u-boot.bin.old' and 'Old' strings in test_efi_capsule_auth2, for example. In the binman world you define UBOOT_BIN_IMAGE_OLD as "u-boot.bin.old", then use that in the sandbox.dtsi Then binman creates the u-boot.bin.old file (unnecessarily in my view) Then in efi_capsule_data() you copy the file to the test directory. So for the last step, you could just create the file again, rather than copying it from the U-Boot build directory. After all, you know the contents. If you like you could put the different contents as binary strings in capsule_defs.py Then you don't need to create the files. There is a lot more I can say about the EFI capsule tests. For now I think we'll have to live with it creating 19 different binman images on sandbox. I think we could put them in an efi_capsules subdir, but that would need to be created somewhere, since binman doesn't do it. I suppose we could make binman automatically create a directory if an entry filename needs it? Anyway, for tests, primarily we need to split things up, so we have, for example: process_capsule_file() which processes the capsule in U-Boot, e.g. using an 'efi' command, then outputs what it did. Then the test can plant the capsule, run that function and check that the output is as expected. This can actually be a unit test, i.e. written in C. Most of the tests can do this. Then we can have one test that checks the whole flow, but it doesn't need to be done for every case, as now. Regards, Simon
Re: [PATCH v7 10/11] doc: Add documentation to highlight capsule generation related updates
hi Simon, On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote: > > Hi Sughosh, > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote: > > > > The EFI capsules can now be generated as part of u-boot build, through > > Please check your patches for 'U-Boot' throughout. Okay > > > binman. Highlight these changes in the documentation. > > > > Signed-off-by: Sughosh Ganu > > --- > > Changes since V6: None > > > > doc/develop/uefi/uefi.rst | 18 ++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst > > index b2854b52a6..3150d6fb9c 100644 > > --- a/doc/develop/uefi/uefi.rst > > +++ b/doc/develop/uefi/uefi.rst > > @@ -318,6 +318,9 @@ Run the following command > >--guid \ > > > > > > +Capsule with firmware version > > +* > > + > > The UEFI specification does not define the firmware versioning mechanism. > > EDK II reference implementation inserts the FMP Payload Header right before > > the payload. It coutains the fw_version and lowest supported version, > > @@ -345,6 +348,21 @@ add --fw-version option in mkeficapsule tool. > > If the --fw-version option is not set, FMP Payload Header is not inserted > > and fw_version is set as 0. > > > > + > > +Capsule Generation through binman > > +* > > + > > +Support has also been added to generate capsules during u-boot build > > +through binman. This requires the platform's DTB to be populated with > > +the capsule entry nodes for binman. The capsules then can be generated > > +by specifying the capsule parameters either through a config file, or > > not a config file at present Yes, will remove the reference. -sughosh > > > +by specifying them as properties in the capsule entry node. > > + > > +Check the arch/sandbox/dts/u-boot.dtsi file for the sandbox platform > > +as reference for how to generate capsules through binman as part of > > +u-boot build. > > + > > + > > Performing the update > > * > > > > -- > > 2.34.1 > > > > Regards, > Simon
Re: [PATCH v7 07/11] btool: mkeficapsule: Add a bintool for EFI capsule generation
hi Simon,
On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> >
> > Add a bintool for generating EFI capsules. This calls the mkeficapsule
> > tool which generates the capsules.
> >
> > Signed-off-by: Sughosh Ganu
> > ---
> > Changes since V6:
> > * Split the changes for mkeficapsule btool into a separate patch, as
> > suggested by Simon Glass.
> > * Use the word commandline consistently, as suggested by Simon Glass.
> >
> > tools/binman/btool/mkeficapsule.py | 101 +
> > 1 file changed, 101 insertions(+)
> > create mode 100644 tools/binman/btool/mkeficapsule.py
> >
>
> Reviewed-by: Simon Glass
>
> > diff --git a/tools/binman/btool/mkeficapsule.py
> > b/tools/binman/btool/mkeficapsule.py
> > new file mode 100644
> > index 00..61179747ff
> > --- /dev/null
> > +++ b/tools/binman/btool/mkeficapsule.py
> > @@ -0,0 +1,101 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +# Copyright 2023 Linaro Limited
> > +#
> > +"""Bintool implementation for mkeficapsule tool
> > +
> > +mkeficapsule is a tool used for generating EFI capsules.
> > +
> > +The following are the commandline options to be provided
> > +to the tool
> > +Usage: mkeficapsule [options]
> > +Options:
> > + -g, --guid guid for image blob type
> > + -i, --index update image index
> > + -I, --instanceupdate hardware instance
> > + -v, --fw-version firmware version
> > + -p, --private-key private key file
> > + -c, --certificate signer's certificate file
> > + -m, --monotonic-count monotonic count
> > + -d, --dump_sig dump signature (*.p7)
> > + -A, --fw-accept firmware accept capsule, requires GUID, no image
> > blob
> > + -R, --fw-revert firmware revert capsule, takes no GUID, no image
> > blob
> > + -o, --capoemflag Capsule OEM Flag, an integer between 0x and
> > 0x
> > + -h, --help print a help message
> > +"""
> > +
> > +from binman import bintool
> > +
> > +class Bintoolmkeficapsule(bintool.Bintool):
> > +"""Handles the 'mkeficapsule' tool
> > +
> > +This bintool is used for generating the EFI capsules. The
> > +capsule generation parameters can either be specified through
> > +commandline, or through a config file.
> > +"""
> > +def __init__(self, name):
> > +super().__init__(name, 'mkeficapsule tool for generating capsules')
> > +
> > +def generate_capsule(self, image_index, image_guid, hardware_instance,
> > + payload, output_fname, priv_key, pub_key,
> > + monotonic_count=0, version=0, oemflags=0):
> > +"""Generate a capsule through commandline-provided parameters
> > +
> > +Args:
> > +image_index (int): Unique number for identifying payload image
> > +image_guid (str): GUID used for identifying the image
>
> I wonder what we can do about this, so that we don't have to speak in
> GUIDs? Is there a registry somewhere of what all these things are? It
> would be nice if you could provide a string like 'u-boot-sandbox' and
> the capsule tool would know what that means.
In the EFI world GUIDs are just identifiers used for uniquely
identifying resources, in this case, images on a given board. There is
no registry of these values, however there are certain resources which
do have truly unique value, e.g. EFI protocols, or standard disk
partitions -- these values are then defined in a specification, like
UEFI.
In this context though, these GUIDs simply serve in identifying an
image for a given board. Typically, the platform code would then
"declare" the images that are supported on that platform using their
associated GUID values. And these GUIDs which are part of the capsule
are then used for checking if the input image on the capsule is indeed
intended for that platform at the time of performing the update.
>
> > +hardware_instance (int): Optional unique hardware instance of
> > +a device in the system. 0 if not being used
> > +payload (str): Path to the input payload image
> > +output_fname (str): Path to the output capsule file
> > +priv_key (str): Path to the private key
> > +pub_key(str): Path to the public key
> > +monotonic_count (int): Count used when signing an image
> > +version (int): Image version (Optional)
> > +oemflags (int): Optional 16 bit OEM flags
> > +
> > +Returns:
> > +str: Tool output
> > +"""
> > +args = [
> > +f'--index={image_index}',
> > +f'--guid={image_guid}',
> > +f'--instance={hardware_instance}'
> > +]
> > +
> > +if version:
> > +args += [f'--fw-version={version}']
> > +if oemflags:
> > +args +=
Re: [PATCH v7 09/11] sandbox: capsule: Generate capsule related files through binman
hi Simon,
On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> >
> > The EFI capsule files can now be generated as part of u-boot
> > build through binman. Add capsule entry nodes in the u-boot.dtsi for
> > the sandbox architecture for generating the capsules. These capsules
> > are then used for testing the EFI capsule update functionality on the
> > sandbox platforms. Also add binman nodes for generating the input
> > files used for these capsules.
> >
> > Remove the corresponding logic which was used for generation of these
> > input files which is now superfluous.
> >
> > Signed-off-by: Sughosh Ganu
> > ---
> > Changes since V6:
> > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule
> > input filenames.
> > * Generate the capsule input files through binman text entries.
> >
> > arch/sandbox/dts/u-boot.dtsi | 363 ++
> > include/sandbox_efi_capsule.h | 11 +
> > test/py/tests/test_efi_capsule/conftest.py| 168 +---
> > test/py/tests/test_efi_capsule/signature.dts | 10 -
> > .../tests/test_efi_capsule/uboot_bin_env.its | 36 --
> > 5 files changed, 385 insertions(+), 203 deletions(-)
> > delete mode 100644 test/py/tests/test_efi_capsule/signature.dts
> > delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
>
> I think you are still getting confused with using filenames when you
> don't need to. See below...
No, I don't think so. This is being done on purpose, since I want
these files to be created. These are then copied to the efi capsule
update tests' data directory, and are then used in testing the
feature. Maybe if you want, I can put a comment to that effect.
>
>
> >
> > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > index 60bd004937..f1b16b41c2 100644
> > --- a/arch/sandbox/dts/u-boot.dtsi
> > +++ b/arch/sandbox/dts/u-boot.dtsi
> > @@ -7,11 +7,374 @@
> > */
> >
> > #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > +
> > +#include
> > +
> > / {
> > #ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > signature {
> > capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > };
> > #endif
> > +
> > + binman: binman {
> > + multiple-images;
> > + };
> > +};
> > +
> > + {
> > + input1 {
> > + filename = UBOOT_BIN_IMAGE_OLD;
> > + text {
> > + text = "u-boot:Old";
> > + };
> > + };
> > +
> > + input2 {
> > + filename = UBOOT_BIN_IMAGE_NEW;
> > + text {
> > + text = "u-boot:New";
> > + };
> > + };
> > +
> > + input3 {
> > + filename = UBOOT_ENV_IMAGE_OLD;
> > + text {
> > + text = "u-boot-env:Old";
> > + };
> > + };
> > +
> > + input4 {
> > + filename = UBOOT_ENV_IMAGE_NEW;
> > + text {
> > + text = "u-boot-env:New";
> > + };
> > + };
>
> All of those nodes can be removed
>
> > +
> > + itb {
> > + filename = UBOOT_FIT_IMAGE;
> > +
> > + fit {
> > + description = "Automatic U-Boot environment update";
> > + #address-cells = <2>;
> > +
> > + images {
> > + u-boot-bin {
> > + description = "U-Boot binary on SPI
> > Flash";
> > + compression = "none";
> > + type = "firmware";
> > + arch = "sandbox";
> > + load = <0>;
> > + blob {
> > + filename =
> > UBOOT_BIN_IMAGE_NEW;
> > + };
>
> instead of this blob node, you should be able to write:
>
> text {
> text = "u-boot:Old";
> };
>
> There is no need to provide filenames in every node.
I know, please check above.
>
> > +
> > + hash-1 {
> > + algo = "sha1";
> > + };
> > + };
> > + u-boot-env {
> > + description = "U-Boot environment
> > on SPI Flash";
> > + compression = "none";
> > + type = "firmware";
> > + arch = "sandbox";
> > + load = <0>;
> > + blob {
> > + filename =
> > UBOOT_ENV_IMAGE_NEW;
> > +
Re: [PATCH v7 11/11] sandbox: trace: Increase trace buffer size
hi Simon,
On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 05:36, Sughosh Ganu wrote:
> >
> > When running the trace test on the sandbox platform, the current size
> > of 16MiB is no longer large enough for capturing the entire trace
> > history, and results in truncation. Use a size of 32MiB for the trace
> > buffer on the sandbox platform while running the trace test.
> >
> > Signed-off-by: Sughosh Ganu
> > ---
> > Changes since V6:
> > * New patch for fixing CI trace test failure.
> >
> > .azure-pipelines.yml| 2 +-
> > .gitlab-ci.yml | 2 +-
> > test/py/tests/test_trace.py | 2 +-
> > 3 files changed, 3 insertions(+), 3 deletions(-)
> >
>
> Reviewed-by: Simon Glass
>
> Do you know what has made it jump up?
I suspect this is due to the increase in the size of the dtb with all
the additional binman nodes. It starts failing with the last capsule
node being added under the binman node.
-sughosh
>
> > diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
> > index 2678e5ae05..2ef4db5318 100644
> > --- a/.azure-pipelines.yml
> > +++ b/.azure-pipelines.yml
> > @@ -275,7 +275,7 @@ stages:
> >TEST_PY_BD: "sandbox"
> >BUILD_ENV: "FTRACE=1 NO_LTO=1"
> >TEST_PY_TEST_SPEC: "trace"
> > - OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
> > CONFIG_TRACE_EARLY_SIZE=0x0100"
> > + OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
> > CONFIG_TRACE_EARLY_SIZE=0x0100 -a CONFIG_TRACE_BUFFER_SIZE=0x0200"
> > coreboot:
> >TEST_PY_BD: "coreboot"
> >TEST_PY_ID: "--id qemu"
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 8010afae95..3e41299658 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -315,7 +315,7 @@ sandbox trace_test.py:
> > TEST_PY_BD: "sandbox"
> > BUILD_ENV: "FTRACE=1 NO_LTO=1"
> > TEST_PY_TEST_SPEC: "trace"
> > -OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
> > CONFIG_TRACE_EARLY_SIZE=0x0100"
> > +OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
> > CONFIG_TRACE_EARLY_SIZE=0x0100 -a CONFIG_TRACE_BUFFER_SIZE=0x0200"
> ><<: *buildman_and_testpy_dfn
> >
> > evb-ast2500 test.py:
> > diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py
> > index ac3e95925e..ad2250920d 100644
> > --- a/test/py/tests/test_trace.py
> > +++ b/test/py/tests/test_trace.py
> > @@ -61,7 +61,7 @@ def collect_trace(cons):
> >
> > # Read out the trace data
> > addr = 0x0200
> > -size = 0x0100
> > +size = 0x0200
> > out = cons.run_command(f'trace calls {addr:x} {size:x}')
> > print(out)
> > fname = os.path.join(TMPDIR, 'trace')
> > --
> > 2.34.1
> >
>
> Regards,
> Simon
Re: [PATCH v7 04/11] capsule: authenticate: Add capsule public key in platform's dtb
hi Simon,
On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> >
> > The EFI capsule authentication logic in u-boot expects the public key
> > in the form of an EFI Signature List(ESL) to be provided as part of
> > the platform's dtb. Currently, the embedding of the ESL file into the
> > dtb needs to be done manually.
> >
> > Add a signature node in the u-boot dtsi file and include the public
> > key through the capsule-key property. This file is per architecture,
> > and is currently being added for sandbox and arm architectures. It
> > will have to be added for other architectures which need to enable
> > capsule authentication support.
> >
> > The path to the ESL file is specified through the
> > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> >
> > Signed-off-by: Sughosh Ganu
> > ---
> > Changes since V6:
> > * Populate the CONFIG_EFI_CAPSULE_ESL_FILE symbol for sandbox and
> > sandbox_flattree which enable capsule authentication.
> >
> > Note:
> > Simon Glass had asked me to rid of the CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > ifdef used in the sandbox' u-boot.dtsi file. However, that results in
> > the sandbox_vpl test failing in CI. Hence that check has been kept.
> >
> >
> > arch/arm/dts/u-boot.dtsi | 14 ++
> > arch/sandbox/dts/u-boot.dtsi | 17 +
> > configs/sandbox_defconfig | 1 +
> > configs/sandbox_flattree_defconfig | 1 +
> > lib/efi_loader/Kconfig | 9 +
> > 5 files changed, 42 insertions(+)
> > create mode 100644 arch/arm/dts/u-boot.dtsi
> > create mode 100644 arch/sandbox/dts/u-boot.dtsi
> >
> > diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> > new file mode 100644
> > index 00..4f31da4521
> > --- /dev/null
> > +++ b/arch/arm/dts/u-boot.dtsi
> > @@ -0,0 +1,14 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/**
> > + * Devicetree file with miscellaneous nodes that will be included
> > + * at build time into the DTB. Currently being used for including
> > + * capsule related information.
> > + */
> > +
> > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > +/ {
> > + signature {
> > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > + };
> > +};
> > +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > new file mode 100644
> > index 00..60bd004937
> > --- /dev/null
> > +++ b/arch/sandbox/dts/u-boot.dtsi
> > @@ -0,0 +1,17 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Devicetree file with miscellaneous nodes that will be included
> > + * at build time into the DTB. Currently being used for including
> > + * capsule related information.
> > + *
> > + */
> > +
> > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > +/ {
> > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > + signature {
> > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > + };
> > +#endif
> > +};
> > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > index b6c4f735f2..779af4abc8 100644
> > --- a/configs/sandbox_defconfig
> > +++ b/configs/sandbox_defconfig
> > @@ -341,6 +341,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> > CONFIG_EFI_CAPSULE_ON_DISK=y
> > CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> > +CONFIG_EFI_CAPSULE_ESL_FILE="../../../board/sandbox/SIGNER.esl"
>
> Can we avoid the path here, and just use e.g. good.esl ?
Unfortunately no. I believe the way incbin works in dts is that it
looks for the binary to be included in the same directory as the dts.
Which is why I have to point it to the location of the esl relative to
the dts.
>
> Perhaps this could be fixed up later, e.g. by adding the board
> directory as an include dir when building the DT?
Again, this is not how incbin seems to work in dts. I tried putting
the esl in one of the existing include directory locations, but it
does not pick the file from those dirs. It works with the assumption
that the bin file is to be in the same dir as the dts.
>
> > CONFIG_EFI_SECURE_BOOT=y
> > CONFIG_TEST_FDTDEC=y
> > CONFIG_UNIT_TEST=y
> > diff --git a/configs/sandbox_flattree_defconfig
> > b/configs/sandbox_flattree_defconfig
> > index 8aa295686d..0ca2e4a5ae 100644
> > --- a/configs/sandbox_flattree_defconfig
> > +++ b/configs/sandbox_flattree_defconfig
> > @@ -227,6 +227,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> > CONFIG_EFI_CAPSULE_ON_DISK=y
> > CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> > CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> > +CONFIG_EFI_CAPSULE_ESL_FILE="../../../board/sandbox/SIGNER.esl"
> > CONFIG_UNIT_TEST=y
> > CONFIG_UT_TIME=y
> > CONFIG_UT_DM=y
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index a22e47616f..0d559ff3a1 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -235,6 +235,15 @@ config
Re: [PATCH v7 03/11] sandbox: capsule: Add keys and certificates needed for capsule update testing
hi Simon,
On Sat, 5 Aug 2023 at 20:34, Simon Glass wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> >
> > Add the private keys and public key certificates which are to be used
> > for capsule authentication while testing the EFI capsule update
> > functionality. There are two pairs of private and public keys. The
> > SIGNER.{key,crt} pair will be used for signing capsules, whilst the
> > SIGNER2.{key,crt} pair is to be used as malicious keys for testing
> > authentication failure cases. The SIGNER.crt is also converted to an
> > EFI Signature List(ESL) file, SIGNER.esl, which is embedded in the
> > platform's device-tree for capsule authentication.
> >
> > Signed-off-by: Sughosh Ganu
> > ---
> > Changes since V6:
> > * New patch that puts the keys and cert files under board/sandbox/
> > directory as suggested Simon Glass.
> >
> > board/sandbox/SIGNER.crt | 19 +++
> > board/sandbox/SIGNER.esl | Bin 0 -> 831 bytes
> > board/sandbox/SIGNER.key | 28
> > board/sandbox/SIGNER2.crt | 19 +++
> > board/sandbox/SIGNER2.key | 28
> > 5 files changed, 94 insertions(+)
> > create mode 100644 board/sandbox/SIGNER.crt
> > create mode 100644 board/sandbox/SIGNER.esl
> > create mode 100644 board/sandbox/SIGNER.key
> > create mode 100644 board/sandbox/SIGNER2.crt
> > create mode 100644 board/sandbox/SIGNER2.key
>
> Can we call these good.* and bad.* so it is clear what they are for?
> Also, please avoid capital letters in filenames.
I was using the same nomenclature that was being used currently by the
efi capsule update tests. But I guess I can change this.
-sughosh
Re: [PATCH v7 08/11] binman: capsule: Add support for generating EFI capsules
Hi Sughosh,
On Sat, 5 Aug 2023 at 09:03, Simon Glass wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
> >
> > Add support in binman for generating EFI capsules. The capsule
> > parameters can be specified through the capsule binman entry. Also add
> > test cases in binman for testing capsule generation.
> >
> > Signed-off-by: Sughosh Ganu
> > ---
> > Changes since V6:
> > * Add macros for the GUID strings in sandbox_efi_capsule.h
> > * Highlight that the private and public keys are mandatory for capsule
> > signing.
> > * Add a URL link to the UEFI spec, as used in the rst files.
> > * Use local vars for private and public keys in BuildSectionData()
> > * Use local vars for input payload and capsule filenames in
> > BuildSectionData().
> > * Drop the ProcessContents() and SetImagePos() as the superclass
> > functions suffice.
> > * Use GUID macro names in the capsule test dts files.
> > * Rename efi_capsule_payload.bin to capsule_input.bin.
> >
> >
> > include/sandbox_efi_capsule.h | 14 ++
>
> Please move this file to a later patch - see below.
>
> Could we have a single header file with all the GUIDs, i.e. sandbox, ARM, etc.
>
> > tools/binman/entries.rst | 62
> > tools/binman/etype/efi_capsule.py | 143 ++
> > tools/binman/ftest.py | 121 +++
> > tools/binman/test/307_capsule.dts | 23 +++
> > tools/binman/test/308_capsule_signed.dts | 25 +++
> > tools/binman/test/309_capsule_version.dts | 24 +++
> > tools/binman/test/310_capsule_signed_ver.dts | 26
> > tools/binman/test/311_capsule_oemflags.dts| 24 +++
> > tools/binman/test/312_capsule_missing_key.dts | 24 +++
> > .../binman/test/313_capsule_missing_index.dts | 22 +++
> > .../binman/test/314_capsule_missing_guid.dts | 19 +++
> > .../test/315_capsule_missing_payload.dts | 19 +++
> > 13 files changed, 546 insertions(+)
> > create mode 100644 include/sandbox_efi_capsule.h
> > create mode 100644 tools/binman/etype/efi_capsule.py
> > create mode 100644 tools/binman/test/307_capsule.dts
> > create mode 100644 tools/binman/test/308_capsule_signed.dts
> > create mode 100644 tools/binman/test/309_capsule_version.dts
> > create mode 100644 tools/binman/test/310_capsule_signed_ver.dts
> > create mode 100644 tools/binman/test/311_capsule_oemflags.dts
> > create mode 100644 tools/binman/test/312_capsule_missing_key.dts
> > create mode 100644 tools/binman/test/313_capsule_missing_index.dts
> > create mode 100644 tools/binman/test/314_capsule_missing_guid.dts
> > create mode 100644 tools/binman/test/315_capsule_missing_payload.dts
> >
[..]
> > diff --git a/tools/binman/test/307_capsule.dts
> > b/tools/binman/test/307_capsule.dts
> > new file mode 100644
> > index 00..86552ff83f
> > --- /dev/null
> > +++ b/tools/binman/test/307_capsule.dts
> > @@ -0,0 +1,23 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +/dts-v1/;
> > +
> > +#include
>
> We can't include sandbox files in binman! I Does it actually matter
> what the GUID value is? Can they all be the same? For now I think it
> is best to have
>
> #define BINMAN_TEST_GUID "xxx"
>
> repeated at the top of each .dts file. Hopefully at some point we can
> figure out a sensible way to provide a decoder ring for this mess, so
> we can use actually names in the binman definition instead of GUIDs.
Oh, having thought about this a bit more, there is a much better way.
In the entry type, allow the image_guid to be a string, like
'binman-test' or 'sandbox-test'. Then binman can have a conversion
function to figure out the GUID to pass to the tool, e.g. in
efi_capsule.py:
TYPE_TO_GUID = {
'binman-test': '09123987987f98712',
'sandbox-test':, '98123987123123987123987',
}
def get_guid(type_str):
'Get the GUID to use for a particular purpose"""
return TYPE_TO_GUID.get(type_str, type_str)
Then you can use these same strings in the sandbox tests, without
needing to include anything.
>
> > +
> > +/ {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + binman {
> > + efi-capsule {
> > + image-index = <0x1>;
> > + /* Image GUID for testing capsule update */
> > + image-type-id = SANDBOX_UBOOT_IMAGE_GUID;
> > + hardware-instance = <0x0>;
> > +
> > + blob {
> > + filename = "capsule_input.bin";
> > + };
> > + };
> > + };
> > +};
Regards,
Simon
Re: [PATCH] net: phy: broadcom: add support for BCM54210E
On 8/5/23 11:34, Rafał Miłecki wrote:
On 5.08.2023 05:38, Marek Vasut wrote:
It's Broadcom PHY simply described as single-port
RGMII 10/100/1000BASE-T PHY. It requires disabling
delay skew and GTXCLK bits.
Ported from Linux kernel commit
0fc9ae1076697 ("net: phy: broadcom: add support for BCM54210E")
Signed-off-by: Marek Vasut
---
Cc: Joe Hershberger
Cc: Marek Vasut
Cc: Michal Simek
Cc: Ramon Fried
Cc: Rasmus Villemoes
Including original author may be a good idea too :)
Done in V2 .
TBH I'm still unsure whether or not CC the kernel people on U-Boot patch
ports.
This patch isn't really correct, see below.
---
drivers/net/phy/broadcom.c | 66 +-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 36c70da181a..0967363b3bc 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -30,10 +30,47 @@
#define MIIM_BCM54XX_EXP_SEL_ER 0x0f00 /* Expansion
register select */
#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC 0x0007
-#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800
+#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_WIRESPEED_EN 0x0010
+#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_EN 0x0080
+#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN 0x0100
+#define MIIM_BCM_AUXCNTL_MISC_FORCE_AMDIX 0x0200
+#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800
+#define MIIM_BCM_AUXCNTL_MISC_WREN 0x8000
#define MIIM_BCM_CHANNEL_WIDTH 0x2000
+#define BCM54810_SHD_CLK_CTL 0x3
+#define BCM54810_SHD_CLK_CTL_GTXCLK_EN BIT(9)
+
+static int bcm54xx_auxctl_read(struct phy_device *phydev, u16 regnum)
+{
+ /* The register must be written to both the Shadow Register
Select and
+ * the Shadow Read Register Selector
+ */
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL,
+ MIIM_BCM54xx_AUXCNTL_ENCODE(regnum));
+ return phy_read(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL);
+}
+
+static int bcm54xx_auxctl_write(struct phy_device *phydev, u16
regnum, u16 val)
+{
+ return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL,
regnum | val);
+}
+
+static int bcm_phy_read_shadow(struct phy_device *phydev, u16 shadow)
+{
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD,
+ MIIM_BCM54XX_SHD_VAL(shadow));
+ return MIIM_BCM54XX_SHD_DATA(phy_read(phydev, MDIO_DEVAD_NONE,
+ MIIM_BCM54XX_SHD));
+}
+
+static int bcm_phy_write_shadow(struct phy_device *phydev, u16
shadow, u16 val)
+{
+ return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD,
+ MIIM_BCM54XX_SHD_WR_ENCODE(shadow, val));
+}
+
static void bcm_phy_write_misc(struct phy_device *phydev,
u16 reg, u16 chl, u16 value)
{
@@ -62,6 +99,23 @@ static int bcm5461_config(struct phy_device *phydev)
return 0;
}
+/* Broadcom BCM54210E */
+static int bcm54210e_config(struct phy_device *phydev)
+{
+ int val;
+
+ val = bcm54xx_auxctl_read(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC);
+ val &= ~MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN;
+ val |= MIIM_BCM_AUXCNTL_MISC_WREN;
+ bcm54xx_auxctl_write(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC, val);
+
+ val = bcm_phy_read_shadow(phydev, BCM54810_SHD_CLK_CTL);
+ val &= ~BCM54810_SHD_CLK_CTL_GTXCLK_EN;
+ bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, val);
+
+ return bcm5461_config(phydev);
+}
Above function hardcodes setup for a specific PHY mode. It should be
done depending on PHY mode specified in DT. See kernel commit
fea7fda7f50a ("net: phy: broadcom: Fix RGMII delays configuration for
BCM54210E")
Updated in v2, thanks !
Re: [PATCH v5 4/4] arm: dts: k3-am64: Sync DT with Linux v6.5-rc1
On 11:14-20230805, Roger Quadros wrote: > Sync all am642-evm/am642-sk related DT files > with Linux v6.5-rc1. > > - drop timer1 in favor of main_timer0 in am64-main.dtsi. > Need to delete clock & power domain properties of > main_timer1 in -r5.dts else won't boot. This is because > timer_init is done during rproc_start to start System Firmware, > but we can't do any clock/power-domain operations before > System Firmware starts. > - same constraint applies to main_uart0 > - drop cpsw3g custom DT property 'mac_efuse' and custom > DT node cpsw-phy-sel as driver picks these from standard > property/node. > - include board dts file in -r5 dts file to avoid duplication > of nodes. Include -u-boot.dtsi on top. > - drop duplicate nodes in -r5 dts and -u-boot.dtsi > > Signed-off-by: Roger Quadros Tested-by: Nishanth Menon Reviewed-by: Nishanth Menon Series looks good to me now. Thanks Roger for updating. -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
[PATCH] Azure: Squash a number of jobs and re-order slightly
To reduce overall job time, move a number of smaller jobs together. These should still be safely under 1 hour total time, but reducing the overall number of jobs should help with the queue slightly. Signed-off-by: Tom Rini --- .azure-pipelines.yml | 72 ++-- 1 file changed, 23 insertions(+), 49 deletions(-) diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 81913bc0686a..31850ae57180 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -475,8 +475,8 @@ stages: # Use almost the same target division in .travis.yml, only merged # 3 small build jobs (arc/microblaze/xtensa) into one. matrix: -arc_microblaze_xtensa: - BUILDMAN: "arc microblaze xtensa" +arc_nios2_m68k_microblaze_xtensa: + BUILDMAN: "arc nios2 microblaze m68k xtensa" amlogic: BUILDMAN: "amlogic" arm11_arm7_arm920t_arm946es: @@ -493,90 +493,64 @@ stages: BUILDMAN: "bcm -x mips" nxp_arm32: BUILDMAN: "freescale -x powerpc,m68k,aarch64,ls101,ls102,ls104,ls108,ls20,lx216" -nxp_ls101x: - BUILDMAN: "freescale" +nxp_ls101x_ls108x: + BUILDMAN: "freescale freescale" nxp_ls102x: BUILDMAN: "freescale -x keymile" nxp_ls104x: BUILDMAN: "freescale" -nxp_ls108x: - BUILDMAN: "freescale" -nxp_ls20xx: - BUILDMAN: "freescale" -nxp_lx216x: - BUILDMAN: "freescale" +nxp_ls20xx_lx216x: + BUILDMAN: "freescale freescale" imx6: BUILDMAN: "mx6 -x boundary,engicam,freescale,technexion,toradex" imx: BUILDMAN: "mx -x mx6,imx8,freescale,technexion,toradex" imx8_imx9: BUILDMAN: "imx8 imx9 -x engicam,technexion,toradex" -keymile: - BUILDMAN: "keymile" +keymiles_siemens_technexion: + BUILDMAN: "keymile siemens technexion" keystone2_keystone3: - BUILDMAN: "k2 k3" + BUILDMAN: "k2 k3 -x siemens,toradex" sandbox_asan: BUILDMAN: "sandbox" OVERRIDE: "-a ASAN" sandbox_clang_asan: BUILDMAN: "sandbox" OVERRIDE: "-O clang-16 -a ASAN" -samsung_socfpga: - BUILDMAN: "samsung socfpga" -sun4i: - BUILDMAN: "sun4i" -sun5i: - BUILDMAN: "sun5i" -sun6i: - BUILDMAN: "sun6i" +samsung_socfpga_renesas: + BUILDMAN: "samsung socfpga renesas" +sun4i_sun9i: + BUILDMAN: "sun4i sun9i" +sun5i_sun6i: + BUILDMAN: "sun5i sun6i" sun7i: BUILDMAN: "sun7i" -sun8i_32bit: - BUILDMAN: "sun8i" -sun8i_64bit: - BUILDMAN: "sun8i" -sun9i: - BUILDMAN: "sun9i" +sun8i: + BUILDMAN: "sun8i" sun50i: BUILDMAN: "sun50i" arm_catch_all: BUILDMAN: "arm -x arm11,arm7,arm9,aarch64,at91,bcm,freescale,kirkwood,mvebu,renesas,siemens,tegra,uniphier,mx,samsung,sunxi,am33xx,omap,toradex,socfpga,k2,k3,zynq" sandbox_x86: BUILDMAN: "sandbox x86" -technexion: - BUILDMAN: "technexion" -kirkwood: - BUILDMAN: "kirkwood" -mvebu: - BUILDMAN: "mvebu" -m68k: - BUILDMAN: "m68k" +kirkwood_mvebu_uniphier: + BUILDMAN: "kirkwood mvebu uniphier" mips: BUILDMAN: "mips" powerpc: BUILDMAN: "powerpc -x keymile" -siemens: - BUILDMAN: "siemens" tegra: BUILDMAN: "tegra -x toradex" -am33xx_no_siemens: - BUILDMAN: "am33xx -x siemens" -omap: - BUILDMAN: "omap" -uniphier: - BUILDMAN: "uniphier" +am33xx_omap: + BUILDMAN: "am33xx omap -x siemens" aarch64_catch_all: BUILDMAN: "aarch64 -x amlogic,bcm,imx8,imx9,k3,tegra,ls1,ls2,lx216,mvebu,uniphier,renesas,sunxi,samsung,socfpga,rk,versal,zynq" rk_non_rockchip_64bit: BUILDMAN: "rk -x rockchip" rk_rockchip_64bit: BUILDMAN: "rk" -renesas: - BUILDMAN: "renesas" -zynq: - BUILDMAN: "zynq" -zynqmp_versal: - BUILDMAN: "versal|zynqmp" +zynq_zynqmp_versal: + BUILDMAN: "zynq versal zynqmp" riscv: BUILDMAN: "riscv" steps: -- 2.34.1
Re: [PATCH v7 10/11] doc: Add documentation to highlight capsule generation related updates
Hi Sughosh, On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote: > > The EFI capsules can now be generated as part of u-boot build, through Please check your patches for 'U-Boot' throughout. > binman. Highlight these changes in the documentation. > > Signed-off-by: Sughosh Ganu > --- > Changes since V6: None > > doc/develop/uefi/uefi.rst | 18 ++ > 1 file changed, 18 insertions(+) > > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst > index b2854b52a6..3150d6fb9c 100644 > --- a/doc/develop/uefi/uefi.rst > +++ b/doc/develop/uefi/uefi.rst > @@ -318,6 +318,9 @@ Run the following command >--guid \ > > > +Capsule with firmware version > +* > + > The UEFI specification does not define the firmware versioning mechanism. > EDK II reference implementation inserts the FMP Payload Header right before > the payload. It coutains the fw_version and lowest supported version, > @@ -345,6 +348,21 @@ add --fw-version option in mkeficapsule tool. > If the --fw-version option is not set, FMP Payload Header is not inserted > and fw_version is set as 0. > > + > +Capsule Generation through binman > +* > + > +Support has also been added to generate capsules during u-boot build > +through binman. This requires the platform's DTB to be populated with > +the capsule entry nodes for binman. The capsules then can be generated > +by specifying the capsule parameters either through a config file, or not a config file at present > +by specifying them as properties in the capsule entry node. > + > +Check the arch/sandbox/dts/u-boot.dtsi file for the sandbox platform > +as reference for how to generate capsules through binman as part of > +u-boot build. > + > + > Performing the update > * > > -- > 2.34.1 > Regards, Simon
Re: [PATCH v7 07/11] btool: mkeficapsule: Add a bintool for EFI capsule generation
Hi Sughosh,
On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
>
> Add a bintool for generating EFI capsules. This calls the mkeficapsule
> tool which generates the capsules.
>
> Signed-off-by: Sughosh Ganu
> ---
> Changes since V6:
> * Split the changes for mkeficapsule btool into a separate patch, as
> suggested by Simon Glass.
> * Use the word commandline consistently, as suggested by Simon Glass.
>
> tools/binman/btool/mkeficapsule.py | 101 +
> 1 file changed, 101 insertions(+)
> create mode 100644 tools/binman/btool/mkeficapsule.py
>
Reviewed-by: Simon Glass
> diff --git a/tools/binman/btool/mkeficapsule.py
> b/tools/binman/btool/mkeficapsule.py
> new file mode 100644
> index 00..61179747ff
> --- /dev/null
> +++ b/tools/binman/btool/mkeficapsule.py
> @@ -0,0 +1,101 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright 2023 Linaro Limited
> +#
> +"""Bintool implementation for mkeficapsule tool
> +
> +mkeficapsule is a tool used for generating EFI capsules.
> +
> +The following are the commandline options to be provided
> +to the tool
> +Usage: mkeficapsule [options]
> +Options:
> + -g, --guid guid for image blob type
> + -i, --index update image index
> + -I, --instanceupdate hardware instance
> + -v, --fw-version firmware version
> + -p, --private-key private key file
> + -c, --certificate signer's certificate file
> + -m, --monotonic-count monotonic count
> + -d, --dump_sig dump signature (*.p7)
> + -A, --fw-accept firmware accept capsule, requires GUID, no image blob
> + -R, --fw-revert firmware revert capsule, takes no GUID, no image blob
> + -o, --capoemflag Capsule OEM Flag, an integer between 0x and
> 0x
> + -h, --help print a help message
> +"""
> +
> +from binman import bintool
> +
> +class Bintoolmkeficapsule(bintool.Bintool):
> +"""Handles the 'mkeficapsule' tool
> +
> +This bintool is used for generating the EFI capsules. The
> +capsule generation parameters can either be specified through
> +commandline, or through a config file.
> +"""
> +def __init__(self, name):
> +super().__init__(name, 'mkeficapsule tool for generating capsules')
> +
> +def generate_capsule(self, image_index, image_guid, hardware_instance,
> + payload, output_fname, priv_key, pub_key,
> + monotonic_count=0, version=0, oemflags=0):
> +"""Generate a capsule through commandline-provided parameters
> +
> +Args:
> +image_index (int): Unique number for identifying payload image
> +image_guid (str): GUID used for identifying the image
I wonder what we can do about this, so that we don't have to speak in
GUIDs? Is there a registry somewhere of what all these things are? It
would be nice if you could provide a string like 'u-boot-sandbox' and
the capsule tool would know what that means.
> +hardware_instance (int): Optional unique hardware instance of
> +a device in the system. 0 if not being used
> +payload (str): Path to the input payload image
> +output_fname (str): Path to the output capsule file
> +priv_key (str): Path to the private key
> +pub_key(str): Path to the public key
> +monotonic_count (int): Count used when signing an image
> +version (int): Image version (Optional)
> +oemflags (int): Optional 16 bit OEM flags
> +
> +Returns:
> +str: Tool output
> +"""
> +args = [
> +f'--index={image_index}',
> +f'--guid={image_guid}',
> +f'--instance={hardware_instance}'
> +]
> +
> +if version:
> +args += [f'--fw-version={version}']
> +if oemflags:
> +args += [f'--capoemflag={oemflags}']
> +if priv_key and pub_key:
> +args += [
> +f'--monotonic-count={monotonic_count}',
> +f'--private-key={priv_key}',
> +f'--certificate={pub_key}'
> +]
It almost seems worth adding two methods in this class, one to build
with keys and one to not. Anyway, we can leave it for now.
> +
> +args += [
> +payload,
> +output_fname
> +]
> +
> +return self.run_cmd(*args)
> +
> +def fetch(self, method):
> +"""Fetch handler for mkeficapsule
> +
> +This builds the tool from source
> +
> +Returns:
> +tuple:
> +str: Filename of fetched file to copy to a suitable directory
> +str: Name of temp directory to remove, or None
> +"""
> +if method != bintool.FETCH_BUILD:
> +return None
> +
> +cmd = ['tools-only_defconfig', 'tools']
> +result = self.build_from_git(
> +
Re: [PATCH v7 09/11] sandbox: capsule: Generate capsule related files through binman
Hi Sughosh,
On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
>
> The EFI capsule files can now be generated as part of u-boot
> build through binman. Add capsule entry nodes in the u-boot.dtsi for
> the sandbox architecture for generating the capsules. These capsules
> are then used for testing the EFI capsule update functionality on the
> sandbox platforms. Also add binman nodes for generating the input
> files used for these capsules.
>
> Remove the corresponding logic which was used for generation of these
> input files which is now superfluous.
>
> Signed-off-by: Sughosh Ganu
> ---
> Changes since V6:
> * Use macros defined in sandbox_efi_capsule for GUIDs and capsule
> input filenames.
> * Generate the capsule input files through binman text entries.
>
> arch/sandbox/dts/u-boot.dtsi | 363 ++
> include/sandbox_efi_capsule.h | 11 +
> test/py/tests/test_efi_capsule/conftest.py| 168 +---
> test/py/tests/test_efi_capsule/signature.dts | 10 -
> .../tests/test_efi_capsule/uboot_bin_env.its | 36 --
> 5 files changed, 385 insertions(+), 203 deletions(-)
> delete mode 100644 test/py/tests/test_efi_capsule/signature.dts
> delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
I think you are still getting confused with using filenames when you
don't need to. See below...
>
> diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> index 60bd004937..f1b16b41c2 100644
> --- a/arch/sandbox/dts/u-boot.dtsi
> +++ b/arch/sandbox/dts/u-boot.dtsi
> @@ -7,11 +7,374 @@
> */
>
> #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> +
> +#include
> +
> / {
> #ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> signature {
> capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> };
> #endif
> +
> + binman: binman {
> + multiple-images;
> + };
> +};
> +
> + {
> + input1 {
> + filename = UBOOT_BIN_IMAGE_OLD;
> + text {
> + text = "u-boot:Old";
> + };
> + };
> +
> + input2 {
> + filename = UBOOT_BIN_IMAGE_NEW;
> + text {
> + text = "u-boot:New";
> + };
> + };
> +
> + input3 {
> + filename = UBOOT_ENV_IMAGE_OLD;
> + text {
> + text = "u-boot-env:Old";
> + };
> + };
> +
> + input4 {
> + filename = UBOOT_ENV_IMAGE_NEW;
> + text {
> + text = "u-boot-env:New";
> + };
> + };
All of those nodes can be removed
> +
> + itb {
> + filename = UBOOT_FIT_IMAGE;
> +
> + fit {
> + description = "Automatic U-Boot environment update";
> + #address-cells = <2>;
> +
> + images {
> + u-boot-bin {
> + description = "U-Boot binary on SPI
> Flash";
> + compression = "none";
> + type = "firmware";
> + arch = "sandbox";
> + load = <0>;
> + blob {
> + filename =
> UBOOT_BIN_IMAGE_NEW;
> + };
instead of this blob node, you should be able to write:
text {
text = "u-boot:Old";
};
There is no need to provide filenames in every node.
> +
> + hash-1 {
> + algo = "sha1";
> + };
> + };
> + u-boot-env {
> + description = "U-Boot environment on
> SPI Flash";
> + compression = "none";
> + type = "firmware";
> + arch = "sandbox";
> + load = <0>;
> + blob {
> + filename =
> UBOOT_ENV_IMAGE_NEW;
> + };
same here and below
> +
> + hash-1 {
> + algo = "sha1";
> + };
> + };
> + };
> + };
> + };
> +
> + capsule1 {
> + filename = "Test01";
> + capsule {
> + type = "efi-capsule";
> + image-index = <0x1>;
> + image-type-id = SANDBOX_UBOOT_IMAGE_GUID;
> +
> + blob {
> +
Re: [PATCH v7 08/11] binman: capsule: Add support for generating EFI capsules
Hi Sughosh, On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote: > > Add support in binman for generating EFI capsules. The capsule > parameters can be specified through the capsule binman entry. Also add > test cases in binman for testing capsule generation. > > Signed-off-by: Sughosh Ganu > --- > Changes since V6: > * Add macros for the GUID strings in sandbox_efi_capsule.h > * Highlight that the private and public keys are mandatory for capsule > signing. > * Add a URL link to the UEFI spec, as used in the rst files. > * Use local vars for private and public keys in BuildSectionData() > * Use local vars for input payload and capsule filenames in > BuildSectionData(). > * Drop the ProcessContents() and SetImagePos() as the superclass > functions suffice. > * Use GUID macro names in the capsule test dts files. > * Rename efi_capsule_payload.bin to capsule_input.bin. > > > include/sandbox_efi_capsule.h | 14 ++ Please move this file to a later patch - see below. Could we have a single header file with all the GUIDs, i.e. sandbox, ARM, etc. > tools/binman/entries.rst | 62 > tools/binman/etype/efi_capsule.py | 143 ++ > tools/binman/ftest.py | 121 +++ > tools/binman/test/307_capsule.dts | 23 +++ > tools/binman/test/308_capsule_signed.dts | 25 +++ > tools/binman/test/309_capsule_version.dts | 24 +++ > tools/binman/test/310_capsule_signed_ver.dts | 26 > tools/binman/test/311_capsule_oemflags.dts| 24 +++ > tools/binman/test/312_capsule_missing_key.dts | 24 +++ > .../binman/test/313_capsule_missing_index.dts | 22 +++ > .../binman/test/314_capsule_missing_guid.dts | 19 +++ > .../test/315_capsule_missing_payload.dts | 19 +++ > 13 files changed, 546 insertions(+) > create mode 100644 include/sandbox_efi_capsule.h > create mode 100644 tools/binman/etype/efi_capsule.py > create mode 100644 tools/binman/test/307_capsule.dts > create mode 100644 tools/binman/test/308_capsule_signed.dts > create mode 100644 tools/binman/test/309_capsule_version.dts > create mode 100644 tools/binman/test/310_capsule_signed_ver.dts > create mode 100644 tools/binman/test/311_capsule_oemflags.dts > create mode 100644 tools/binman/test/312_capsule_missing_key.dts > create mode 100644 tools/binman/test/313_capsule_missing_index.dts > create mode 100644 tools/binman/test/314_capsule_missing_guid.dts > create mode 100644 tools/binman/test/315_capsule_missing_payload.dts > > diff --git a/include/sandbox_efi_capsule.h b/include/sandbox_efi_capsule.h > new file mode 100644 > index 00..da71b87a18 > --- /dev/null > +++ b/include/sandbox_efi_capsule.h > @@ -0,0 +1,14 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) 2023, Linaro Limited > + */ > + > +#if !defined(_SANDBOX_EFI_CAPSULE_H_) > +#define _SANDBOX_EFI_CAPSULE_H_ > + > +#define SANDBOX_UBOOT_IMAGE_GUID "09d7cf52-0720-4710-91d1-08469b7fe9c8" > +#define SANDBOX_UBOOT_ENV_IMAGE_GUID "5a7021f5-fef2-48b4-aaba-832e777418c0" > +#define SANDBOX_FIT_IMAGE_GUID "3673b45d-6a7c-46f3-9e60-adabb03f7937" > +#define SANDBOX_INCORRECT_GUID "058b7d83-50d5-4c47-a195-60d86ad341c4" These should not be needed in binman tests. > + > +#endif /* _SANDBOX_EFI_CAPSULE_H_ */ > diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst > index f2376932be..fc094b9c08 100644 > --- a/tools/binman/entries.rst > +++ b/tools/binman/entries.rst > @@ -468,6 +468,68 @@ updating the EC on startup via software sync. > > > > +.. _etype_efi_capsule: > + > +Entry: capsule: Entry for generating EFI Capsule files > +-- > + > +The parameters needed for generation of the capsules can > +be provided as properties in the entry. > + > +Properties / Entry arguments: > +- image-index: Unique number for identifying corresponding > + payload image. Number between 1 and descriptor count, i.e. > + the total number of firmware images that can be updated. > +- image-type-id: Image GUID which will be used for identifying the > + updatable image on the board. > +- hardware-instance: Optional number for identifying unique > + hardware instance of a device in the system. Default value of 0 > + for images where value is not to be used. > +- fw-version: Optional value of image version that can be put on > + the capsule through the Firmware Management Protocol(FMP) header. > +- monotonic-count: Count used when signing an image. > +- private-key: Path to PEM formatted .key private key file. This property > + is mandatory for generating signed capsules. > +- public-key-cert: Path to PEM formatted .crt public key certificate > + file. This property is mandatory for generating signed capsules. > +- oem-flags - Optional OEM flags to be passed through capsule header. > + > +Since this is a
Re: [PATCH v7 11/11] sandbox: trace: Increase trace buffer size
Hi Sughosh,
On Sat, 5 Aug 2023 at 05:36, Sughosh Ganu wrote:
>
> When running the trace test on the sandbox platform, the current size
> of 16MiB is no longer large enough for capturing the entire trace
> history, and results in truncation. Use a size of 32MiB for the trace
> buffer on the sandbox platform while running the trace test.
>
> Signed-off-by: Sughosh Ganu
> ---
> Changes since V6:
> * New patch for fixing CI trace test failure.
>
> .azure-pipelines.yml| 2 +-
> .gitlab-ci.yml | 2 +-
> test/py/tests/test_trace.py | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
Reviewed-by: Simon Glass
Do you know what has made it jump up?
> diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
> index 2678e5ae05..2ef4db5318 100644
> --- a/.azure-pipelines.yml
> +++ b/.azure-pipelines.yml
> @@ -275,7 +275,7 @@ stages:
>TEST_PY_BD: "sandbox"
>BUILD_ENV: "FTRACE=1 NO_LTO=1"
>TEST_PY_TEST_SPEC: "trace"
> - OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
> CONFIG_TRACE_EARLY_SIZE=0x0100"
> + OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
> CONFIG_TRACE_EARLY_SIZE=0x0100 -a CONFIG_TRACE_BUFFER_SIZE=0x0200"
> coreboot:
>TEST_PY_BD: "coreboot"
>TEST_PY_ID: "--id qemu"
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 8010afae95..3e41299658 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -315,7 +315,7 @@ sandbox trace_test.py:
> TEST_PY_BD: "sandbox"
> BUILD_ENV: "FTRACE=1 NO_LTO=1"
> TEST_PY_TEST_SPEC: "trace"
> -OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
> CONFIG_TRACE_EARLY_SIZE=0x0100"
> +OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
> CONFIG_TRACE_EARLY_SIZE=0x0100 -a CONFIG_TRACE_BUFFER_SIZE=0x0200"
><<: *buildman_and_testpy_dfn
>
> evb-ast2500 test.py:
> diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py
> index ac3e95925e..ad2250920d 100644
> --- a/test/py/tests/test_trace.py
> +++ b/test/py/tests/test_trace.py
> @@ -61,7 +61,7 @@ def collect_trace(cons):
>
> # Read out the trace data
> addr = 0x0200
> -size = 0x0100
> +size = 0x0200
> out = cons.run_command(f'trace calls {addr:x} {size:x}')
> print(out)
> fname = os.path.join(TMPDIR, 'trace')
> --
> 2.34.1
>
Regards,
Simon
Re: [PATCH v7 04/11] capsule: authenticate: Add capsule public key in platform's dtb
Hi Sughosh,
On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
>
> The EFI capsule authentication logic in u-boot expects the public key
> in the form of an EFI Signature List(ESL) to be provided as part of
> the platform's dtb. Currently, the embedding of the ESL file into the
> dtb needs to be done manually.
>
> Add a signature node in the u-boot dtsi file and include the public
> key through the capsule-key property. This file is per architecture,
> and is currently being added for sandbox and arm architectures. It
> will have to be added for other architectures which need to enable
> capsule authentication support.
>
> The path to the ESL file is specified through the
> CONFIG_EFI_CAPSULE_ESL_FILE symbol.
>
> Signed-off-by: Sughosh Ganu
> ---
> Changes since V6:
> * Populate the CONFIG_EFI_CAPSULE_ESL_FILE symbol for sandbox and
> sandbox_flattree which enable capsule authentication.
>
> Note:
> Simon Glass had asked me to rid of the CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> ifdef used in the sandbox' u-boot.dtsi file. However, that results in
> the sandbox_vpl test failing in CI. Hence that check has been kept.
>
>
> arch/arm/dts/u-boot.dtsi | 14 ++
> arch/sandbox/dts/u-boot.dtsi | 17 +
> configs/sandbox_defconfig | 1 +
> configs/sandbox_flattree_defconfig | 1 +
> lib/efi_loader/Kconfig | 9 +
> 5 files changed, 42 insertions(+)
> create mode 100644 arch/arm/dts/u-boot.dtsi
> create mode 100644 arch/sandbox/dts/u-boot.dtsi
>
> diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> new file mode 100644
> index 00..4f31da4521
> --- /dev/null
> +++ b/arch/arm/dts/u-boot.dtsi
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/**
> + * Devicetree file with miscellaneous nodes that will be included
> + * at build time into the DTB. Currently being used for including
> + * capsule related information.
> + */
> +
> +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> +/ {
> + signature {
> + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> + };
> +};
> +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> new file mode 100644
> index 00..60bd004937
> --- /dev/null
> +++ b/arch/sandbox/dts/u-boot.dtsi
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Devicetree file with miscellaneous nodes that will be included
> + * at build time into the DTB. Currently being used for including
> + * capsule related information.
> + *
> + */
> +
> +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> +/ {
> +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> + signature {
> + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> + };
> +#endif
> +};
> +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index b6c4f735f2..779af4abc8 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -341,6 +341,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> CONFIG_EFI_CAPSULE_ON_DISK=y
> CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> +CONFIG_EFI_CAPSULE_ESL_FILE="../../../board/sandbox/SIGNER.esl"
Can we avoid the path here, and just use e.g. good.esl ?
Perhaps this could be fixed up later, e.g. by adding the board
directory as an include dir when building the DT?
> CONFIG_EFI_SECURE_BOOT=y
> CONFIG_TEST_FDTDEC=y
> CONFIG_UNIT_TEST=y
> diff --git a/configs/sandbox_flattree_defconfig
> b/configs/sandbox_flattree_defconfig
> index 8aa295686d..0ca2e4a5ae 100644
> --- a/configs/sandbox_flattree_defconfig
> +++ b/configs/sandbox_flattree_defconfig
> @@ -227,6 +227,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> CONFIG_EFI_CAPSULE_ON_DISK=y
> CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> +CONFIG_EFI_CAPSULE_ESL_FILE="../../../board/sandbox/SIGNER.esl"
> CONFIG_UNIT_TEST=y
> CONFIG_UT_TIME=y
> CONFIG_UT_DM=y
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index a22e47616f..0d559ff3a1 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -235,6 +235,15 @@ config EFI_CAPSULE_MAX
> Select the max capsule index value used for capsule report
> variables. This value is used to create CapsuleMax variable.
>
> +config EFI_CAPSULE_ESL_FILE
> + string "Path to the EFI Signature List File"
> + default ""
> + depends on EFI_CAPSULE_AUTHENTICATE
> + help
> + Provides the absolute path to the EFI Signature List file which
It isn't really an absolute path as it doesn't start with /
You really can't/shouldn't use absolute paths in a U-Boot build.
> + will be embedded in the platform's device tree and used for
> + capsule authentication at the time of capsule update.
> +
> config EFI_DEVICE_PATH_TO_TEXT
> bool "Device path to text protocol"
> default y
> --
>
Re: [PATCH v7 06/11] sandbox: Build the mkeficapsule tool for the sandbox variants
On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote: > > Build the mkeficapsule tool for all the sandbox variants. This tool > will be used subsequently for testing capsule generation in binman. > > Signed-off-by: Sughosh Ganu > --- > Changes since V6: > * New patch which has been split up from the binman capsule entry > support patch from earlier version, as suggested by Simon Glass. > * Enables mkeficapsule tool for all sandbox variants, instead of only > for sandbox_spl variant. > > tools/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Reviewed-by: Simon Glass At some point after this series is in, please take a look at why this tool isn't enabled for all builds.
Re: [PATCH v7 03/11] sandbox: capsule: Add keys and certificates needed for capsule update testing
Hi Sughosh,
On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu wrote:
>
> Add the private keys and public key certificates which are to be used
> for capsule authentication while testing the EFI capsule update
> functionality. There are two pairs of private and public keys. The
> SIGNER.{key,crt} pair will be used for signing capsules, whilst the
> SIGNER2.{key,crt} pair is to be used as malicious keys for testing
> authentication failure cases. The SIGNER.crt is also converted to an
> EFI Signature List(ESL) file, SIGNER.esl, which is embedded in the
> platform's device-tree for capsule authentication.
>
> Signed-off-by: Sughosh Ganu
> ---
> Changes since V6:
> * New patch that puts the keys and cert files under board/sandbox/
> directory as suggested Simon Glass.
>
> board/sandbox/SIGNER.crt | 19 +++
> board/sandbox/SIGNER.esl | Bin 0 -> 831 bytes
> board/sandbox/SIGNER.key | 28
> board/sandbox/SIGNER2.crt | 19 +++
> board/sandbox/SIGNER2.key | 28
> 5 files changed, 94 insertions(+)
> create mode 100644 board/sandbox/SIGNER.crt
> create mode 100644 board/sandbox/SIGNER.esl
> create mode 100644 board/sandbox/SIGNER.key
> create mode 100644 board/sandbox/SIGNER2.crt
> create mode 100644 board/sandbox/SIGNER2.key
Can we call these good.* and bad.* so it is clear what they are for?
Also, please avoid capital letters in filenames.
Regards,
Simon
Re: [PATCH] pinctrl: Check pinconfig nodes pre-reloc status recursively
Hi Massimo,
On 2023-08-05 16:06, Massimo Pegorer wrote:
> Hi Jonas,
>
> Il giorno sab 5 ago 2023 alle ore 13:11 Jonas Karlman ha
> scritto:
>
>> Pinconfig nodes normally bind recursively with PINCTRL_FULL and
>> PINCONF_RECURSIVE enabled. However, during U-Boot proper pre-relocation
>> any node marked with e.g. bootph-all will not bind unless its parent is
>> also marked for pre-reloc.
>>
>> group1 {
>> pinconf1 {
>> bootph-all;
>> };
>> };
>>
>> This cause the following warning message to be shown during U-Boot
>> proper pre-reloc stage on Rockchip RK3568 devices.
>>
>> ns16550_serial serial@fe66: pinctrl_select_state_full:
>> uclass_get_device_by_phandle_id: err=-19
>>
>> Check pinconfig nodes pre-reloc status recursively to fix this and to
>> make pinconfig_post_bind work same at both U-Boot proper pre-reloc and
>> at TPL/SPL stage.
>>
>> Signed-off-by: Jonas Karlman
>> ---
>> drivers/pinctrl/pinctrl-uclass.c | 18 +-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-uclass.c
>> b/drivers/pinctrl/pinctrl-uclass.c
>> index 73dd7b1038bb..fe2ba5021a78 100644
>> --- a/drivers/pinctrl/pinctrl-uclass.c
>> +++ b/drivers/pinctrl/pinctrl-uclass.c
>> @@ -100,6 +100,22 @@ static int pinctrl_select_state_full(struct udevice
>> *dev, const char *statename)
>> return 0;
>> }
>>
>> +static bool ofnode_pre_reloc_recursive(ofnode parent)
>> +{
>> + ofnode child;
>> +
>> + if (ofnode_pre_reloc(parent))
>> + return true;
>> +
>> + if (CONFIG_IS_ENABLED(PINCONF_RECURSIVE)) {
>> + ofnode_for_each_subnode(child, parent)
>> + if (ofnode_pre_reloc_recursive(child))
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> /**
>> * pinconfig_post_bind() - post binding for PINCONFIG uclass
>> * Recursively bind its children as pinconfig devices.
>> @@ -119,7 +135,7 @@ static int pinconfig_post_bind(struct udevice *dev)
>>
>> dev_for_each_subnode(node, dev) {
>> if (pre_reloc_only &&
>> - !ofnode_pre_reloc(node))
>> + !ofnode_pre_reloc_recursive(node))
>> continue;
>> /*
>> * If this node has "compatible" property, this is not
>> --
>> 2.41.0
>>
>
> I've fixed the same issue for Rockchip RK3308 a couple of days ago with
> patch [1]
> as part of the series [2], adding required nodes with 'bootph-some-ram'
> property
> into *-u-boot.dtsi.
>
> With your solution, is there any risk not to warn about any pin that should
> be configured
> but is not due to any ancestor missing in device tree (in pre-reloc boot
> phase, I mean)?
Yes, with this any pinconf group that may contain a child/leaf node that
have bootph prop will bind so that the child/leaf node also can bind and
be referenced at pre-reloc phase. It is also fully recursive so should
fix the issue with any missed ancestor, when it comes to pinconfig.
For TPL/SPL the ofnode_pre_reloc always return true so this has not been
an issue at that phase. This change should make pinconfig_post_bind
behave same as it does in TPL/SPL where any child/leaf node bind.
Regards,
Jonas
>
> Thanks.
>
> Massimo
>
> [1]
> https://patchwork.ozlabs.org/project/uboot/patch/20230803110813.175956-4-massimo.pegorer+...@gmail.com/
>
> [2]
> https://patchwork.ozlabs.org/project/uboot/cover/20230803110813.175956-1-massimo.pegorer+...@gmail.com/
>
[PATCH v2] net: phy: broadcom: add support for BCM54210E
It's Broadcom PHY simply described as single-port
RGMII 10/100/1000BASE-T PHY. It requires disabling
delay skew and GTXCLK bits.
BCM54210E support ported from Linux kernel commit
0fc9ae1076697 ("net: phy: broadcom: add support for BCM54210E")
AUX/SHD/bcm54xx_config_clock_delay update ported from Linux 6.5-rc4 commit
28e219aea0b9e ("net: phy: broadcom: drop brcm_phy_setbits() and use
phy_set_bits() instead")
Signed-off-by: Marek Vasut
---
Cc: Joe Hershberger
Cc: Marek Vasut
Cc: Michal Simek
Cc: Rafał Miłecki
Cc: Ramon Fried
Cc: Rasmus Villemoes
---
V2: Add RGMII delay fix from up to Linux 6.5-rc4 per suggestion from Rafał
---
drivers/net/phy/broadcom.c | 101 -
1 file changed, 100 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 36c70da181a..82e3bbef7dd 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -30,10 +30,87 @@
#define MIIM_BCM54XX_EXP_SEL_ER0x0f00 /* Expansion register
select */
#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC 0x0007
-#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800
+#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_WIRESPEED_EN 0x0010
+#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_EN 0x0080
+#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN0x0100
+#define MIIM_BCM_AUXCNTL_MISC_FORCE_AMDIX 0x0200
+#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800
+#define MIIM_BCM_AUXCNTL_MISC_WREN 0x8000
#define MIIM_BCM_CHANNEL_WIDTH0x2000
+#define BCM54810_SHD_CLK_CTL 0x3
+#define BCM54810_SHD_CLK_CTL_GTXCLK_EN BIT(9)
+
+static int bcm54xx_auxctl_read(struct phy_device *phydev, u16 regnum)
+{
+ /* The register must be written to both the Shadow Register Select and
+* the Shadow Read Register Selector
+*/
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL,
+ MIIM_BCM54xx_AUXCNTL_ENCODE(regnum));
+ return phy_read(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL);
+}
+
+static int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val)
+{
+ return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL, regnum
| val);
+}
+
+static int bcm_phy_read_shadow(struct phy_device *phydev, u16 shadow)
+{
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD,
+ MIIM_BCM54XX_SHD_VAL(shadow));
+ return MIIM_BCM54XX_SHD_DATA(phy_read(phydev, MDIO_DEVAD_NONE,
+ MIIM_BCM54XX_SHD));
+}
+
+static int bcm_phy_write_shadow(struct phy_device *phydev, u16 shadow, u16 val)
+{
+ return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD,
+MIIM_BCM54XX_SHD_WR_ENCODE(shadow, val));
+}
+
+static int bcm54xx_config_clock_delay(struct phy_device *phydev)
+{
+ int rc, val;
+
+ /* handling PHY's internal RX clock delay */
+ val = bcm54xx_auxctl_read(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC);
+ val |= MIIM_BCM_AUXCNTL_MISC_WREN;
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+ /* Disable RGMII RXC-RXD skew */
+ val &= ~MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN;
+ }
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
+ /* Enable RGMII RXC-RXD skew */
+ val |= MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN;
+ }
+ rc = bcm54xx_auxctl_write(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC, val);
+ if (rc < 0)
+ return rc;
+
+ /* handling PHY's internal TX clock delay */
+ val = bcm_phy_read_shadow(phydev, BCM54810_SHD_CLK_CTL);
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
+ /* Disable internal TX clock delay */
+ val &= ~BCM54810_SHD_CLK_CTL_GTXCLK_EN;
+ }
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+ /* Enable internal TX clock delay */
+ val |= BCM54810_SHD_CLK_CTL_GTXCLK_EN;
+ }
+ rc = bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, val);
+ if (rc < 0)
+ return rc;
+
+ return 0;
+}
+
static void bcm_phy_write_misc(struct phy_device *phydev,
u16 reg, u16 chl, u16 value)
{
@@ -62,6 +139,18 @@ static int bcm5461_config(struct phy_device *phydev)
return 0;
}
+/* Broadcom BCM54210E */
+static int bcm54210e_config(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = bcm54xx_config_clock_delay(phydev);
+ if (ret < 0)
+ return ret;
+
+ return bcm5461_config(phydev);
+}
+
static int bcm54xx_parse_status(struct
Re: [PATCH] pinctrl: Check pinconfig nodes pre-reloc status recursively
Hi Jonas,
Il giorno sab 5 ago 2023 alle ore 13:11 Jonas Karlman ha
scritto:
> Pinconfig nodes normally bind recursively with PINCTRL_FULL and
> PINCONF_RECURSIVE enabled. However, during U-Boot proper pre-relocation
> any node marked with e.g. bootph-all will not bind unless its parent is
> also marked for pre-reloc.
>
> group1 {
> pinconf1 {
> bootph-all;
> };
> };
>
> This cause the following warning message to be shown during U-Boot
> proper pre-reloc stage on Rockchip RK3568 devices.
>
> ns16550_serial serial@fe66: pinctrl_select_state_full:
> uclass_get_device_by_phandle_id: err=-19
>
> Check pinconfig nodes pre-reloc status recursively to fix this and to
> make pinconfig_post_bind work same at both U-Boot proper pre-reloc and
> at TPL/SPL stage.
>
> Signed-off-by: Jonas Karlman
> ---
> drivers/pinctrl/pinctrl-uclass.c | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/pinctrl-uclass.c
> b/drivers/pinctrl/pinctrl-uclass.c
> index 73dd7b1038bb..fe2ba5021a78 100644
> --- a/drivers/pinctrl/pinctrl-uclass.c
> +++ b/drivers/pinctrl/pinctrl-uclass.c
> @@ -100,6 +100,22 @@ static int pinctrl_select_state_full(struct udevice
> *dev, const char *statename)
> return 0;
> }
>
> +static bool ofnode_pre_reloc_recursive(ofnode parent)
> +{
> + ofnode child;
> +
> + if (ofnode_pre_reloc(parent))
> + return true;
> +
> + if (CONFIG_IS_ENABLED(PINCONF_RECURSIVE)) {
> + ofnode_for_each_subnode(child, parent)
> + if (ofnode_pre_reloc_recursive(child))
> + return true;
> + }
> +
> + return false;
> +}
> +
> /**
> * pinconfig_post_bind() - post binding for PINCONFIG uclass
> * Recursively bind its children as pinconfig devices.
> @@ -119,7 +135,7 @@ static int pinconfig_post_bind(struct udevice *dev)
>
> dev_for_each_subnode(node, dev) {
> if (pre_reloc_only &&
> - !ofnode_pre_reloc(node))
> + !ofnode_pre_reloc_recursive(node))
> continue;
> /*
> * If this node has "compatible" property, this is not
> --
> 2.41.0
>
I've fixed the same issue for Rockchip RK3308 a couple of days ago with
patch [1]
as part of the series [2], adding required nodes with 'bootph-some-ram'
property
into *-u-boot.dtsi.
With your solution, is there any risk not to warn about any pin that should
be configured
but is not due to any ancestor missing in device tree (in pre-reloc boot
phase, I mean)?
Thanks.
Massimo
[1]
https://patchwork.ozlabs.org/project/uboot/patch/20230803110813.175956-4-massimo.pegorer+...@gmail.com/
[2]
https://patchwork.ozlabs.org/project/uboot/cover/20230803110813.175956-1-massimo.pegorer+...@gmail.com/
[RFC] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation
Devices for nodes with e.g. bootph-pre-ram are initialized three times.
1. At SPL stage (always bind and probe only if used)
2. At U-Boot proper pre-reloc (always bind and probe)
3. At U-Boot proper normal (always bind and probe only if used)
Change ofnode_pre_reloc to report a node with bootph-pre-ram/sram prop
with a pre-reloc status only after U-Boot proper pre-relocation stage.
This prevents the device from being probed at U-Boot proper pre-reloc.
Signed-off-by: Jonas Karlman
---
I am not sure if U-Boot proper pre-reloc behaves like this by design and
if there is some other way to signal that a device should not be probed
during U-Boot proper pre-reloc stage if it has been probed at SPL stage.
For my use-case I added bootph-pre-ram prop to my RK8xx device node to
make the PMIC usable in SPL. However, I have no need for this device to
probe at U-Boot proper pre-reloc stage just after jumping out of TF-A.
And moments later bind and probe yet again at U-Boot proper normal stage.
The bootph-pre-ram prop was used to have the device usable in SPL, else
I could have used bootph-all or added bootph-some-ram prop to indicate
use at U-Boot proper pre-reloc stage.
drivers/core/ofnode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 8df16e56af5c..ebd5a408ae58 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -1353,7 +1353,7 @@ bool ofnode_pre_reloc(ofnode node)
*/
if (ofnode_read_bool(node, "bootph-pre-ram") ||
ofnode_read_bool(node, "bootph-pre-sram"))
- return true;
+ return !!(gd->flags & GD_FLG_RELOC);
if (IS_ENABLED(CONFIG_OF_TAG_MIGRATE)) {
/* detect and handle old tags */
--
2.41.0
[PULL] u-boot-usb/master
The following changes since commit 9787da0d32c2d58bae790a16ded0fe0c150c3280: Merge branch '2023-08-04-toradex-platform-updates' (2023-08-04 16:04:11 -0400) are available in the Git repository at: git://source.denx.de/u-boot-usb.git master for you to fetch changes up to 276c0c8e8a4543ce0b2c4ea020536c64ead98342: cmd: Enable BIND by default if we have USB_ETHER (2023-08-05 06:05:06 +0200) Marek Vasut (3): usb: gadget: ether: Inline functions used once usb: gadget: ether: Move probe function above driver structure usb: gadget: ether: Handle gadget driver registration in probe and remove Tom Rini (1): cmd: Enable BIND by default if we have USB_ETHER cmd/Kconfig| 1 + drivers/usb/gadget/ether.c | 171 - 2 files changed, 75 insertions(+), 97 deletions(-)
Re: [PATCH v2 3/7] power: regulator-uclass: perform regulator setup inside uclass
Hi,
On 2023-07-21 07:34, Svyatoslav Ryhel wrote:
> чт, 20 лип. 2023 р. о 22:43 Simon Glass пише:
>>
>> Hi Svyatoslav,
>>
>> On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel wrote:
>>>
>>> Regulators initial setup was previously dependent on board call.
>>> To move from this behaviour were introduced two steps. First is
>>> that all individual regulators will be probed just after binding
>>
>> We must not probe devices automatically when bound. The i2c interface
>> may not be available, etc. Do a probe afterwards.
>>
>> Perhaps I am misunderstanding this, iwc please reword this commit message.
>
> After bound. If the regulator is a self-sufficient i2c device then it
> will be bound
> after i2c is available by code design so i2c interface should be
> available at that
> moment. At least led and gpio uclasses perform this for initial setup
> of devices.
>
> Platform regulators (aka fixed/gpio regulators) work perfectly fine. I
> have no i2c
> regulators to test deeply.
>
> As for now only one uclass is not compatible with this system - PMIC which has
> strong dependency between regulator and main mfd. This is why probing after
> bind for pmic regulators is disabled and pmic regulators are probed on pmic
> core
> post_probe.
This sounds more like a possible bug of some dependency not being
probed in correct order or not leaving the device in a ready state
after probe.
If pmic regulators are bind in pmic bind with the pmic as parent, then
at regulator probe the driver core ensure that pmic has probed before
regulator use.
This works perfect fine with the RK8xx I2C PMIC and its regulators.
Wich a call to device_get_supply_regulator(dev, "test-supply", )
probe happens in i2c, pmic and regulator order.
>
>>> which ensures that regulator pdata is filled and second is setting
>>> up regulator in post probe which enseres that correct regulator
>>> state according to pdata is reached.
I think that the approach in this patch is trying to change too much all
at once.
Have made some adjustments that I think should help with transition.
- Added a flag to protect regulator_autoset from being called more then
once for each regulator, in a separate patch.
- Changed to only probe-after-bind on regulators marked as
always-on/boot-on.
And it works something like this:
static int regulator_post_bind(struct udevice *dev)
{
[...]
if (uc_pdata->always_on || uc_pdata->boot_on)
dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
}
static int regulator_post_probe(struct udevice *dev)
{
ret = regulator_autoset(dev);
}
With that any always-on/boot-on regulator would automatically probe and
autoset after bind, remaining would only probe and autoset if they are
used.
This approach should also prevent having to change existing code that
may call autoset, and cleanup can be done in follow-up patches/series.
Probe-after-bind and call to autoset could also be protected with a new
Kconfig to help with transition.
See following for a working example using a new driver that depends
on that regulators have been autoset prior to regulator_get_value.
https://github.com/Kwiboo/u-boot-rockchip/compare/master...rk3568-io-domain
Or the two commits separate:
power: regulator: Only run autoset once for each regulator
https://github.com/Kwiboo/u-boot-rockchip/commit/05db4dbcb8f908b417ed5cae8a7a325c82112d75
power: regulator: Perform regulator setup inside uclass
https://github.com/Kwiboo/u-boot-rockchip/commit/489bd5d2c1a2a33824eac4f2d54399ef5dff4fdf
Regards,
Jonas
>>>
>>> Signed-off-by: Svyatoslav Ryhel
>>> ---
>>> drivers/power/regulator/regulator-uclass.c | 212 ++---
>>> include/power/regulator.h | 119
>>> 2 files changed, 62 insertions(+), 269 deletions(-)
>>
>> Regards,
>> SImon
[PATCH v4] rockchip: rk3568: Add EmbedFire Lubancat 2 support
LubanCat2 is a rk3568 based SBC from EmbedFire.
Specification:
- Rockchip rk3568
- LPDDR4/4X 1/2/4/8 GB
- TF scard slot
- eMMC 8/32/64/128 GB
- Gigabit ethernet x 2
- HDMI out
- USB 2.0 Host x 1
- USB 2.0 Type-C OTG x 1
- USB 3.0 Host x 1
- Mini PCIE interface for WIFI/BT module
- M.2 key for 2280 NVME
- 40 pin header
The dts file is sync from linux mainline.
Signed-off-by: Andy Yan
---
Changes in v4:
- Add entry for this board to evb_rk3568/MAINTAINERS
- document this board at doc/board/rockchip/rockchip.rst
- restore the copyright header of dts from linux kernel
- remove ETH_DESIGNWARE from defconfig
- remove mmc-hs200-1_8v from u-boot.dsti
Changes in v3:
- some alphabetical order update
- disable all SPI flash related options.
- remove bootph-all for pinctrl
- add emmc_datastrobe pinconfig for hs200/hs400 in u-boot.dtsi
- use USB_DWC3_GENERIC driver as Jonas suggested.
- add CONFIG_SPL_DM_SEQ_ALIAS
Changes in v2:
- enable SPL_FIT_SIGNATURE
arch/arm/dts/Makefile | 1 +
arch/arm/dts/rk3568-lubancat-2-u-boot.dtsi | 27 +
arch/arm/dts/rk3568-lubancat-2.dts | 733 +
board/rockchip/evb_rk3568/MAINTAINERS | 7 +
configs/lubancat-2-rk3568_defconfig| 85 +++
doc/board/rockchip/rockchip.rst| 1 +
6 files changed, 854 insertions(+)
create mode 100644 arch/arm/dts/rk3568-lubancat-2-u-boot.dtsi
create mode 100644 arch/arm/dts/rk3568-lubancat-2.dts
create mode 100644 configs/lubancat-2-rk3568_defconfig
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index bd518064f3..64c885dcf9 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -178,6 +178,7 @@ dtb-$(CONFIG_ROCKCHIP_RK3568) += \
rk3566-soquartz-cm4.dtb \
rk3566-soquartz-model-a.dtb \
rk3568-evb.dtb \
+ rk3568-lubancat-2.dtb \
rk3568-nanopi-r5c.dtb \
rk3568-nanopi-r5s.dtb \
rk3568-odroid-m1.dtb \
diff --git a/arch/arm/dts/rk3568-lubancat-2-u-boot.dtsi
b/arch/arm/dts/rk3568-lubancat-2-u-boot.dtsi
new file mode 100644
index 00..27c6277523
--- /dev/null
+++ b/arch/arm/dts/rk3568-lubancat-2-u-boot.dtsi
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2023 Rockchip Electronics Co., Ltd
+ * (C) Copyright 2023 Andy Yan
+ */
+
+#include "rk356x-u-boot.dtsi"
+
+/ {
+ chosen {
+ stdout-path =
+ };
+};
+
+ {
+ cap-mmc-highspeed;
+ mmc-ddr-1_8v;
+ mmc-hs400-1_8v;
+ mmc-hs400-enhanced-strobe;
+ pinctrl-0 = <_bus8 _clk _cmd _datastrobe>;
+};
+
+ {
+ bootph-all;
+ clock-frequency = <2400>;
+ status = "okay";
+};
diff --git a/arch/arm/dts/rk3568-lubancat-2.dts
b/arch/arm/dts/rk3568-lubancat-2.dts
new file mode 100644
index 00..e653b067aa
--- /dev/null
+++ b/arch/arm/dts/rk3568-lubancat-2.dts
@@ -0,0 +1,733 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/*
+ * Copyright (c) 2021 Rockchip Electronics Co., Ltd.
+ * Copyright (c) 2022 EmbedFire
+ */
+
+/dts-v1/;
+#include
+#include
+#include
+#include
+#include "rk3568.dtsi"
+
+/ {
+ model = "EmbedFire LubanCat 2";
+ compatible = "embedfire,lubancat-2", "rockchip,rk3568";
+
+ aliases {
+ ethernet0 =
+ ethernet1 =
+ mmc0 =
+ mmc1 =
+ };
+
+ chosen: chosen {
+ stdout-path = "serial2:150n8";
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ user_led: user-led {
+ label = "user_led";
+ linux,default-trigger = "heartbeat";
+ default-state = "on";
+ gpios = < RK_PC7 GPIO_ACTIVE_LOW>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_led_pin>;
+ };
+ };
+
+ hdmi-con {
+ compatible = "hdmi-connector";
+ type = "a";
+
+ port {
+ hdmi_con_in: endpoint {
+ remote-endpoint = <_out_con>;
+ };
+ };
+ };
+
+ dc_5v: dc-5v-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "dc_5v";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <500>;
+ regulator-max-microvolt = <500>;
+ };
+
+ vcc3v3_sys: vcc3v3-sys-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc3v3_sys";
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ vin-supply = <_sys>;
+ };
+
+ vcc5v0_sys: vcc5v0-sys-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "vcc5v0_sys";
+
[PATCH v7 11/11] sandbox: trace: Increase trace buffer size
When running the trace test on the sandbox platform, the current size
of 16MiB is no longer large enough for capturing the entire trace
history, and results in truncation. Use a size of 32MiB for the trace
buffer on the sandbox platform while running the trace test.
Signed-off-by: Sughosh Ganu
---
Changes since V6:
* New patch for fixing CI trace test failure.
.azure-pipelines.yml| 2 +-
.gitlab-ci.yml | 2 +-
test/py/tests/test_trace.py | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 2678e5ae05..2ef4db5318 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -275,7 +275,7 @@ stages:
TEST_PY_BD: "sandbox"
BUILD_ENV: "FTRACE=1 NO_LTO=1"
TEST_PY_TEST_SPEC: "trace"
- OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
CONFIG_TRACE_EARLY_SIZE=0x0100"
+ OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
CONFIG_TRACE_EARLY_SIZE=0x0100 -a CONFIG_TRACE_BUFFER_SIZE=0x0200"
coreboot:
TEST_PY_BD: "coreboot"
TEST_PY_ID: "--id qemu"
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 8010afae95..3e41299658 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -315,7 +315,7 @@ sandbox trace_test.py:
TEST_PY_BD: "sandbox"
BUILD_ENV: "FTRACE=1 NO_LTO=1"
TEST_PY_TEST_SPEC: "trace"
-OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
CONFIG_TRACE_EARLY_SIZE=0x0100"
+OVERRIDE: "-a CONFIG_TRACE=y -a CONFIG_TRACE_EARLY=y -a
CONFIG_TRACE_EARLY_SIZE=0x0100 -a CONFIG_TRACE_BUFFER_SIZE=0x0200"
<<: *buildman_and_testpy_dfn
evb-ast2500 test.py:
diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py
index ac3e95925e..ad2250920d 100644
--- a/test/py/tests/test_trace.py
+++ b/test/py/tests/test_trace.py
@@ -61,7 +61,7 @@ def collect_trace(cons):
# Read out the trace data
addr = 0x0200
-size = 0x0100
+size = 0x0200
out = cons.run_command(f'trace calls {addr:x} {size:x}')
print(out)
fname = os.path.join(TMPDIR, 'trace')
--
2.34.1
[PATCH v7 10/11] doc: Add documentation to highlight capsule generation related updates
The EFI capsules can now be generated as part of u-boot build, through binman. Highlight these changes in the documentation. Signed-off-by: Sughosh Ganu --- Changes since V6: None doc/develop/uefi/uefi.rst | 18 ++ 1 file changed, 18 insertions(+) diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index b2854b52a6..3150d6fb9c 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -318,6 +318,9 @@ Run the following command --guid \ +Capsule with firmware version +* + The UEFI specification does not define the firmware versioning mechanism. EDK II reference implementation inserts the FMP Payload Header right before the payload. It coutains the fw_version and lowest supported version, @@ -345,6 +348,21 @@ add --fw-version option in mkeficapsule tool. If the --fw-version option is not set, FMP Payload Header is not inserted and fw_version is set as 0. + +Capsule Generation through binman +* + +Support has also been added to generate capsules during u-boot build +through binman. This requires the platform's DTB to be populated with +the capsule entry nodes for binman. The capsules then can be generated +by specifying the capsule parameters either through a config file, or +by specifying them as properties in the capsule entry node. + +Check the arch/sandbox/dts/u-boot.dtsi file for the sandbox platform +as reference for how to generate capsules through binman as part of +u-boot build. + + Performing the update * -- 2.34.1
[PATCH v7 09/11] sandbox: capsule: Generate capsule related files through binman
The EFI capsule files can now be generated as part of u-boot
build through binman. Add capsule entry nodes in the u-boot.dtsi for
the sandbox architecture for generating the capsules. These capsules
are then used for testing the EFI capsule update functionality on the
sandbox platforms. Also add binman nodes for generating the input
files used for these capsules.
Remove the corresponding logic which was used for generation of these
input files which is now superfluous.
Signed-off-by: Sughosh Ganu
---
Changes since V6:
* Use macros defined in sandbox_efi_capsule for GUIDs and capsule
input filenames.
* Generate the capsule input files through binman text entries.
arch/sandbox/dts/u-boot.dtsi | 363 ++
include/sandbox_efi_capsule.h | 11 +
test/py/tests/test_efi_capsule/conftest.py| 168 +---
test/py/tests/test_efi_capsule/signature.dts | 10 -
.../tests/test_efi_capsule/uboot_bin_env.its | 36 --
5 files changed, 385 insertions(+), 203 deletions(-)
delete mode 100644 test/py/tests/test_efi_capsule/signature.dts
delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
index 60bd004937..f1b16b41c2 100644
--- a/arch/sandbox/dts/u-boot.dtsi
+++ b/arch/sandbox/dts/u-boot.dtsi
@@ -7,11 +7,374 @@
*/
#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
+
+#include
+
/ {
#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
signature {
capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
};
#endif
+
+ binman: binman {
+ multiple-images;
+ };
+};
+
+ {
+ input1 {
+ filename = UBOOT_BIN_IMAGE_OLD;
+ text {
+ text = "u-boot:Old";
+ };
+ };
+
+ input2 {
+ filename = UBOOT_BIN_IMAGE_NEW;
+ text {
+ text = "u-boot:New";
+ };
+ };
+
+ input3 {
+ filename = UBOOT_ENV_IMAGE_OLD;
+ text {
+ text = "u-boot-env:Old";
+ };
+ };
+
+ input4 {
+ filename = UBOOT_ENV_IMAGE_NEW;
+ text {
+ text = "u-boot-env:New";
+ };
+ };
+
+ itb {
+ filename = UBOOT_FIT_IMAGE;
+
+ fit {
+ description = "Automatic U-Boot environment update";
+ #address-cells = <2>;
+
+ images {
+ u-boot-bin {
+ description = "U-Boot binary on SPI
Flash";
+ compression = "none";
+ type = "firmware";
+ arch = "sandbox";
+ load = <0>;
+ blob {
+ filename = UBOOT_BIN_IMAGE_NEW;
+ };
+
+ hash-1 {
+ algo = "sha1";
+ };
+ };
+ u-boot-env {
+ description = "U-Boot environment on
SPI Flash";
+ compression = "none";
+ type = "firmware";
+ arch = "sandbox";
+ load = <0>;
+ blob {
+ filename = UBOOT_ENV_IMAGE_NEW;
+ };
+
+ hash-1 {
+ algo = "sha1";
+ };
+ };
+ };
+ };
+ };
+
+ capsule1 {
+ filename = "Test01";
+ capsule {
+ type = "efi-capsule";
+ image-index = <0x1>;
+ image-type-id = SANDBOX_UBOOT_IMAGE_GUID;
+
+ blob {
+ filename = UBOOT_BIN_IMAGE_NEW;
+ };
+ };
+ };
+
+ capsule2 {
+ filename = "Test02";
+ capsule {
+ type = "efi-capsule";
+ image-index = <0x2>;
+ image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID;
+
+ blob {
+ filename = UBOOT_ENV_IMAGE_NEW;
+ };
+ };
+ };
+
+ capsule3 {
+ filename = "Test03";
+ capsule {
+ type =
[PATCH v7 08/11] binman: capsule: Add support for generating EFI capsules
Add support in binman for generating EFI capsules. The capsule parameters can be specified through the capsule binman entry. Also add test cases in binman for testing capsule generation. Signed-off-by: Sughosh Ganu --- Changes since V6: * Add macros for the GUID strings in sandbox_efi_capsule.h * Highlight that the private and public keys are mandatory for capsule signing. * Add a URL link to the UEFI spec, as used in the rst files. * Use local vars for private and public keys in BuildSectionData() * Use local vars for input payload and capsule filenames in BuildSectionData(). * Drop the ProcessContents() and SetImagePos() as the superclass functions suffice. * Use GUID macro names in the capsule test dts files. * Rename efi_capsule_payload.bin to capsule_input.bin. include/sandbox_efi_capsule.h | 14 ++ tools/binman/entries.rst | 62 tools/binman/etype/efi_capsule.py | 143 ++ tools/binman/ftest.py | 121 +++ tools/binman/test/307_capsule.dts | 23 +++ tools/binman/test/308_capsule_signed.dts | 25 +++ tools/binman/test/309_capsule_version.dts | 24 +++ tools/binman/test/310_capsule_signed_ver.dts | 26 tools/binman/test/311_capsule_oemflags.dts| 24 +++ tools/binman/test/312_capsule_missing_key.dts | 24 +++ .../binman/test/313_capsule_missing_index.dts | 22 +++ .../binman/test/314_capsule_missing_guid.dts | 19 +++ .../test/315_capsule_missing_payload.dts | 19 +++ 13 files changed, 546 insertions(+) create mode 100644 include/sandbox_efi_capsule.h create mode 100644 tools/binman/etype/efi_capsule.py create mode 100644 tools/binman/test/307_capsule.dts create mode 100644 tools/binman/test/308_capsule_signed.dts create mode 100644 tools/binman/test/309_capsule_version.dts create mode 100644 tools/binman/test/310_capsule_signed_ver.dts create mode 100644 tools/binman/test/311_capsule_oemflags.dts create mode 100644 tools/binman/test/312_capsule_missing_key.dts create mode 100644 tools/binman/test/313_capsule_missing_index.dts create mode 100644 tools/binman/test/314_capsule_missing_guid.dts create mode 100644 tools/binman/test/315_capsule_missing_payload.dts diff --git a/include/sandbox_efi_capsule.h b/include/sandbox_efi_capsule.h new file mode 100644 index 00..da71b87a18 --- /dev/null +++ b/include/sandbox_efi_capsule.h @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2023, Linaro Limited + */ + +#if !defined(_SANDBOX_EFI_CAPSULE_H_) +#define _SANDBOX_EFI_CAPSULE_H_ + +#define SANDBOX_UBOOT_IMAGE_GUID "09d7cf52-0720-4710-91d1-08469b7fe9c8" +#define SANDBOX_UBOOT_ENV_IMAGE_GUID "5a7021f5-fef2-48b4-aaba-832e777418c0" +#define SANDBOX_FIT_IMAGE_GUID "3673b45d-6a7c-46f3-9e60-adabb03f7937" +#define SANDBOX_INCORRECT_GUID "058b7d83-50d5-4c47-a195-60d86ad341c4" + +#endif /* _SANDBOX_EFI_CAPSULE_H_ */ diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index f2376932be..fc094b9c08 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -468,6 +468,68 @@ updating the EC on startup via software sync. +.. _etype_efi_capsule: + +Entry: capsule: Entry for generating EFI Capsule files +-- + +The parameters needed for generation of the capsules can +be provided as properties in the entry. + +Properties / Entry arguments: +- image-index: Unique number for identifying corresponding + payload image. Number between 1 and descriptor count, i.e. + the total number of firmware images that can be updated. +- image-type-id: Image GUID which will be used for identifying the + updatable image on the board. +- hardware-instance: Optional number for identifying unique + hardware instance of a device in the system. Default value of 0 + for images where value is not to be used. +- fw-version: Optional value of image version that can be put on + the capsule through the Firmware Management Protocol(FMP) header. +- monotonic-count: Count used when signing an image. +- private-key: Path to PEM formatted .key private key file. This property + is mandatory for generating signed capsules. +- public-key-cert: Path to PEM formatted .crt public key certificate + file. This property is mandatory for generating signed capsules. +- oem-flags - Optional OEM flags to be passed through capsule header. + +Since this is a subclass of Entry_section, all properties of the parent +class also apply here. + +For more details on the description of the capsule format, and the capsule +update functionality, refer Section 8.5 and Chapter 23 in the `UEFI +specification`_. + +The capsule parameters like image index and image GUID are passed as +properties in the entry. The payload to be used in the capsule is to be +provided as a subnode of the capsule entry.
[PATCH v7 07/11] btool: mkeficapsule: Add a bintool for EFI capsule generation
Add a bintool for generating EFI capsules. This calls the mkeficapsule
tool which generates the capsules.
Signed-off-by: Sughosh Ganu
---
Changes since V6:
* Split the changes for mkeficapsule btool into a separate patch, as
suggested by Simon Glass.
* Use the word commandline consistently, as suggested by Simon Glass.
tools/binman/btool/mkeficapsule.py | 101 +
1 file changed, 101 insertions(+)
create mode 100644 tools/binman/btool/mkeficapsule.py
diff --git a/tools/binman/btool/mkeficapsule.py
b/tools/binman/btool/mkeficapsule.py
new file mode 100644
index 00..61179747ff
--- /dev/null
+++ b/tools/binman/btool/mkeficapsule.py
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2023 Linaro Limited
+#
+"""Bintool implementation for mkeficapsule tool
+
+mkeficapsule is a tool used for generating EFI capsules.
+
+The following are the commandline options to be provided
+to the tool
+Usage: mkeficapsule [options]
+Options:
+ -g, --guid guid for image blob type
+ -i, --index update image index
+ -I, --instanceupdate hardware instance
+ -v, --fw-version firmware version
+ -p, --private-key private key file
+ -c, --certificate signer's certificate file
+ -m, --monotonic-count monotonic count
+ -d, --dump_sig dump signature (*.p7)
+ -A, --fw-accept firmware accept capsule, requires GUID, no image blob
+ -R, --fw-revert firmware revert capsule, takes no GUID, no image blob
+ -o, --capoemflag Capsule OEM Flag, an integer between 0x and 0x
+ -h, --help print a help message
+"""
+
+from binman import bintool
+
+class Bintoolmkeficapsule(bintool.Bintool):
+"""Handles the 'mkeficapsule' tool
+
+This bintool is used for generating the EFI capsules. The
+capsule generation parameters can either be specified through
+commandline, or through a config file.
+"""
+def __init__(self, name):
+super().__init__(name, 'mkeficapsule tool for generating capsules')
+
+def generate_capsule(self, image_index, image_guid, hardware_instance,
+ payload, output_fname, priv_key, pub_key,
+ monotonic_count=0, version=0, oemflags=0):
+"""Generate a capsule through commandline-provided parameters
+
+Args:
+image_index (int): Unique number for identifying payload image
+image_guid (str): GUID used for identifying the image
+hardware_instance (int): Optional unique hardware instance of
+a device in the system. 0 if not being used
+payload (str): Path to the input payload image
+output_fname (str): Path to the output capsule file
+priv_key (str): Path to the private key
+pub_key(str): Path to the public key
+monotonic_count (int): Count used when signing an image
+version (int): Image version (Optional)
+oemflags (int): Optional 16 bit OEM flags
+
+Returns:
+str: Tool output
+"""
+args = [
+f'--index={image_index}',
+f'--guid={image_guid}',
+f'--instance={hardware_instance}'
+]
+
+if version:
+args += [f'--fw-version={version}']
+if oemflags:
+args += [f'--capoemflag={oemflags}']
+if priv_key and pub_key:
+args += [
+f'--monotonic-count={monotonic_count}',
+f'--private-key={priv_key}',
+f'--certificate={pub_key}'
+]
+
+args += [
+payload,
+output_fname
+]
+
+return self.run_cmd(*args)
+
+def fetch(self, method):
+"""Fetch handler for mkeficapsule
+
+This builds the tool from source
+
+Returns:
+tuple:
+str: Filename of fetched file to copy to a suitable directory
+str: Name of temp directory to remove, or None
+"""
+if method != bintool.FETCH_BUILD:
+return None
+
+cmd = ['tools-only_defconfig', 'tools']
+result = self.build_from_git(
+'https://source.denx.de/u-boot/u-boot.git',
+cmd,
+'tools/mkeficapsule')
+return result
--
2.34.1
[PATCH v7 06/11] sandbox: Build the mkeficapsule tool for the sandbox variants
Build the mkeficapsule tool for all the sandbox variants. This tool will be used subsequently for testing capsule generation in binman. Signed-off-by: Sughosh Ganu --- Changes since V6: * New patch which has been split up from the binman capsule entry support patch from earlier version, as suggested by Simon Glass. * Enables mkeficapsule tool for all sandbox variants, instead of only for sandbox_spl variant. tools/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/Kconfig b/tools/Kconfig index 6e23f44d55..353a855243 100644 --- a/tools/Kconfig +++ b/tools/Kconfig @@ -91,10 +91,10 @@ config TOOLS_SHA512 Enable SHA512 support in the tools builds config TOOLS_MKEFICAPSULE - bool "Build efimkcapsule command" - default y if EFI_CAPSULE_ON_DISK + bool "Build mkeficapsule tool" + default y if EFI_CAPSULE_ON_DISK || SANDBOX help - This command allows users to create a UEFI capsule file and, + This tool allows users to create a UEFI capsule file and, optionally sign that file. If you want to enable UEFI capsule update feature on your target, you certainly need this. -- 2.34.1
[PATCH v7 05/11] doc: capsule: Document the new mechanism to embed ESL file into dtb
Update the document to specify how the EFI Signature List(ESL) file
can be embedded into the platform's dtb as part of the u-boot build.
Signed-off-by: Sughosh Ganu
Reviewed-by: Simon Glass
---
Changes since V6: None
doc/develop/uefi/uefi.rst | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index a7a41f2fac..b2854b52a6 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -522,20 +522,16 @@ and used by the steps highlighted below.
...
}
-You can do step-4 manually with
+You can perform step-4 by defining the Kconfig symbol
+CONFIG_EFI_CAPSULE_ESL_FILE. Once this has been done, the signature
+node can be added to the u-boot.dtsi file. For reference, check the
+u-boot.dtsi file for the sandbox architecture. If this node has not
+been added to the architecture's u-boot.dtsi file, this needs to be
+done. The node has currently been added for the sandbox and arm
+architectures' in the u-boot.dtsi file. Once the u-boot.dtsi file has
+been added with the signature node, the esl file will automatically
+get embedded into the platform's dtb as part of u-boot build.
-.. code-block:: console
-
-$ dtc -@ -I dts -O dtb -o signature.dtbo signature.dts
-$ fdtoverlay -i orig.dtb -o new.dtb -v signature.dtbo
-
-where signature.dts looks like::
-
-&{/} {
-signature {
-capsule-key = /incbin/("CRT.esl");
-};
-};
Anti-rollback Protection
--
2.34.1
[PATCH v7 04/11] capsule: authenticate: Add capsule public key in platform's dtb
The EFI capsule authentication logic in u-boot expects the public key
in the form of an EFI Signature List(ESL) to be provided as part of
the platform's dtb. Currently, the embedding of the ESL file into the
dtb needs to be done manually.
Add a signature node in the u-boot dtsi file and include the public
key through the capsule-key property. This file is per architecture,
and is currently being added for sandbox and arm architectures. It
will have to be added for other architectures which need to enable
capsule authentication support.
The path to the ESL file is specified through the
CONFIG_EFI_CAPSULE_ESL_FILE symbol.
Signed-off-by: Sughosh Ganu
---
Changes since V6:
* Populate the CONFIG_EFI_CAPSULE_ESL_FILE symbol for sandbox and
sandbox_flattree which enable capsule authentication.
Note:
Simon Glass had asked me to rid of the CONFIG_EFI_HAVE_CAPSULE_SUPPORT
ifdef used in the sandbox' u-boot.dtsi file. However, that results in
the sandbox_vpl test failing in CI. Hence that check has been kept.
arch/arm/dts/u-boot.dtsi | 14 ++
arch/sandbox/dts/u-boot.dtsi | 17 +
configs/sandbox_defconfig | 1 +
configs/sandbox_flattree_defconfig | 1 +
lib/efi_loader/Kconfig | 9 +
5 files changed, 42 insertions(+)
create mode 100644 arch/arm/dts/u-boot.dtsi
create mode 100644 arch/sandbox/dts/u-boot.dtsi
diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
new file mode 100644
index 00..4f31da4521
--- /dev/null
+++ b/arch/arm/dts/u-boot.dtsi
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0+
+/**
+ * Devicetree file with miscellaneous nodes that will be included
+ * at build time into the DTB. Currently being used for including
+ * capsule related information.
+ */
+
+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
+/ {
+ signature {
+ capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
+ };
+};
+#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
new file mode 100644
index 00..60bd004937
--- /dev/null
+++ b/arch/sandbox/dts/u-boot.dtsi
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Devicetree file with miscellaneous nodes that will be included
+ * at build time into the DTB. Currently being used for including
+ * capsule related information.
+ *
+ */
+
+#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
+/ {
+#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
+ signature {
+ capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
+ };
+#endif
+};
+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index b6c4f735f2..779af4abc8 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -341,6 +341,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
CONFIG_EFI_CAPSULE_ON_DISK=y
CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
CONFIG_EFI_CAPSULE_AUTHENTICATE=y
+CONFIG_EFI_CAPSULE_ESL_FILE="../../../board/sandbox/SIGNER.esl"
CONFIG_EFI_SECURE_BOOT=y
CONFIG_TEST_FDTDEC=y
CONFIG_UNIT_TEST=y
diff --git a/configs/sandbox_flattree_defconfig
b/configs/sandbox_flattree_defconfig
index 8aa295686d..0ca2e4a5ae 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -227,6 +227,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
CONFIG_EFI_CAPSULE_ON_DISK=y
CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
CONFIG_EFI_CAPSULE_AUTHENTICATE=y
+CONFIG_EFI_CAPSULE_ESL_FILE="../../../board/sandbox/SIGNER.esl"
CONFIG_UNIT_TEST=y
CONFIG_UT_TIME=y
CONFIG_UT_DM=y
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index a22e47616f..0d559ff3a1 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -235,6 +235,15 @@ config EFI_CAPSULE_MAX
Select the max capsule index value used for capsule report
variables. This value is used to create CapsuleMax variable.
+config EFI_CAPSULE_ESL_FILE
+ string "Path to the EFI Signature List File"
+ default ""
+ depends on EFI_CAPSULE_AUTHENTICATE
+ help
+ Provides the absolute path to the EFI Signature List file which
+ will be embedded in the platform's device tree and used for
+ capsule authentication at the time of capsule update.
+
config EFI_DEVICE_PATH_TO_TEXT
bool "Device path to text protocol"
default y
--
2.34.1
[PATCH v7 03/11] sandbox: capsule: Add keys and certificates needed for capsule update testing
Add the private keys and public key certificates which are to be used
for capsule authentication while testing the EFI capsule update
functionality. There are two pairs of private and public keys. The
SIGNER.{key,crt} pair will be used for signing capsules, whilst the
SIGNER2.{key,crt} pair is to be used as malicious keys for testing
authentication failure cases. The SIGNER.crt is also converted to an
EFI Signature List(ESL) file, SIGNER.esl, which is embedded in the
platform's device-tree for capsule authentication.
Signed-off-by: Sughosh Ganu
---
Changes since V6:
* New patch that puts the keys and cert files under board/sandbox/
directory as suggested Simon Glass.
board/sandbox/SIGNER.crt | 19 +++
board/sandbox/SIGNER.esl | Bin 0 -> 831 bytes
board/sandbox/SIGNER.key | 28
board/sandbox/SIGNER2.crt | 19 +++
board/sandbox/SIGNER2.key | 28
5 files changed, 94 insertions(+)
create mode 100644 board/sandbox/SIGNER.crt
create mode 100644 board/sandbox/SIGNER.esl
create mode 100644 board/sandbox/SIGNER.key
create mode 100644 board/sandbox/SIGNER2.crt
create mode 100644 board/sandbox/SIGNER2.key
diff --git a/board/sandbox/SIGNER.crt b/board/sandbox/SIGNER.crt
new file mode 100644
index 00..82d8576a64
--- /dev/null
+++ b/board/sandbox/SIGNER.crt
@@ -0,0 +1,19 @@
+-BEGIN CERTIFICATE-
+MIIDDzCCAfegAwIBAgIUUzrWhMi7oPFshQP6eFlccqf7exswDQYJKoZIhvcNAQEL
+BQAwFjEUMBIGA1UEAwwLVEVTVF9TSUdORVIwIBcNMjMwODA0MTgwNzQyWhgPMzAw
+MzEwMDYxODA3NDJaMBYxFDASBgNVBAMMC1RFU1RfU0lHTkVSMIIBIjANBgkqhkiG
+9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsAX2ldD9Y0c0utd1NU/uFW7jFbMRV4cByWOc
++Rcer/nFgX9yta7ivu3BJ1ueWR17zRNiQpIzLyEipoSPwyyViD5wLrPLRXVP0dru
+aCWyiPm+hm7mpjvwhvR7F2efJTguq9nJI4scaL7APUhbIXHHSL9mK8IlbFnshaR/
+qwd//nBW64HVqWlHNd+uxpFP2Qp0kQwb1b80USNWuMtjaIBam2R1xxDac1jSd001
+4X/XcDORxRpJl+0gONw7Ws2nuggeBGlCsy2Fo9/mngEG3bwa7qSmUM9T1Cp+1+vg
+Rmi7ox7Yb4m2KaTXoD76mydcQW7+fQkCvpUVC8AtOTWMOfrCMQIDAQABo1MwUTAd
+BgNVHQ4EFgQUHvG7Xchqzwdggky+oyzlpNem8UowHwYDVR0jBBgwFoAUHvG7Xchq
+zwdggky+oyzlpNem8UowDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC
+AQEAUn1ncSqeXbQAHNrVOFldLwu70hNlMxf2z4EfH2M7vJgrpwkRuIFw7PXNITBh
+CImd/ghm5NGFysrK7BwdHkFvUXZV3rE93BhcLC9leWfky33kW9olIzpE14i5FfBn
+ABmaokPhOrzAneGzU35sZHNotlqOrzgpKVkpOWrykhYZ5Qjk8Sz0xvzuG8TJc20s
+2og+W8Rm2u/xI9xPxtFbq9vUjvFS35o1pm+vkzpgNdo4YS1PG37BW/aopsooLSk7
+9Rxv5vzNXtQqeZ5qBdKbAVh3OsgqwigTmXVvOX3xpy9r9qiimhaISxCt83RZ7wQW
+I19t9pXyxAi6u7MRhJZlAeH/3w==
+-END CERTIFICATE-
diff --git a/board/sandbox/SIGNER.esl b/board/sandbox/SIGNER.esl
new file mode 100644
index
..f8cc272309b2f80113c29e22bc9fdd5c767b4667
GIT binary patch
literal 831
zcmZ1^?2Da*aux2_hA(f&|m&&@%1|1@gOCPI%=`vTjNcb9GchtTi3D3+YdNud
z!N;6d=3f<$i4vT_x7NA^t?#f
z>a)U0PLquF6_u8?^dHul+F@6qxB0YdssF`W?=n<3b^P4dmiKI#^@p}E)#B;%RW0;Z
z-#n?@Et9eDfUQTgV*{b|~VRC6NVv@WS%&0hbnAnbMH)s>mjlOWk4kw?f2&||$2#il@?9KqESXN5
zbz2wTe>RVi?d~3_cT1K9oDaUDRd@aM1GkLbi{)Ma(#i_ui5G`j(PuTIhpoN
z73DslYiZhJ`RkA&6EhzH
z-bULy5-~dZsg>zZPS-w(zNM;c<#N4ar|5@t2FY2AoF7{4IWYI(=HR-Vl;VtSQGM$z
zG|ez&@#LeqJ9NJDsL
xX})Fc$L0Fj-|CD!3Bu=aCF
[PATCH v7 02/11] nuvoton: npcm845-evb: Add a newline at the end of file
Add a newline at the end of the dts, without which the build fails when including the u-boot.dtsi file. Signed-off-by: Sughosh Ganu Reviewed-by: Simon Glass Reviewed-by: Ilias Apalodimas --- Changes since V6: None arch/arm/dts/nuvoton-npcm845-evb.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/dts/nuvoton-npcm845-evb.dts b/arch/arm/dts/nuvoton-npcm845-evb.dts index 3cab7807e3..a93666cb41 100644 --- a/arch/arm/dts/nuvoton-npcm845-evb.dts +++ b/arch/arm/dts/nuvoton-npcm845-evb.dts @@ -354,4 +354,4 @@ _pins _pins >; -}; \ No newline at end of file +}; -- 2.34.1
[PATCH v7 01/11] binman: bintool: Build a tool from a list of commands
Add support to build a tool from source with a list of commands. This
is useful when a tool can be built with multiple commands instead of a
single command.
Signed-off-by: Sughosh Ganu
Reviewed-by: Simon Glass
---
Changes since V6: None
tools/binman/bintool.py | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py
index 0b0f56dbbb..3c4ad1adbb 100644
--- a/tools/binman/bintool.py
+++ b/tools/binman/bintool.py
@@ -328,7 +328,7 @@ class Bintool:
return result.stdout
@classmethod
-def build_from_git(cls, git_repo, make_target, bintool_path, flags=None):
+def build_from_git(cls, git_repo, make_targets, bintool_path, flags=None):
"""Build a bintool from a git repo
This clones the repo in a temporary directory, builds it with 'make',
@@ -336,7 +336,8 @@ class Bintool:
Args:
git_repo (str): URL of git repo
-make_target (str): Target to pass to 'make' to build the tool
+make_targets (list of str): List of targets to pass to 'make' to
build
+the tool
bintool_path (str): Relative path of the tool in the repo, after
build is complete
flags (list of str): Flags or variables to pass to make, or None
@@ -350,12 +351,14 @@ class Bintool:
tmpdir = tempfile.mkdtemp(prefix='binmanf.')
print(f"- clone git repo '{git_repo}' to '{tmpdir}'")
tools.run('git', 'clone', '--depth', '1', git_repo, tmpdir)
-print(f"- build target '{make_target}'")
-cmd = ['make', '-C', tmpdir, '-j', f'{multiprocessing.cpu_count()}',
- make_target]
-if flags:
-cmd += flags
-tools.run(*cmd)
+for target in make_targets:
+print(f"- build target '{target}'")
+cmd = ['make', '-C', tmpdir, '-j',
f'{multiprocessing.cpu_count()}',
+ target]
+if flags:
+cmd += flags
+tools.run(*cmd)
+
fname = os.path.join(tmpdir, bintool_path)
if not os.path.exists(fname):
print(f"- File '{fname}' was not produced")
--
2.34.1
[PATCH v7 00/11] Integrate EFI capsule tasks into u-boot's build flow
This patchset aims to bring two capsule related tasks under the u-boot build flow. One is the embedding of the public key into the platform's dtb. The public key is in the form of an EFI Signature List(ESL) file and is used for capsule authentication. This is being achieved by adding the signature node containing the capsule public key in the architecture's u-boot.dtsi file. Currently, the u-boot.dtsi file has been added for the sandbox and arm architectures. The path to the ESL file is being provided through a Kconfig symbol(CONFIG_EFI_CAPSULE_ESL_FILE). Changes have also been made to the test flow so that the keys used for signing the capsule, and the ESL file, are generated prior to invoking the u-boot's build, which enables embedding the ESL file into the dtb as part of the u-boot build. The other task is related to generation of capsules. The capsules can be generated as part of u-boot build, and this is being achieved through binman, by adding a capsule entry type. The capsules can be generated by specifying the capsule parameters as properties under the capsule entry node. Changes have also been made to the efi capsule update feature testing setup on the sandbox variants. Currently, the capsule files and the public key ESL file are generated after u-boot has been built. This logic has been changed so that the capsule input files along with the keys needed for capsule signing and authentication are generated prior to initiation of the u-boot build. The placement of all the keys and public key certificates needed for signing and authenticating capsules is under the board/sandbox/ directory. The other input files needed for testing the EFI capsule update feature are being generated through binman for the sandbox platform. The document has been updated to reflect the above changes. Changes since V6: After discussing with Simon and Tom over irc, this series puts only the keys and cert files under the board/sandbox/ directory. The other set of capsule input files needed for capsule update feature testing are being generated through binman. The other patch specific changes are as under. * New patch that puts the keys and cert files under board/sandbox/ directory as suggested Simon Glass. * Populate the CONFIG_EFI_CAPSULE_ESL_FILE symbol for sandbox and sandbox_flattree which enable capsule authentication. * New patch which has been split up from the binman capsule entry support patch from earlier version, as suggested by Simon Glass. * Enables mkeficapsule tool for all sandbox variants, instead of only for sandbox_spl variant. * Split the changes for mkeficapsule btool into a separate patch, as suggested by Simon Glass. * Use the word commandline consistently, as suggested by Simon Glass. * Add macros for the GUID strings in sandbox_efi_capsule.h * Highlight that the private and public keys are mandatory for capsule signing. * Add a URL link to the UEFI spec, as used in the rst files. * Use local vars for private and public keys in BuildSectionData() * Use local vars for input payload and capsule filenames in BuildSectionData(). * Drop the ProcessContents() and SetImagePos() as the superclass functions suffice. * Use GUID macro names in the capsule test dts files. * Rename efi_capsule_payload.bin to capsule_input.bin. * Use macros defined in sandbox_efi_capsule for GUIDs and capsule input filenames. * Generate the capsule input files through binman text entries. * New patch for fixing CI trace test failure. Sughosh Ganu (11): binman: bintool: Build a tool from a list of commands nuvoton: npcm845-evb: Add a newline at the end of file sandbox: capsule: Add keys and certificates needed for capsule update testing capsule: authenticate: Add capsule public key in platform's dtb doc: capsule: Document the new mechanism to embed ESL file into dtb sandbox: Build the mkeficapsule tool for the sandbox variants btool: mkeficapsule: Add a bintool for EFI capsule generation binman: capsule: Add support for generating EFI capsules sandbox: capsule: Generate capsule related files through binman doc: Add documentation to highlight capsule generation related updates sandbox: trace: Increase trace buffer size .azure-pipelines.yml | 2 +- .gitlab-ci.yml| 2 +- arch/arm/dts/nuvoton-npcm845-evb.dts | 2 +- arch/arm/dts/u-boot.dtsi | 14 + arch/sandbox/dts/u-boot.dtsi | 380 ++ board/sandbox/SIGNER.crt | 19 + board/sandbox/SIGNER.esl | Bin 0 -> 831 bytes board/sandbox/SIGNER.key | 28 ++ board/sandbox/SIGNER2.crt | 19 + board/sandbox/SIGNER2.key | 28 ++ configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig| 1 + doc/develop/uefi/uefi.rst | 40 +-
Re: [PATCH 3/5] clk: rockchip: rk3568: Include UART clocks in SPL
On 2023-08-04 12:05, Eugen Hristev wrote:
> On 8/4/23 12:33, Jonas Karlman wrote:
>> The clock driver for RK3568 does not include support for UART clocks in
>> SPL. This result in the following message with high enough loglevel.
>>
>>ns16550_serial serial@fe66: pinctrl_select_state_full:
>> uclass_get_device_by_phandle_id: err=-19
The change to include UART clocks in SPL does not fix above warning
message. Instead the patch at [1] fixes this warning.
This change is however fixing that the ns16550_serial can get the UART
clock rate from clock instead of falling back on clock-frequency
property in DT. Will re-spin with an updated commit message.
[1]
https://patchwork.ozlabs.org/project/uboot/patch/20230805111041.750550-1-jo...@kwiboo.se/
Regards,
Jonas
>>
>> Fix this by including support for UART clocks in SPL.
>>
>> Fixes: 4a262feba3a5 ("rockchip: rk3568: add clock driver")
>> Signed-off-by: Jonas Karlman
>> ---
>
> Something like this it's also done for the 3588, and I still have not
> figured out why. When the rk3588 clock driver was added, I commented on
> this, but the result was just that the 'clock not found' message was
> removed.
>
> I am happy that you found some time to do these changes
>
> Reviewed-by: Eugen Hristev
>
[PATCH] pinctrl: Check pinconfig nodes pre-reloc status recursively
Pinconfig nodes normally bind recursively with PINCTRL_FULL and
PINCONF_RECURSIVE enabled. However, during U-Boot proper pre-relocation
any node marked with e.g. bootph-all will not bind unless its parent is
also marked for pre-reloc.
group1 {
pinconf1 {
bootph-all;
};
};
This cause the following warning message to be shown during U-Boot
proper pre-reloc stage on Rockchip RK3568 devices.
ns16550_serial serial@fe66: pinctrl_select_state_full:
uclass_get_device_by_phandle_id: err=-19
Check pinconfig nodes pre-reloc status recursively to fix this and to
make pinconfig_post_bind work same at both U-Boot proper pre-reloc and
at TPL/SPL stage.
Signed-off-by: Jonas Karlman
---
drivers/pinctrl/pinctrl-uclass.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c
index 73dd7b1038bb..fe2ba5021a78 100644
--- a/drivers/pinctrl/pinctrl-uclass.c
+++ b/drivers/pinctrl/pinctrl-uclass.c
@@ -100,6 +100,22 @@ static int pinctrl_select_state_full(struct udevice *dev,
const char *statename)
return 0;
}
+static bool ofnode_pre_reloc_recursive(ofnode parent)
+{
+ ofnode child;
+
+ if (ofnode_pre_reloc(parent))
+ return true;
+
+ if (CONFIG_IS_ENABLED(PINCONF_RECURSIVE)) {
+ ofnode_for_each_subnode(child, parent)
+ if (ofnode_pre_reloc_recursive(child))
+ return true;
+ }
+
+ return false;
+}
+
/**
* pinconfig_post_bind() - post binding for PINCONFIG uclass
* Recursively bind its children as pinconfig devices.
@@ -119,7 +135,7 @@ static int pinconfig_post_bind(struct udevice *dev)
dev_for_each_subnode(node, dev) {
if (pre_reloc_only &&
- !ofnode_pre_reloc(node))
+ !ofnode_pre_reloc_recursive(node))
continue;
/*
* If this node has "compatible" property, this is not
--
2.41.0
[PATCH] net: phy: motorcomm: Add support for YT8511 PHY
The YT8511 ethernet PHYs can be found on e.g. the SOQuartz or
the Quartz64. Add rudimentary support for them.
Signed-off-by: Nicolas Frattaroli
---
drivers/net/phy/Kconfig | 2 +-
drivers/net/phy/motorcomm.c | 88 +
2 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 0c3c39a550..3d96938eab 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -224,7 +224,7 @@ config PHY_MOTORCOMM
tristate "Motorcomm PHYs"
help
Enables support for Motorcomm network PHYs.
- Currently supports the YT8531 Gigabit Ethernet PHYs.
+ Currently supports the YT8511 and YT8531 Gigabit Ethernet PHYs.
config PHY_MSCC
bool "Microsemi Corp Ethernet PHYs support"
diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index e822fd76f2..8635a960d6 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -11,6 +11,7 @@
#include
#include
+#define PHY_ID_YT8511 0x010a
#define PHY_ID_YT8531 0x4f51e91b
#define PHY_ID_MASKGENMASK(31, 0)
@@ -26,6 +27,31 @@
#define YTPHY_DTS_OUTPUT_CLK_25M 2500
#define YTPHY_DTS_OUTPUT_CLK_125M 12500
+#define YT8511_EXT_CLK_GATE0x0c
+#define YT8511_EXT_DELAY_DRIVE 0x0d
+#define YT8511_EXT_SLEEP_CTRL 0x27
+
+/* 2b00 25m from pll
+ * 2b01 25m from xtl *default*
+ * 2b10 62.m from pll
+ * 2b11 125m from pll
+ */
+#define YT8511_CLK_125M(BIT(2) | BIT(1))
+#define YT8511_PLLON_SLP BIT(14)
+
+/* RX Delay enabled = 1.8ns 1000T, 8ns 10/100T */
+#define YT8511_DELAY_RXBIT(0)
+
+/* TX Gig-E Delay is bits 7:4, default 0x5
+ * TX Fast-E Delay is bits 15:12, default 0xf
+ * Delay = 150ps * N - 250ps
+ * On = 2000ps, off = 50ps
+ */
+#define YT8511_DELAY_GE_TX_EN (0xf << 4)
+#define YT8511_DELAY_GE_TX_DIS (0x2 << 4)
+#define YT8511_DELAY_FE_TX_EN (0xf << 12)
+#define YT8511_DELAY_FE_TX_DIS (0x2 << 12)
+
#define YT8531_SCR_SYNCE_ENABLEBIT(6)
/* 1b0 output 25m clock *default*
* 1b1 output 125m clock
@@ -347,6 +373,58 @@ static void ytphy_dt_parse(struct phy_device *phydev)
priv->flag |= TX_CLK_1000_INVERTED;
}
+static int yt8511_config(struct phy_device *phydev)
+{
+ u32 ge, fe;
+ int ret;
+
+ ret = genphy_config_aneg(phydev);
+ if (ret < 0)
+ return ret;
+
+ switch (phydev->interface) {
+ case PHY_INTERFACE_MODE_RGMII:
+ ge = YT8511_DELAY_GE_TX_DIS;
+ fe = YT8511_DELAY_FE_TX_DIS;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ ge = YT8511_DELAY_RX | YT8511_DELAY_GE_TX_DIS;
+ fe = YT8511_DELAY_FE_TX_DIS;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ ge = YT8511_DELAY_GE_TX_EN;
+ fe = YT8511_DELAY_FE_TX_EN;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ ge = YT8511_DELAY_RX | YT8511_DELAY_GE_TX_EN;
+ fe = YT8511_DELAY_FE_TX_EN;
+ break;
+ default: /* do not support other modes */
+ return -EOPNOTSUPP;
+ }
+
+ ret = ytphy_modify_ext(phydev, YT8511_EXT_CLK_GATE,
+ (YT8511_DELAY_RX | YT8511_DELAY_GE_TX_EN), ge);
+ if (ret < 0)
+ return ret;
+ /* set clock mode to 125m */
+ ret = ytphy_modify_ext(phydev, YT8511_EXT_CLK_GATE,
+ YT8511_CLK_125M, YT8511_CLK_125M);
+ if (ret < 0)
+ return ret;
+ ret = ytphy_modify_ext(phydev, YT8511_EXT_DELAY_DRIVE,
+ YT8511_DELAY_FE_TX_EN, fe);
+ if (ret < 0)
+ return ret;
+ /* sleep control, disable PLL in sleep for now */
+ ret = ytphy_modify_ext(phydev, YT8511_EXT_SLEEP_CTRL, YT8511_PLLON_SLP,
+ 0);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
static int yt8531_config(struct phy_device *phydev)
{
struct ytphy_plat_priv *priv = phydev->priv;
@@ -425,6 +503,16 @@ static int yt8531_probe(struct phy_device *phydev)
return 0;
}
+U_BOOT_PHY_DRIVER(motorcomm8511) = {
+ .name = "YT8511 Gigabit Ethernet",
+ .uid = PHY_ID_YT8511,
+ .mask = PHY_ID_MASK,
+ .features = PHY_GBIT_FEATURES,
+ .config= _config,
+ .startup = _startup,
+ .shutdown = _shutdown,
+};
+
U_BOOT_PHY_DRIVER(motorcomm8531) = {
.name = "YT8531 Gigabit Ethernet",
.uid = PHY_ID_YT8531,
--
2.41.0
Re: [PATCH] net: phy: broadcom: add support for BCM54210E
On 5.08.2023 05:38, Marek Vasut wrote:
It's Broadcom PHY simply described as single-port
RGMII 10/100/1000BASE-T PHY. It requires disabling
delay skew and GTXCLK bits.
Ported from Linux kernel commit
0fc9ae1076697 ("net: phy: broadcom: add support for BCM54210E")
Signed-off-by: Marek Vasut
---
Cc: Joe Hershberger
Cc: Marek Vasut
Cc: Michal Simek
Cc: Ramon Fried
Cc: Rasmus Villemoes
Including original author may be a good idea too :)
This patch isn't really correct, see below.
---
drivers/net/phy/broadcom.c | 66 +-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 36c70da181a..0967363b3bc 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -30,10 +30,47 @@
#define MIIM_BCM54XX_EXP_SEL_ER 0x0f00 /* Expansion register
select */
#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC 0x0007
-#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800
+#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_WIRESPEED_EN 0x0010
+#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_EN 0x0080
+#define MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN0x0100
+#define MIIM_BCM_AUXCNTL_MISC_FORCE_AMDIX 0x0200
+#define MIIM_BCM_AUXCNTL_ACTL_SMDSP_EN 0x0800
+#define MIIM_BCM_AUXCNTL_MISC_WREN 0x8000
#define MIIM_BCM_CHANNEL_WIDTH0x2000
+#define BCM54810_SHD_CLK_CTL0x3
+#define BCM54810_SHD_CLK_CTL_GTXCLK_EN BIT(9)
+
+static int bcm54xx_auxctl_read(struct phy_device *phydev, u16 regnum)
+{
+ /* The register must be written to both the Shadow Register Select and
+* the Shadow Read Register Selector
+*/
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL,
+ MIIM_BCM54xx_AUXCNTL_ENCODE(regnum));
+ return phy_read(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL);
+}
+
+static int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val)
+{
+ return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXCNTL, regnum
| val);
+}
+
+static int bcm_phy_read_shadow(struct phy_device *phydev, u16 shadow)
+{
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD,
+ MIIM_BCM54XX_SHD_VAL(shadow));
+ return MIIM_BCM54XX_SHD_DATA(phy_read(phydev, MDIO_DEVAD_NONE,
+ MIIM_BCM54XX_SHD));
+}
+
+static int bcm_phy_write_shadow(struct phy_device *phydev, u16 shadow, u16 val)
+{
+ return phy_write(phydev, MDIO_DEVAD_NONE, MIIM_BCM54XX_SHD,
+MIIM_BCM54XX_SHD_WR_ENCODE(shadow, val));
+}
+
static void bcm_phy_write_misc(struct phy_device *phydev,
u16 reg, u16 chl, u16 value)
{
@@ -62,6 +99,23 @@ static int bcm5461_config(struct phy_device *phydev)
return 0;
}
+/* Broadcom BCM54210E */
+static int bcm54210e_config(struct phy_device *phydev)
+{
+ int val;
+
+ val = bcm54xx_auxctl_read(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC);
+ val &= ~MIIM_BCM_AUXCNTL_SHDWSEL_MISC_RGMII_SKEW_EN;
+ val |= MIIM_BCM_AUXCNTL_MISC_WREN;
+ bcm54xx_auxctl_write(phydev, MIIM_BCM_AUXCNTL_SHDWSEL_MISC, val);
+
+ val = bcm_phy_read_shadow(phydev, BCM54810_SHD_CLK_CTL);
+ val &= ~BCM54810_SHD_CLK_CTL_GTXCLK_EN;
+ bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, val);
+
+ return bcm5461_config(phydev);
+}
Above function hardcodes setup for a specific PHY mode. It should be
done depending on PHY mode specified in DT. See kernel commit
fea7fda7f50a ("net: phy: broadcom: Fix RGMII delays configuration for
BCM54210E")
+
static int bcm54xx_parse_status(struct phy_device *phydev)
{
unsigned int mii_reg;
@@ -311,6 +365,16 @@ static int bcm5482_startup(struct phy_device *phydev)
return bcm54xx_parse_status(phydev);
}
+U_BOOT_PHY_DRIVER(bcm54210e) = {
+ .name = "Broadcom BCM54210E",
+ .uid = 0x600d84a0,
+ .mask = 0xfff0,
+ .features = PHY_GBIT_FEATURES,
+ .config = _config,
+ .startup = _startup,
+ .shutdown = _shutdown,
+};
+
U_BOOT_PHY_DRIVER(bcm5461s) = {
.name = "Broadcom BCM5461S",
.uid = 0x2060c0,
[PATCH v5 4/4] arm: dts: k3-am64: Sync DT with Linux v6.5-rc1
Sync all am642-evm/am642-sk related DT files
with Linux v6.5-rc1.
- drop timer1 in favor of main_timer0 in am64-main.dtsi.
Need to delete clock & power domain properties of
main_timer1 in -r5.dts else won't boot. This is because
timer_init is done during rproc_start to start System Firmware,
but we can't do any clock/power-domain operations before
System Firmware starts.
- same constraint applies to main_uart0
- drop cpsw3g custom DT property 'mac_efuse' and custom
DT node cpsw-phy-sel as driver picks these from standard
property/node.
- include board dts file in -r5 dts file to avoid duplication
of nodes. Include -u-boot.dtsi on top.
- drop duplicate nodes in -r5 dts and -u-boot.dtsi
Signed-off-by: Roger Quadros
---
arch/arm/dts/k3-am64-main.dtsi| 171 ++-
arch/arm/dts/k3-am64-mcu.dtsi | 53 +-
arch/arm/dts/k3-am64-thermal.dtsi | 33
arch/arm/dts/k3-am64.dtsi | 22 +--
arch/arm/dts/k3-am642-evm-u-boot.dtsi | 57 +++
arch/arm/dts/k3-am642-evm.dts | 173 ++-
arch/arm/dts/k3-am642-r5-evm.dts | 231 +++---
arch/arm/dts/k3-am642-r5-sk.dts | 218 +++-
arch/arm/dts/k3-am642-sk-u-boot.dtsi | 52 ++
arch/arm/dts/k3-am642-sk.dts | 166 +-
arch/arm/dts/k3-am642.dtsi| 1 +
11 files changed, 604 insertions(+), 573 deletions(-)
create mode 100644 arch/arm/dts/k3-am64-thermal.dtsi
diff --git a/arch/arm/dts/k3-am64-main.dtsi b/arch/arm/dts/k3-am64-main.dtsi
index 5e8036f32d..1664d9f024 100644
--- a/arch/arm/dts/k3-am64-main.dtsi
+++ b/arch/arm/dts/k3-am64-main.dtsi
@@ -228,12 +228,161 @@
};
};
+ main_timer0: timer@240 {
+ compatible = "ti,am654-timer";
+ reg = <0x00 0x240 0x00 0x400>;
+ interrupts = ;
+ clocks = <_clks 36 1>;
+ clock-names = "fck";
+ assigned-clocks = <_clks 36 1>;
+ assigned-clock-parents = <_clks 36 2>;
+ power-domains = <_pds 36 TI_SCI_PD_EXCLUSIVE>;
+ ti,timer-pwm;
+ };
+
+ main_timer1: timer@241 {
+ compatible = "ti,am654-timer";
+ reg = <0x00 0x241 0x00 0x400>;
+ interrupts = ;
+ clocks = <_clks 37 1>;
+ clock-names = "fck";
+ assigned-clocks = <_clks 37 1>;
+ assigned-clock-parents = <_clks 37 2>;
+ power-domains = <_pds 37 TI_SCI_PD_EXCLUSIVE>;
+ ti,timer-pwm;
+ };
+
+ main_timer2: timer@242 {
+ compatible = "ti,am654-timer";
+ reg = <0x00 0x242 0x00 0x400>;
+ interrupts = ;
+ clocks = <_clks 38 1>;
+ clock-names = "fck";
+ assigned-clocks = <_clks 38 1>;
+ assigned-clock-parents = <_clks 38 2>;
+ power-domains = <_pds 38 TI_SCI_PD_EXCLUSIVE>;
+ ti,timer-pwm;
+ };
+
+ main_timer3: timer@243 {
+ compatible = "ti,am654-timer";
+ reg = <0x00 0x243 0x00 0x400>;
+ interrupts = ;
+ clocks = <_clks 39 1>;
+ clock-names = "fck";
+ assigned-clocks = <_clks 39 1>;
+ assigned-clock-parents = <_clks 39 2>;
+ power-domains = <_pds 39 TI_SCI_PD_EXCLUSIVE>;
+ ti,timer-pwm;
+ };
+
+ main_timer4: timer@244 {
+ compatible = "ti,am654-timer";
+ reg = <0x00 0x244 0x00 0x400>;
+ interrupts = ;
+ clocks = <_clks 40 1>;
+ clock-names = "fck";
+ assigned-clocks = <_clks 40 1>;
+ assigned-clock-parents = <_clks 40 2>;
+ power-domains = <_pds 40 TI_SCI_PD_EXCLUSIVE>;
+ ti,timer-pwm;
+ };
+
+ main_timer5: timer@245 {
+ compatible = "ti,am654-timer";
+ reg = <0x00 0x245 0x00 0x400>;
+ interrupts = ;
+ clocks = <_clks 41 1>;
+ clock-names = "fck";
+ assigned-clocks = <_clks 41 1>;
+ assigned-clock-parents = <_clks 41 2>;
+ power-domains = <_pds 41 TI_SCI_PD_EXCLUSIVE>;
+ ti,timer-pwm;
+ };
+
+ main_timer6: timer@246 {
+ compatible = "ti,am654-timer";
+ reg = <0x00 0x246 0x00 0x400>;
+ interrupts = ;
+ clocks = <_clks 42 1>;
+ clock-names = "fck";
+ assigned-clocks = <_clks 42 1>;
+ assigned-clock-parents = <_clks 42 2>;
+ power-domains = <_pds 42 TI_SCI_PD_EXCLUSIVE>;
+ ti,timer-pwm;
+ };
+
+ main_timer7: timer@247 {
+ compatible = "ti,am654-timer";
+ reg = <0x00
[PATCH v5 3/4] doc: board: ti: am64: Add boot flow diagram
Add documenatation and boot flow diagram for AM64 EVM/SoC. Suggested-by: Nishanth Menon Signed-off-by: Roger Quadros Reviewed-by: Nishanth Menon Tested-by: Nishanth Menon #SK-AM64B --- doc/board/ti/am64x_evm.rst | 197 +++ doc/board/ti/img/boot_diagram_am64.svg | 1702 doc/board/ti/k3.rst|1 + 3 files changed, 1900 insertions(+) create mode 100644 doc/board/ti/am64x_evm.rst create mode 100644 doc/board/ti/img/boot_diagram_am64.svg diff --git a/doc/board/ti/am64x_evm.rst b/doc/board/ti/am64x_evm.rst new file mode 100644 index 00..8d3795eb32 --- /dev/null +++ b/doc/board/ti/am64x_evm.rst @@ -0,0 +1,197 @@ +.. SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause +.. sectionauthor:: Nishanth Menon + +AM64 Platforms +== + +Introduction: +- +The AM642 SoC belongs to the K3 Multicore SoC architecture platform, +providing advanced system integration to enable applications such as +Motor Drives, PLC, Remote IO and IoT Gateways. + +Some highlights of this SoC are: + +* Dual Cortex-A53s in a single cluster, two clusters of dual Cortex-R5F + MCUs, and a single Cortex-M4F. +* Two Gigabit Industrial Communication Subsystems (ICSSG). +* Integrated Ethernet switch supporting up to a total of two external + ports. +* PCIe-GEN2x1L, USB3/USB2, 2xCAN-FD, eMMC and SD, UFS, OSPI memory + controller, QSPI, I2C, eCAP/eQEP, ePWM, ADC, among other + peripherals. +* Centralized System Controller for Security, Power, and Resource + Management (DMSC). + +More details can be found in the Technical Reference Manual: + https://www.ti.com/lit/pdf/spruim2 + +Platform information: + +* AM64-EVM: https://www.ti.com/tool/TMDS64EVM +* AM64-SK: https://www.ti.com/tool/SK-AM64B + +Boot Flow: +-- +Below is the pictorial representation of boot flow: + +.. image:: img/boot_diagram_am64.svg + +- Here TIFS acts as master and provides all the critical services. R5/A53 + requests TIFS to get these services done as shown in the above diagram. + +Sources: + + +.. include:: k3.rst +:start-after: .. k3_rst_include_start_boot_sources +:end-before: .. k3_rst_include_end_boot_sources + +Build procedure: + +0. Setup the environment variables: + +.. include:: k3.rst +:start-after: .. k3_rst_include_start_common_env_vars_desc +:end-before: .. k3_rst_include_end_common_env_vars_desc + +.. include:: k3.rst +:start-after: .. k3_rst_include_start_board_env_vars_desc +:end-before: .. k3_rst_include_end_board_env_vars_desc + +Set the variables corresponding to this platform: + +.. include:: k3.rst +:start-after: .. k3_rst_include_start_common_env_vars_defn +:end-before: .. k3_rst_include_end_common_env_vars_defn +.. code-block:: bash + + $ export UBOOT_CFG_CORTEXR=am64x_evm_r5_defconfig + $ export UBOOT_CFG_CORTEXA=am64x_evm_a53_defconfig + $ export TFA_BOARD=lite + $ # we dont use any extra TFA parameters + $ unset TFA_EXTRA_ARGS + $ export OPTEE_PLATFORM=k3-am64x + $ # we dont use any extra TFA parameters + $ unset OPTEE_EXTRA_ARGS + +.. am64x_evm_rst_include_start_build_steps + +1. Trusted Firmware-A: + +.. include:: k3.rst +:start-after: .. k3_rst_include_start_build_steps_tfa +:end-before: .. k3_rst_include_end_build_steps_tfa + + +2. OP-TEE: + +.. include:: k3.rst +:start-after: .. k3_rst_include_start_build_steps_optee +:end-before: .. k3_rst_include_end_build_steps_optee + +3. U-Boot: + +* 4.1 R5: + +.. include:: k3.rst +:start-after: .. k3_rst_include_start_build_steps_spl_r5 +:end-before: .. k3_rst_include_end_build_steps_spl_r5 + +* 4.2 A53: + +.. include:: k3.rst +:start-after: .. k3_rst_include_start_build_steps_uboot +:end-before: .. k3_rst_include_end_build_steps_uboot +.. am64x_evm_rst_include_end_build_steps + +Target Images +-- +In order to boot we need tiboot3.bin, tispl.bin and u-boot.img. Each SoC +variant (GP, HS-FS, HS-SE) requires a different source for these files. + + - GP + +* tiboot3-am64x-gp-evm.bin from step 3.1 +* tispl.bin_unsigned, u-boot.img_unsigned from step 3.2 + + - HS-FS + +* tiboot3-am64x-hs-fs-evm.bin from step 3.1 +* tispl.bin, u-boot.img from step 3.2 + + - HS-SE + +* tiboot3-am64x-hs-evm.bin from step 3.1 +* tispl.bin, u-boot.img from step 3.2 + +Image formats: +-- + +- tiboot3.bin + +.. image:: img/multi_cert_tiboot3.bin.svg + +- tispl.bin + +.. image:: img/nodm_tispl.bin.svg + +Switch Setting for Boot Mode + + +Boot Mode pins provide means to select the boot mode and options before the +device is powered up. After every POR, they are the main source to populate +the Boot Parameter Tables. + +The following table shows some common boot modes used on AM64 platform. More +details can be found in the Technical Reference Manual: +https://www.ti.com/lit/pdf/spruim2 under the `Boot Mode Pins` section. + +.. list-table::
[PATCH v5 2/4] Revert "ARM: dts: k3-am642-sk-u-boot: add PMIC node"
This reverts commit 28a4c3113445d4400639f357fae0def007a41093.
This node should be in the board DT file and should come from upstream.
Moreover, this PMIC is no present on all variants of am642-sk
and will need a separate board DT file.
Signed-off-by: Roger Quadros
Reviewed-by: Nishanth Menon
Tested-by: Nishanth Menon #SK-AM64B
---
arch/arm/dts/k3-am642-sk-u-boot.dtsi | 61
1 file changed, 61 deletions(-)
diff --git a/arch/arm/dts/k3-am642-sk-u-boot.dtsi
b/arch/arm/dts/k3-am642-sk-u-boot.dtsi
index 3d6be025bd..4431750dc6 100644
--- a/arch/arm/dts/k3-am642-sk-u-boot.dtsi
+++ b/arch/arm/dts/k3-am642-sk-u-boot.dtsi
@@ -54,67 +54,6 @@
pinctrl-names = "default";
pinctrl-0 = <_i2c0_pins_default>;
clock-frequency = <40>;
-
- tps65219: pmic@30 {
- compatible = "ti,tps65219";
- reg = <0x30>;
-
- regulators {
- buck1_reg: buck1 {
- regulator-name = "VDD_CORE";
- regulator-min-microvolt = <75>;
- regulator-max-microvolt = <75>;
- regulator-boot-on;
- regulator-always-on;
- };
-
- buck2_reg: buck2 {
- regulator-name = "VCC1V8";
- regulator-min-microvolt = <180>;
- regulator-max-microvolt = <180>;
- regulator-boot-on;
- regulator-always-on;
- };
-
- buck3_reg: buck3 {
- regulator-name = "VDD_LPDDR4";
- regulator-min-microvolt = <110>;
- regulator-max-microvolt = <110>;
- regulator-boot-on;
- regulator-always-on;
- };
-
- ldo1_reg: ldo1 {
- regulator-name = "VDDSHV_SD_IO_PMIC";
- regulator-min-microvolt = <3300>;
- regulator-max-microvolt = <3300>;
- };
-
- ldo2_reg: ldo2 {
- regulator-name = "VDDAR_CORE";
- regulator-min-microvolt = <85>;
- regulator-max-microvolt = <85>;
- regulator-boot-on;
- regulator-always-on;
- };
-
- ldo3_reg: ldo3 {
- regulator-name = "VDDA_1V8";
- regulator-min-microvolt = <1800>;
- regulator-max-microvolt = <1800>;
- regulator-boot-on;
- regulator-always-on;
- };
-
- ldo4_reg: ldo4 {
- regulator-name = "VDD_PHY_2V5";
- regulator-min-microvolt = <2500>;
- regulator-max-microvolt = <2500>;
- regulator-boot-on;
- regulator-always-on;
- };
- };
- };
};
_uart0 {
--
2.34.1
[PATCH v5 1/4] board: ti: am64x: Recognize AM64-HSEVM
AM64-HSEVM is AM64-GPEVM with High Security Device.
Gets rid of "Unidentified board claims AM64-HSEVM in eeprom header".
Signed-off-by: Roger Quadros
Acked-by: Andrew Davis
Reviewed-by: Nishanth Menon
Tested-by: Nishanth Menon #SK-AM64B
---
board/ti/am64x/evm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/ti/am64x/evm.c b/board/ti/am64x/evm.c
index 96f4e3013a..a080b2b0d2 100644
--- a/board/ti/am64x/evm.c
+++ b/board/ti/am64x/evm.c
@@ -18,7 +18,8 @@
#include "../common/board_detect.h"
-#define board_is_am64x_gpevm() board_ti_k3_is("AM64-GPEVM")
+#define board_is_am64x_gpevm() (board_ti_k3_is("AM64-GPEVM") || \
+ board_ti_k3_is("AM64-HSEVM"))
#define board_is_am64x_skevm() (board_ti_k3_is("AM64-SKEVM") || \
board_ti_k3_is("AM64B-SKEVM"))
--
2.34.1
[PATCH v5 0/4] arm: dts: k3-am64: Sync DT with Linux
Hi, This series syncs AM64 DT files from Linux v6.5-rc1. Tested on AM642-EVM GP SR1.0 and AM642-SK-EVM HS-FS SR2.0. cheers, -roger Changelog: v5: -squash documenation change to correct patch. Update commit log. -add Nishanth's Reviewed/Tested by v4: -Add documentation for am642-evm and am642-sk-evm. -Add license to am64 svg file -drop mmc0 pinmux from k3-am642-r5-evm.dts -drop stdout-path from -r5 & -u-boot dts. -drop vtt-supply and vtt pinmux from memorycontroller in k3-am642-r5-evm.dts -am642-evm: move bootph-pre-ram for main_esm, mcu_esm into -r5 .dts v3: - include board DT file in -r5 DT file. - move including -u-boot.dtsi file at the top of -r5 DT file. - drop duplicate nodes - document why we need to delete clock/power properties from main_timer0 v2: - Sync with v6.5-rc1 - Rebase on latest uboot/master - CPSW node cleanup - Timer node cleanup Roger Quadros (4): board: ti: am64x: Recognize AM64-HSEVM Revert "ARM: dts: k3-am642-sk-u-boot: add PMIC node" doc: board: ti: am64: Add boot flow diagram arm: dts: k3-am64: Sync DT with Linux v6.5-rc1 arch/arm/dts/k3-am64-main.dtsi | 171 ++- arch/arm/dts/k3-am64-mcu.dtsi | 53 +- arch/arm/dts/k3-am64-thermal.dtsi | 33 + arch/arm/dts/k3-am64.dtsi | 22 +- arch/arm/dts/k3-am642-evm-u-boot.dtsi | 57 +- arch/arm/dts/k3-am642-evm.dts | 173 ++- arch/arm/dts/k3-am642-r5-evm.dts | 231 +--- arch/arm/dts/k3-am642-r5-sk.dts| 218 +-- arch/arm/dts/k3-am642-sk-u-boot.dtsi | 113 +- arch/arm/dts/k3-am642-sk.dts | 166 ++- arch/arm/dts/k3-am642.dtsi |1 + board/ti/am64x/evm.c |3 +- doc/board/ti/am64x_evm.rst | 197 +++ doc/board/ti/img/boot_diagram_am64.svg | 1702 doc/board/ti/k3.rst|1 + 15 files changed, 2506 insertions(+), 635 deletions(-) create mode 100644 arch/arm/dts/k3-am64-thermal.dtsi create mode 100644 doc/board/ti/am64x_evm.rst create mode 100644 doc/board/ti/img/boot_diagram_am64.svg -- 2.34.1
