[PATCH] clk: sunxi: Add an A31/A23/A33 legacy PRCM MFD driver
When the CCU binding and driver for the PRCM were written, it seems the
intention was to convert the A31 and A23/A33 devicetrees to use them.
However, that never happened, so those SoCs still use the old binding,
with an MFD for the PRCM, and separate DT nodes for clocks and resets.
The specifier in the legacy clock/reset bindings is the register bit
offset, so the drivers are trivial. Only the outer PRCM node has a reg
property, so the clock/reset drivers use the parent device's MMIO base.
Signed-off-by: Samuel Holland
---
I didn't reuse the sunxi gate/reset ops, because the driver is actually
smaller without them. I tested this driver on an A33 tablet.
drivers/clk/sunxi/Kconfig | 13 +++-
drivers/clk/sunxi/Makefile | 1 +
drivers/clk/sunxi/clk_sun6i_prcm.c | 104 +
3 files changed, 116 insertions(+), 2 deletions(-)
create mode 100644 drivers/clk/sunxi/clk_sun6i_prcm.c
diff --git a/drivers/clk/sunxi/Kconfig b/drivers/clk/sunxi/Kconfig
index bf11fad6eef..759ef410b66 100644
--- a/drivers/clk/sunxi/Kconfig
+++ b/drivers/clk/sunxi/Kconfig
@@ -39,11 +39,20 @@ config CLK_SUN6I_A31
on Allwinner A31/A31s SoC.
config CLK_SUN6I_A31_R
- bool "Clock driver for Allwinner A31 generation PRCM"
+ bool "Clock driver for Allwinner A31 generation PRCM (CCU)"
default SUNXI_GEN_SUN6I
help
This enables common clock driver support for the PRCM
- in Allwinner A31/A31s/A23/A33/A83T/H3/A64/H5 SoCs.
+ in Allwinner A31/A31s/A23/A33/A83T/H3/A64/H5 SoCs using
+ the CCU binding.
+
+config CLK_SUN6I_PRCM
+ bool "Clock driver for Allwinner A31 generation PRCM (legacy)"
+ default MACH_SUN6I || MACH_SUN8I_A23 || MACH_SUN8I_A33
+ help
+ This enables common clock driver support for the PRCM
+ in Allwinner A31/A31s/A23/A33 SoCs using the legacy PRCM
+ MFD binding.
config CLK_SUN8I_A23
bool "Clock driver for Allwinner A23/A33"
diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 895da02ebea..3266409cc7a 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_CLK_SUN4I_A10) += clk_a10.o
obj-$(CONFIG_CLK_SUN5I_A10S) += clk_a10s.o
obj-$(CONFIG_CLK_SUN6I_A31) += clk_a31.o
obj-$(CONFIG_CLK_SUN6I_A31_R) += clk_a31_r.o
+obj-$(CONFIG_CLK_SUN6I_PRCM) += clk_sun6i_prcm.o
obj-$(CONFIG_CLK_SUN8I_A23) += clk_a23.o
obj-$(CONFIG_CLK_SUN8I_A83T) += clk_a83t.o
obj-$(CONFIG_CLK_SUN8I_R40) += clk_r40.o
diff --git a/drivers/clk/sunxi/clk_sun6i_prcm.c
b/drivers/clk/sunxi/clk_sun6i_prcm.c
new file mode 100644
index 000..488b47e77a7
--- /dev/null
+++ b/drivers/clk/sunxi/clk_sun6i_prcm.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2022 Samuel Holland
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+struct sun6i_prcm_plat {
+ void *base;
+};
+
+static int sun6i_prcm_set_field(struct udevice *dev, uint reg,
+ u32 mask, bool set)
+{
+ struct sun6i_prcm_plat *plat = dev_get_plat(dev->parent);
+
+ clrsetbits_le32(plat->base + reg, mask, set ? mask : 0);
+
+ return 0;
+}
+
+static int sun6i_prcm_clk_enable(struct clk *clk)
+{
+ return sun6i_prcm_set_field(clk->dev, 0x28, BIT(clk->id), true);
+}
+
+static int sun6i_prcm_clk_disable(struct clk *clk)
+{
+ return sun6i_prcm_set_field(clk->dev, 0x28, BIT(clk->id), false);
+}
+
+static const struct clk_ops sun6i_prcm_clk_ops = {
+ .enable = sun6i_prcm_clk_enable,
+ .disable= sun6i_prcm_clk_disable,
+};
+
+static const struct udevice_id sun6i_prcm_clk_ids[] = {
+ { .compatible = "allwinner,sun6i-a31-apb0-gates-clk" },
+ { .compatible = "allwinner,sun8i-a23-apb0-gates-clk" },
+ { }
+};
+
+U_BOOT_DRIVER(sun6i_prcm_clk) = {
+ .name = "sun6i_prcm_clk",
+ .id = UCLASS_CLK,
+ .of_match = sun6i_prcm_clk_ids,
+ .ops= _prcm_clk_ops,
+};
+
+static int sun6i_prcm_reset_assert(struct reset_ctl *reset)
+{
+ return sun6i_prcm_set_field(reset->dev, 0xb0, BIT(reset->id), false);
+}
+
+static int sun6i_prcm_reset_deassert(struct reset_ctl *reset)
+{
+ return sun6i_prcm_set_field(reset->dev, 0xb0, BIT(reset->id), true);
+}
+
+static const struct reset_ops sun6i_prcm_reset_ops = {
+ .rst_assert = sun6i_prcm_reset_assert,
+ .rst_deassert = sun6i_prcm_reset_deassert,
+};
+
+static const struct udevice_id sun6i_prcm_reset_ids[] = {
+ { .compatible = "allwinner,sun6i-a31-clock-reset" },
+ { }
+};
+
+U_BOOT_DRIVER(sun6i_prcm_reset) = {
+ .name = "sun6i_prcm_reset",
+ .id = UCLASS_RESET,
+ .of_match = sun6i_prcm_reset_ids,
+ .ops= _prcm_reset_ops,
+};
+
+static int sun6i_prcm_of_to_plat(struct udevice *dev)
+{
+ struct sun6i_prcm_plat *plat =
Re: Boot Loader Specification (UAPI Group)
On Wed, Nov 16, 2022 at 10:50:57AM +, Sebastian Veit wrote: > Dear devs of U-Boot, > > I'd like to know if there's already a work in progress > on the support of the "fairly new" Boot Loader Specification [1]. No, as I don't think anyone here had heard of that project until now. Hopefully if there's interest, that group will reach out or go and work with the other existing groups that have already defined specifications in that area. -- Tom signature.asc Description: PGP signature
Re: Boot Loader Specification (UAPI Group)
> Date: Wed, 16 Nov 2022 10:50:57 + > From: Sebastian Veit > > Dear devs of U-Boot, > > I'd like to know if there's already a work in progress > on the support of the "fairly new" Boot Loader Specification [1]. > > Summary of the new spec: Defines a set of file formats and > naming conventions to allow distribution independent > boot loader menus supportable by multiple bootloaders. > > Don't confuse this specification with the one you've already > implemented for syslinux [2]. It seems like the similar name > confused people in the past [3]. > > Maybe it'll be part of the new bootflow design - that has > been introduced in v2022.07 [4] - some time in the future? > > > Thanks in advance, > Sebastian Veit https://xkcd.com/927/ (sorry, couldn't resist)
Re: Boot Loader Specification (UAPI Group)
On 11/16/22 11:50, Sebastian Veit wrote: Dear devs of U-Boot, I'd like to know if there's already a work in progress on the support of the "fairly new" Boot Loader Specification [1]. Summary of the new spec: Defines a set of file formats and naming conventions to allow distribution independent boot loader menus supportable by multiple bootloaders. Hello Sebastian, Thanks for the pointers. Unfortunately it is unclear where the specification should be used. What is the spec good for on EFI systems where BootOrder and Boot variables define what is to be booted? Do you want to replace extlinux/ ? Do you expect GRUB to consider the entries when building grub.cfg? How will the entries be created? Hopefully there is some code to do the job. Best regards Heinrich Don't confuse this specification with the one you've already implemented for syslinux [2]. It seems like the similar name confused people in the past [3]. Maybe it'll be part of the new bootflow design - that has been introduced in v2022.07 [4] - some time in the future? Thanks in advance, Sebastian Veit [1] https://uapi-group.org/specifications/specs/boot_loader_specification/ [2] https://wiki.syslinux.org/wiki/index.php?title=EXTLINUX [3] https://lists.denx.de/pipermail/u-boot/2018-October/344721.html [4] https://source.denx.de/u-boot/u-boot/-/issues/1
Re: problem with reading the NV memory index of TPM slb9670
+Ilias Apalodimas On Tue, 15 Nov 2022 at 23:12, Tasos Terzidis wrote: > > Hello, > > I tried to use a tpm2 command for reading the NV memory index of an slb9670 > TPM. > > I noticed in file cmd/tpm-v2.c, that the command tpm2 nv_read_value (as tpm > nv_read_value) > > is not implemented. In file lib/tpm-v2.c there is the tpm2_nv_read_value > function. > > Do you plan to implement the command tpm2 nv_read_value in the future ? > > > > Then I used the > > tpm nv_read_value index addr count > > "Read bytes from space to memory address ". > > > > The syntax I used is > > tpm nv_read_value 0x1 0x3000 20 > > I’m trying to read 32 bytes from 0x1 index nv memory of TPM > > to 0x3000 memory of u-boot > > U-boot can’t understand the command and returns the help listing all the > commands for tpm. > > I also used > > tpm nv_read_value 0x0 0x3000 20 > > 0x0 as an offset to 0x1 index of NV memory (following the syntax of > tpm2_nvread command o TSS tpm –tools) > > > Same result, list of commands for tpm > > > > Other commands of tpm such as tpm2 pcr read or extend functions as expected > in u-boot environment > > > > I can’t understand what I do wrong > > > > Thank you > > Tasos Terzids
Re: [PATCH v3 3/4] arm_ffa: introduce Arm FF-A low-level driver
Hi,
On Wed, 16 Nov 2022 at 06:03, Abdellatif El Khlifi
wrote:
>
> On Tue, Nov 15, 2022 at 08:24:24AM -0700, Simon Glass wrote:
> > Hi,
> >
> > On Mon, 1 Aug 2022 at 11:21, Abdellatif El Khlifi
> > wrote:
> > >
> > > Add the driver implementing Arm Firmware Framework for Armv8-A v1.0
> > >
> > > The Firmware Framework for Arm A-profile processors (FF-A)
> > > describes interfaces (ABIs) that standardize communication
> > > between the Secure World and Normal World leveraging TrustZone
> > > technology.
> > >
> > > This driver uses 64-bit registers as per SMCCCv1.2 spec and comes
> > > on top of the SMCCC layer. The driver provides the FF-A ABIs needed for
> > > querying the FF-A framework from the secure world.
> > >
> > > 32-bit version of the ABIs is supported and 64-bit version of FFA_RXTX_MAP
> > > and FFA_MSG_SEND_DIRECT_{REQ, RESP}.
> > >
> > > In u-boot FF-A design, FF-A is considered as a discoverable bus.
> > > The Secure World is considered as one entity to communicate with
> > > using the FF-A bus. FF-A communication is handled by one device and
> > > one instance (the bus). This FF-A driver takes care of all the
> > > interactions between Normal world and Secure World.
> > >
> > > The driver exports its operations to be used by upper layers.
> > >
> > > Exported operations:
> > >
> > > - partition_info_get
> > > - sync_send_receive
> > > - rxtx_unmap
> > >
> > > This implementation provides an optional feature to copy the driver data
> > > to EFI runtime area.
> > >
> > > Signed-off-by: Abdellatif El Khlifi
> > > Cc: Tom Rini
> > > Cc: Ilias Apalodimas
> > > Cc: Jens Wiklander
> > > ---
> > > MAINTAINERS|6 +
> > > common/board_r.c |7 +
> > > drivers/Kconfig|2 +
> > > drivers/Makefile |1 +
> > > drivers/arm-ffa/Kconfig| 33 +
> > > drivers/arm-ffa/Makefile |7 +
> > > drivers/arm-ffa/arm-ffa-uclass.c | 16 +
> > > drivers/arm-ffa/arm_ffa_prv.h | 219
> > > drivers/arm-ffa/core.c | 1338
> > > drivers/arm-ffa/efi_ffa_runtime_data_mgr.c | 94 ++
> > > include/arm_ffa.h | 132 ++
> > > include/dm/uclass-id.h |1 +
> > > include/uuid.h |8 +
> > > lib/efi_loader/efi_boottime.c | 17 +
> > > lib/uuid.c | 65 +
> > > 15 files changed, 1946 insertions(+)
> > > create mode 100644 drivers/arm-ffa/Kconfig
> > > create mode 100644 drivers/arm-ffa/Makefile
> > > create mode 100644 drivers/arm-ffa/arm-ffa-uclass.c
> > > create mode 100644 drivers/arm-ffa/arm_ffa_prv.h
> > > create mode 100644 drivers/arm-ffa/core.c
> > > create mode 100644 drivers/arm-ffa/.c
> > > create mode 100644 include/arm_ffa.h
> >
> > Please add something to doc/ so people know what this is.
> >
> > Since you are adding a new uclass you need a sandbox driver and tests.
> >
> > The driver appears to have no operations, but there is a bus_ops. The
> > ops should go in the driver, I suspect, and should pass the device as
> > the first arg.
> >
> > Can FFA_ERR_STAT_SUCCESS be 0 so you don't have to sprinkle the code with
> > it?
> >
> > Why is it using EFI things? Can this driver only be used with UEFI? I
> > hope not, if it is an official way of updating firmware.
> >
> > Please don't add more things to board_r.c - we are trying to remove
> > this init over time. If it is a device it should be probed as needed.
> >
> > Is there a device tree binding?
> >
> > Also should this go in drivers/misc instead of creating a whole new subdir?
>
> Hi Simon, thanks for reviewing.
>
> All the above comments have already been addressed in the new versions
> of the patchset. Please refer to the latest version v7 [1].
>
> By the way I'd like to highlight the following:
>
> - The FF-A driver documentation is at doc/arch/arm64.ffa.rst, please
> refer to it since it provides helpful details about the FF-A support
> in U-Boot
OK
> - The patchset comes with Sandbox driver and tests [2]
OK I suppose I saw that previously and forgot.
> - The driver has operations defined in struct ffa_bus_ops (include/arm_ffa.h).
> ffa_bus_ops_get() gets the ops. All these are in the driver
> (drivers/firmware/arm-ffa/core.c)
Can you please push a tree somewhere so I can look?
> - The FF-A bus has only 1 device. No multiple instances. So passing the
> device doesn't make sense in our case
It must still pass the device.
> - FFA_ERR_STAT_SUCCESS has been removed and replaced with 0
OK
> - The driver is independent from EFI and can be compiled without EFI
Oh, so what is ffa_copy_runtime_data() for?
> - FF-A bus discovery has been removed from the initcall level (board_r.c)
Good.
> Discovery is done on demand. Clients can call ffa_bus_discover() when
> they want to
Re: Boot Loader Specification (UAPI Group)
+Tom Hi Sebastian, On Wed, 16 Nov 2022 at 05:08, Sebastian Veit wrote: > > Dear devs of U-Boot, > > I'd like to know if there's already a work in progress > on the support of the "fairly new" Boot Loader Specification [1]. > > Summary of the new spec: Defines a set of file formats and > naming conventions to allow distribution independent > boot loader menus supportable by multiple bootloaders. > > Don't confuse this specification with the one you've already > implemented for syslinux [2]. It seems like the similar name > confused people in the past [3]. > > Maybe it'll be part of the new bootflow design - that has > been introduced in v2022.07 [4] - some time in the future? I don't know of any ongoing work for this. In fact I'm not sure I've heard of UAPI. It would be possible to implement this as a new bootmeth - see the boot/ directory for other ones. Migration from distro scripts is underway, with the next series likely to be sent by the end of this month. Regards, Simon > > > Thanks in advance, > Sebastian Veit > > > [1] https://uapi-group.org/specifications/specs/boot_loader_specification/ > [2] https://wiki.syslinux.org/wiki/index.php?title=EXTLINUX > [3] https://lists.denx.de/pipermail/u-boot/2018-October/344721.html > [4] https://source.denx.de/u-boot/u-boot/-/issues/1
Re: [PATCH 1/3] binman: add sign option for binman
Hi Ivan,
On Thu, 15 Sept 2022 at 13:44, Ivan Mikhaylov wrote:
>
> On Wed, 2022-09-07 at 15:10 -0600, Simon Glass wrote:
> > Hi Ivan,
> >
> > Section data comes from the BuildSectionData() method, so you could
> > try calling that.
> >
> > See also collect_contents_to_file()
> >
> > Regards,
> > Simon
>
> Simon, I've tried both these ways and they both don't work to me. What
> I've got:
>
> def SignEntries(image_fname, input_fname, privatekey_fname, algo,
> entry_paths):
> image_fname = os.path.abspath(image_fname)
> image = Image.FromFile(image_fname)
> state.PrepareFromLoadedData(image)
> image.LoadData()
>
> 1. BuildSectionData
>
> for entry_path in entry_paths:
> entry = image.FindEntryPath(entry_path)
>
> try:
> entry.BuildSectionData(True)
> except Exception as e:
> logging.error(traceback.format_exc())
>
>
> ERROR:root:AttributeError: 'NoneType' object has no attribute 'run'
>
> 2. collect_contents_to_file
>
> for entry_path in entry_paths:
> entry = image.FindEntryPath(entry_path)
>
> try:
> entry.collect_contents_to_file([entry.name], "prefix",
> 1024)
> except Exception as e:
> logging.error(traceback.format_exc())
>
> ERROR:root:AttributeError: 'str' object has no attribute
> 'ObtainContents'
This seems to be getting a string instead of an entry object. Can you
try -D to see? See 'Writing new entries and debugging'.
>
> 3. GetData
>
> for entry_path in entry_paths:
> entry = image.FindEntryPath(entry_path)
>
> print("--- DATA ---")
> data = entry.GetData(True)
> print(data)
> print("~~~ DATA ~~~")
>
> --- DATA ---
> Node '/fit/images/u-boot-1/u-boot': GetData: size 0x4
>Node '/fit/images/u-boot-1': GetPaddedDataForEntry: size None
>Node '/fit/images/u-boot-1': GetData: 1 entries, total size 0x4
> Node '/fit/images/fdt-1/u-boot-spl-dtb': GetData: size 0x4f7
> Node '/fit/images/fdt-1': GetPaddedDataForEntry: size None
> Node '/fit/images/fdt-1': GetData: 1 entries, total size 0x4f7
> Deleted temporary directory '/tmp/binman.z81eqcfz'
> binman: 'NoneType' object has no attribute 'run'
This might be trying to call tools.run() so use -D to see where the error is.
>
> There is no problem with getting data from GetData around start of the
> year. Maybe some regression?
>
> All this ran with this:
> binman -v5 sign -i image.bin -k test_key.key -a sha256,rsa4096 fit
>
> `fit` in entry_paths and image contains FIT section with name `fit`.
>
> binman ls -i image.bin
> Name Image-pos Size Entry-type Offset
> Uncomp-size
> ---
>
> main-section 0 10 section 0
> fit 1 c0a fit 1
> u-boot-1 101044 section104
> u-boot 101044 u-boot 0
> fdt-1 101c8 4f7 section1c8
> u-boot-spl-dtb 101c8 4f7 u-boot-spl-dtb 0
> fdtmap 10c0a 4f5 fdtmap 10c0a
>
>
> Seems something went wrong, any ideas? Or did I misuse?
Can you please push a tree somewhere so I can try this?
Regards,
Simon
Re: U-Boot SPL not getting generated
Hi Venkatakrishnan, On Wed, 16 Nov 2022 at 16:56, Venkatakrishnan S ic11539 wrote: > > Hi, > > I am trying to generate u-boot spl for a custom processor based out of > risc-v arch. I have done the defconfig for u-boot proper and is > working now. I am able to use that alone with opensbi. I am trying to > generate u-boot spl for the same board and I am not able to generate > it despite adding the config option in the defconfig for the board. > > The options enabled/added for u-boot spl are : > > CONFIG_SPL_DM_SPI=y > CONFIG_SPL_MMC_SUPPORT=y > CONFIG_SPL=y > CONFIG_SPL_BUILD=y > CONFIG_SPL_SPI_FLASH_SUPPORT=y > CONFIG_SPL_SPI_SUPPORT=y > CONFIG_SPL_LOAD_FIT_ADDRESS=0x8000 > CONFIG_SPL_CLK=y You cannot enable CONFIG_SPL=y in the defconfig, if that is what you are doing. It needs to happen in the Kconfig as with other boards. Check the .config to see what is actually enabled. Regards, Simon
Re: [PATCH] image: fit: Fix not verifying data configuration
Hi Sean,
On Thu, 13 Oct 2022 at 09:41, Sean Anderson wrote:
>
>
>
> On 10/13/22 3:14 AM, Rasmus Villemoes wrote:
> > On 12/10/2022 18.28, Sean Anderson wrote:
> >> On 10/12/22 08:59, Simon Glass wrote:
> >>> Hi Sean,
> >>>
> >>> On Tue, 11 Oct 2022 at 17:25, Sean Anderson
> >>> wrote:
>
> When reading data from a FIT image, we must verify the configuration we
> get it from. This is because when we have a key with required = "conf",
> the image does not need any particular signature or hash. The
> configuration is the only required verification, so we must verify it.
>
> Users of fit_get_data_node are liable to load unsigned data unless the
> user has set required = "image". Even then, they are vulnerable to
> mix-and-match attacks. This also affects other callers of
> fit_image_verify which don't first call fit_config_verify, such as
> source and imxtract. I don't think there is a backwards-compatible way
> to fix these interfaces. Fundamentally, selecting data by image when
> images are not required to be verified is unsafe.
>
> Fixes: 37feaf2f727 ("image: fit: Add some helpers for getting data")
> Signed-off-by: Sean Anderson
> ---
>
> boot/image-fit.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/boot/image-fit.c b/boot/image-fit.c
> index 9c04ff78a15..632fd405e29 100644
> --- a/boot/image-fit.c
> +++ b/boot/image-fit.c
> @@ -1948,7 +1948,14 @@ int fit_get_data_node(const void *fit, const
> char *image_uname,
> int fit_get_data_conf_prop(const void *fit, const char *prop_name,
> const void **data, size_t *size)
> {
> - int noffset = fit_conf_get_node(fit, NULL);
> + int ret, noffset = fit_conf_get_node(fit, NULL);
> +
> + if (noffset < 0)
> + return noffset;
> +
> + ret = fit_config_verify(fit, noffset);
> + if (ret)
> + return ret;
>
> noffset = fit_conf_get_prop_node(fit, noffset, prop_name);
> return fit_get_data_tail(fit, noffset, data, size);
> --
> 2.35.1.1320.gc452695387.dirty
>
> >>>
> >>> This is supposed to work by first verifying the configuration with
> >>> fit_config_verify(). After that, images in that configuration can be
> >>> freely loaded, verified by the hash that each image has.
> >>
> >> Well, this function was made to replaces several cases where code loaded
> >> a FIT image from somewhere, and then wanted to get data from an image
> >> based on the configuration. Typically they only want to extract one
> >> image, which is the common case for e.g. loading firmware. This idea of
> >> this function is to make the common case of "find me the image data from
> >> the default config and verify it" easier. If you look at the existing
> >> code which uses this function, they do not verify the configuration
> >> first. This is mainly because the original versions of this code which I
> >> replaced with this function did not verify the configuration either.
> >>
> >> So while the above process works for an integrated verification process,
> >> like what is done by bootm, it doesn't really work for one-off loading
> >> of image data. In particular, the requirements to make this secure
> >> (using required = "image" for your key), are not default. When I was
> >> trying to determine whether the source command would be OK to use to
> >> load some configuration, I looked at it and saw that it did
> >> fit_image_verify. I thought that was fine, but if you use required =
> >> "config", then all that does is verify the hash.
> >
> > Yeah, so I've raised this problem with the "source" shell command
> > previously, but never got a satisfactory answer:
> >
> > https://lore.kernel.org/u-boot/7d711133-d513-5bcb-52f2-a9dbaa9ee...@prevas.dk/
> >
> > So does your patch now mean that it's possible to get a
> > bootscript-wrapped-in-a-FIT-image verified, possibly by adding some
> > dummy (or not so dummy?) "configurations" node? Can you give a complete
> > .its showing how I can build a verifiable boot script?
>
> No. I didn't convert source because it also checks to ensure that the
> image type is correct, which fit_get_data_node doesn't check. However,
> it still uses the image name to determine the data to source, which has
> all the problems as discussed above.
>
> I think to do this right we would need either
>
> - A version of fit_image_verify which treats required = "config" as
> required = "image". This could be used for cases where the caller
> doesn't verify a config (such as in cases when the user specifies an
> image directly).
Without config verification we are subject to mix-and-match attacks.
> - Add support for specifying a config node. This would be something like
> the addr#config syntax used by bootm. Of
Re: [PATCH] fdt: Fix bounds check in devfdt_get_addr_index
Hi Samuel,
On Thu, 17 Nov 2022 at 21:00, Samuel Holland wrote:
>
> Hi Simon,
>
> On 11/7/22 17:35, Simon Glass wrote:
> > Hi Samuel,
> >
> > On Mon, 31 Oct 2022 at 13:27, Simon Glass wrote:
> >>
> >> On Sun, 30 Oct 2022 at 21:41, Samuel Holland wrote:
> >>>
> >>> reg must contain enough cells for the entire next address/size pair
> >>> after skipping `index` pairs. The previous code allows an out-of-bounds
> >>> read when na + ns > 1.
> >>>
> >>> Fixes: 69b41388ba45 ("dm: core: Add a new api to get indexed device
> >>> address")
> >>> Signed-off-by: Samuel Holland
> >>> ---
> >>>
> >>> drivers/core/fdtaddr.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> Reviewed-by: Simon Glass
> >
> > Somehow this break PPC in CI:
> >
> > https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/524776
> >
> > Can you please take a look?
>
> The cause is the call to lists_bind_fdt() inside serial_check_stdout(),
> which sets the UART's parent device to gd->dm_root, when the real parent
> should be a simple-bus node with a ranges property.
>
> Then when we go to probe the UART, devfdt_get_addr_index() does:
>
> int parent = dev_of_offset(dev->parent);
> // ...
> na = fdt_address_cells(gd->fdt_blob, parent);
>
> So devfdt_get_addr_index() sees the wrong number of address/size cells
> (2 instead of 1) and the check fails. This only worked previously
> because the check in devfdt_get_addr_index() would always succeed for
> index == 0.
>
> This patch fixes things, by setting the UART's parent device correctly:
>
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -623,9 +623,7 @@
> .priv_auto = sizeof(struct ns16550),
> .probe = ns16550_serial_probe,
> .ops= _serial_ops,
> -#if !CONFIG_IS_ENABLED(OF_CONTROL)
> .flags = DM_FLAG_PRE_RELOC,
> -#endif
We should put the dm tag in the device tree node instead.
> };
>
> DM_DRIVER_ALIAS(ns16550_serial, ti_da830_uart)
>
> Or maybe devfdt_get_addr_index() should be looking at the FDT node
> parent, instead of the DM parent. This also fixes things:
>
> --- a/drivers/core/fdtaddr.c
> +++ b/drivers/core/fdtaddr.c
> @@ -22,7 +22,7 @@
> {
> #if CONFIG_IS_ENABLED(OF_REAL)
> int offset = dev_of_offset(dev);
> - int parent = dev_of_offset(dev->parent);
> + int parent = fdt_parent_offset(gd->fdt_blob, offset);
That is slow, best avoided.
> fdt_addr_t addr;
>
> if (CONFIG_IS_ENABLED(OF_TRANSLATE)) {
Regards,
Simon
Re: [PATCH v2020.01] tool: ifwitool: The function localtime() can return NULL.
On Fri, 18 Nov 2022 at 05:36, Mikhail Ilin wrote: > > This will cause the local_time pointer is passed as the 4th argument > to function strftime() to also point to NULL. This result in a > segmentation fault. Thus, it's necessary to add a check of the local_time > pointer to NULL. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Mikhail Ilin > --- > tools/ifwitool.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH] tool: ifwitool: Fix buffer overflow
On Fri, 18 Nov 2022 at 05:37, Mikhail Ilin wrote:
>
> An incorrect 1st parameter is passed to the fix_member()
> function. Should use a pointer to the beginning of the parent structure
> (bpdt or subpart_dir, because are boxed), not to their fields. Otherwise,
> this leads to an overrun of the structure boundary, since in the
> fix_member() function, an 'offset' is made, relative to the 1st argument,
> which itself is an 'offset' from the beginning of the structure.
>
> Signed-off-by: Mikhail Ilin
> ---
> tools/ifwitool.c | 44 +++-
> 1 file changed, 19 insertions(+), 25 deletions(-)
Reviewed-by: Simon Glass
Fixes: 56bf4f86307 ("x86: Add ifwitool for Intel Integrated Firmware Image")
Re: [PATCH v9 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface
On 11/18/22 10:37, Masahisa Kojima wrote:
Hi Heinrhch,
On Fri, 18 Nov 2022 at 08:07, Heinrich Schuchardt wrote:
On 11/16/22 11:28, Masahisa Kojima wrote:
This commit adds the menu-driven UEFI Secure Boot Key
enrollment interface. User can enroll PK, KEK, db
and dbx by selecting file.
Only the signed EFI Signature List(s) with an authenticated
header, typically '.auth' file, is accepted.
To clear the PK, KEK, db and dbx, user needs to enroll the null key
signed by PK or KEK.
Signed-off-by: Masahisa Kojima
Reviewed-by: Ilias Apalodimas
---
Changes in v9:
- move file size check, set ret = EFI_INVALID_PARAMETER
Changes in v8:
- fix missing efi_file_close_int() call
Changes in v7:
- only accept .auth file.
- remove creating time based authenticated variable
- update commit message
- use efi_file_size()
Changes in v6:
- use efi_secure_boot_enabled()
- replace with WIN_CERT_REVISION_2_0 from pe.h
- call efi_build_signature_store() to check the valid EFI Signature List
- update comment
Changes in v4:
- add CONFIG_EFI_MM_COMM_TEE dependency
- fix error handling
Changes in v3:
- fix error handling
Changes in v2:
- allow to enroll .esl file
- fix typos
- add function comments
cmd/Makefile | 5 +
cmd/eficonfig.c | 3 +
cmd/eficonfig_sbkey.c | 246 ++
include/efi_config.h | 5 +
4 files changed, 259 insertions(+)
create mode 100644 cmd/eficonfig_sbkey.c
diff --git a/cmd/Makefile b/cmd/Makefile
index 2444d116c0..0b6a96c1d9 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -66,6 +66,11 @@ obj-$(CONFIG_CMD_EEPROM) += eeprom.o
obj-$(CONFIG_EFI) += efi.o
obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o
obj-$(CONFIG_CMD_EFICONFIG) += eficonfig.o
+ifdef CONFIG_CMD_EFICONFIG
+ifdef CONFIG_EFI_MM_COMM_TEE
+obj-$(CONFIG_EFI_SECURE_BOOT) += eficonfig_sbkey.o
+endif
+endif
obj-$(CONFIG_CMD_ELF) += elf.o
obj-$(CONFIG_CMD_EROFS) += erofs.o
obj-$(CONFIG_HUSH_PARSER) += exit.o
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 12babb76c2..d79194794b 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -2435,6 +2435,9 @@ static const struct eficonfig_item
maintenance_menu_items[] = {
{"Edit Boot Option", eficonfig_process_edit_boot_option},
{"Change Boot Order", eficonfig_process_change_boot_order},
{"Delete Boot Option", eficonfig_process_delete_boot_option},
+#if (CONFIG_IS_ENABLED(EFI_SECURE_BOOT) && CONFIG_IS_ENABLED(EFI_MM_COMM_TEE))
+ {"Secure Boot Configuration", eficonfig_process_secure_boot_config},
+#endif
{"Quit", eficonfig_process_quit},
};
diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c
new file mode 100644
index 00..e9aaf76bf8
--- /dev/null
+++ b/cmd/eficonfig_sbkey.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Menu-driven UEFI Secure Boot Key Maintenance
+ *
+ * Copyright (c) 2022 Masahisa Kojima, Linaro Limited
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+enum efi_sbkey_signature_type {
+ SIG_TYPE_X509 = 0,
+ SIG_TYPE_HASH,
+ SIG_TYPE_CRL,
+ SIG_TYPE_RSA2048,
+};
+
+struct eficonfig_sigtype_to_str {
+ efi_guid_t sig_type;
+ char *str;
+ enum efi_sbkey_signature_type type;
+};
+
+static const struct eficonfig_sigtype_to_str sigtype_to_str[] = {
+ {EFI_CERT_X509_GUID,"X509", SIG_TYPE_X509},
+ {EFI_CERT_SHA256_GUID, "SHA256", SIG_TYPE_HASH},
+ {EFI_CERT_X509_SHA256_GUID, "X509_SHA256 CRL", SIG_TYPE_CRL},
+ {EFI_CERT_X509_SHA384_GUID, "X509_SHA384 CRL", SIG_TYPE_CRL},
+ {EFI_CERT_X509_SHA512_GUID, "X509_SHA512 CRL", SIG_TYPE_CRL},
+ /* U-Boot does not support the following signature types */
+/* {EFI_CERT_RSA2048_GUID, "RSA2048",
SIG_TYPE_RSA2048}, */
+/* {EFI_CERT_RSA2048_SHA256_GUID, "RSA2048_SHA256",
SIG_TYPE_RSA2048}, */
+/* {EFI_CERT_SHA1_GUID,"SHA1", SIG_TYPE_HASH}, */
+/* {EFI_CERT_RSA2048_SHA_GUID, "RSA2048_SHA", SIG_TYPE_RSA2048
}, */
+/* {EFI_CERT_SHA224_GUID, "SHA224", SIG_TYPE_HASH}, */
+/* {EFI_CERT_SHA384_GUID, "SHA384", SIG_TYPE_HASH}, */
+/* {EFI_CERT_SHA512_GUID, "SHA512", SIG_TYPE_HASH}, */
+};
+
+/**
+ * file_have_auth_header() - check file has EFI_VARIABLE_AUTHENTICATION_2
header
+ * @buf: pointer to file
+ * @size:file size
+ * Return: true if file has auth header, false otherwise
+ */
+static bool file_have_auth_header(void *buf, efi_uintn_t size)
+{
+ struct efi_variable_authentication_2 *auth = buf;
+
+ if (auth->auth_info.hdr.wCertificateType != WIN_CERT_TYPE_EFI_GUID)
+ return false;
+
+ if (guidcmp(>auth_info.cert_type, _guid_cert_type_pkcs7))
+ return false;
+
+ return true;
+}
+
+/**
+ *
Re: [PATCH 2/2] rockchip: mkimage: make RC4 key const
On Fri, 18 Nov 2022 at 17:13, John Keeping wrote: > > This is read-only data, so mark it as such. > > Signed-off-by: John Keeping Reviewed-by: Philipp Tomsich
Re: [PATCH 1/2] rc4: mark key as const
On Fri, 18 Nov 2022 at 17:13, John Keeping wrote: > > Key data is never written so the parameter can be const, which allows > putting fixed keys in .rodata. > > Signed-off-by: John Keeping Reviewed-by: Philipp Tomsich
[PATCH 1/2] rc4: mark key as const
Key data is never written so the parameter can be const, which allows
putting fixed keys in .rodata.
Signed-off-by: John Keeping
---
include/rc4.h | 2 +-
lib/rc4.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/rc4.h b/include/rc4.h
index c1ff1349d4..d1257f20a4 100644
--- a/include/rc4.h
+++ b/include/rc4.h
@@ -15,6 +15,6 @@
* @len: Length of buffer in bytes
* @key: 16-byte key to use
*/
-void rc4_encode(unsigned char *buf, unsigned int len, unsigned char key[16]);
+void rc4_encode(unsigned char *buf, unsigned int len, const unsigned char
key[16]);
#endif
diff --git a/lib/rc4.c b/lib/rc4.c
index 0c00439843..720112d1fd 100644
--- a/lib/rc4.c
+++ b/lib/rc4.c
@@ -12,7 +12,7 @@
#endif
#include
-void rc4_encode(unsigned char *buf, unsigned int len, unsigned char key[16])
+void rc4_encode(unsigned char *buf, unsigned int len, const unsigned char
key[16])
{
unsigned char s[256], k[256], temp;
unsigned short i, j, t;
--
2.38.1
[PATCH 2/2] rockchip: mkimage: make RC4 key const
This is read-only data, so mark it as such.
Signed-off-by: John Keeping
---
tools/rkcommon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/rkcommon.c b/tools/rkcommon.c
index 0db45c2d41..fab61949e1 100644
--- a/tools/rkcommon.c
+++ b/tools/rkcommon.c
@@ -154,7 +154,7 @@ struct spl_params {
static struct spl_params spl_params = { 0 };
-static unsigned char rc4_key[16] = {
+static const unsigned char rc4_key[16] = {
124, 78, 3, 4, 85, 5, 9, 7,
45, 44, 123, 56, 23, 13, 23, 17
};
--
2.38.1
Re: [PATCH] Revert "lib: sparse: Make CHUNK_TYPE_RAW buffer aligned"
On 11/18/22 07:13, Gary Bisson wrote:
> This reverts commit 62649165cb02ab95b57360bb362886935f524f26.
>
> The patch decreased the write performance quite a bit.
> Here is an example on an i.MX 8M Quad platform.
> - Before the revert:
> Sending sparse 'vendor' 1/2 (516436 KB)OKAY [ 5.113s]
> Writing 'vendor' OKAY [128.335s]
> Sending sparse 'vendor' 2/2 (76100 KB) OKAY [ 0.802s]
> Writing 'vendor' OKAY [ 27.902s]
> - After the revert:
> Sending sparse 'vendor' 1/2 (516436 KB)OKAY [ 5.310s]
> Writing 'vendor' OKAY [ 18.041s]
> Sending sparse 'vendor' 2/2 (76100 KB) OKAY [ 1.244s]
> Writing 'vendor' OKAY [ 2.663s]
>
> Considering that the patch only moves buffer around to avoid a warning
> message about misaligned buffers, let's keep the best performances.
So what is the point of this warning?
--Sean
> Signed-off-by: Gary Bisson
> Signed-off-by: Troy Kisky
> ---
> lib/image-sparse.c | 69 ++
> 1 file changed, 8 insertions(+), 61 deletions(-)
>
> diff --git a/lib/image-sparse.c b/lib/image-sparse.c
> index 5ec0f94ab3e..d80fdbbf58e 100644
> --- a/lib/image-sparse.c
> +++ b/lib/image-sparse.c
> @@ -46,66 +46,9 @@
> #include
>
> #include
> -#include
>
> static void default_log(const char *ignored, char *response) {}
>
> -static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info,
> -lbaint_t blk, lbaint_t blkcnt,
> -void *data,
> -char *response)
> -{
> - lbaint_t n = blkcnt, write_blks, blks = 0, aligned_buf_blks = 100;
> - uint32_t *aligned_buf = NULL;
> -
> - if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
> - write_blks = info->write(info, blk, n, data);
> - if (write_blks < n)
> - goto write_fail;
> -
> - return write_blks;
> - }
> -
> - aligned_buf = memalign(ARCH_DMA_MINALIGN, info->blksz *
> aligned_buf_blks);
> - if (!aligned_buf) {
> - info->mssg("Malloc failed for: CHUNK_TYPE_RAW", response);
> - return -ENOMEM;
> - }
> -
> - while (blkcnt > 0) {
> - n = min(aligned_buf_blks, blkcnt);
> - memcpy(aligned_buf, data, n * info->blksz);
> -
> - /* write_blks might be > n due to NAND bad-blocks */
> - write_blks = info->write(info, blk + blks, n, aligned_buf);
> - if (write_blks < n) {
> - free(aligned_buf);
> - goto write_fail;
> - }
> -
> - blks += write_blks;
> - data += n * info->blksz;
> - blkcnt -= n;
> - }
> -
> - free(aligned_buf);
> - return blks;
> -
> -write_fail:
> - if (IS_ERR_VALUE(write_blks)) {
> - printf("%s: Write failed, block #" LBAFU " [" LBAFU "]
> (%lld)\n",
> -__func__, blk + blks, n, (long long)write_blks);
> - info->mssg("flash write failure", response);
> - return write_blks;
> - }
> -
> - /* write_blks < n */
> - printf("%s: Write failed, block #" LBAFU " [" LBAFU "]\n",
> -__func__, blk + blks, n);
> - info->mssg("flash write failure(incomplete)", response);
> - return -1;
> -}
> -
> int write_sparse_image(struct sparse_storage *info,
> const char *part_name, void *data, char *response)
> {
> @@ -209,11 +152,15 @@ int write_sparse_image(struct sparse_storage *info,
> return -1;
> }
>
> - blks = write_sparse_chunk_raw(info, blk, blkcnt,
> - data, response);
> - if (blks < 0)
> + blks = info->write(info, blk, blkcnt, data);
> + /* blks might be > blkcnt (eg. NAND bad-blocks) */
> + if (blks < blkcnt) {
> + printf("%s: %s" LBAFU " [" LBAFU "]\n",
> +__func__, "Write failed, block #",
> +blk, blks);
> + info->mssg("flash write failure", response);
> return -1;
> -
> + }
> blk += blks;
> bytes_written += ((u64)blkcnt) * info->blksz;
> total_blocks += chunk_header->chunk_sz;
Re: [PATCH v2 0/8] imx8: switch missing boards to binman
Hi Oliver, On Fri, Nov 18, 2022 at 10:48 AM Oliver Graute wrote: > Ok, I can confirm it builds without the binary blobs with this changes. > I'll change it in patches accordingly Your series has already landed in master. Please send any incremental patches to address Marcel's feedback on top of it.
Re: [PATCH v2 0/8] imx8: switch missing boards to binman
On 11/11/22, Fabio Estevam wrote:
> On Fri, Nov 11, 2022 at 2:40 PM Fabio Estevam wrote:
>
> > I removed SPL support, which does not seems to be needed as the scufw
> > handles DDR init.
> >
> > I don't have access to an imx8qm/qxp board here.
> >
> > Could you try removing SPL support from your board and see if it boots
> > with binman support?
>
> Ok, let's SPL for now as this is a different topic for discussion.
>
>
> With the change below against u-boot-imx master-next branch, the
> imx8qm/qxp boards build fine without blobs:
>
> diff --git a/arch/arm/dts/imx8qm-u-boot.dtsi b/arch/arm/dts/imx8qm-u-boot.dtsi
> index 3507489a813c..a3e0af48109b 100644
> --- a/arch/arm/dts/imx8qm-u-boot.dtsi
> +++ b/arch/arm/dts/imx8qm-u-boot.dtsi
> @@ -10,7 +10,7 @@
> };
>
> {
> -#ifdef CONFIG_SPL
> +#ifdef CONFIG_SPL_BUILD
> u-boot-spl-ddr {
> align = <4>;
> align-size = <4>;
> @@ -50,7 +50,7 @@
> arch = "arm64";
> compression = "none";
> description = "U-Boot (64-bit)";
> - load = ;
> + load = ;
> type = "standalone";
>
> uboot-blob {
> diff --git a/arch/arm/dts/imx8qxp-u-boot.dtsi
> b/arch/arm/dts/imx8qxp-u-boot.dtsi
> index 01183f8ade63..7622c40906f1 100644
> --- a/arch/arm/dts/imx8qxp-u-boot.dtsi
> +++ b/arch/arm/dts/imx8qxp-u-boot.dtsi
> @@ -10,7 +10,7 @@
> };
>
> {
> -#ifdef CONFIG_SPL
> +#ifdef CONFIG_SPL_BUILD
> u-boot-spl-ddr {
> align = <4>;
> align-size = <4>;
> @@ -50,7 +50,7 @@
> arch = "arm64";
> compression = "none";
> description = "U-Boot (64-bit)";
> - load = ;
> + load = ;
> type = "standalone";
>
> uboot-blob {
>
> Could you please test it on your boards?
Ok, I can confirm it builds without the binary blobs with this changes.
I'll change it in patches accordingly
Best regards,
Oliver
Re: [PATCH v2 0/8] imx8: switch missing boards to binman
On 11/11/22, Fabio Estevam wrote:
> Hi Oliver,
>
> On Fri, Nov 11, 2022 at 9:54 AM Oliver Graute wrote:
>
> > imx8mm_evk_defconfig
> > apalis-imx8_defconfig
> >
> > But where is the difference here?
>
> With the change below, the imx8qm_mek_defconfig builds without errors
> in the absence of the blobs:
>
> diff --git a/arch/arm/dts/imx8qm-u-boot.dtsi b/arch/arm/dts/imx8qm-u-boot.dtsi
> index 3507489a813c..442e64badc39 100644
> --- a/arch/arm/dts/imx8qm-u-boot.dtsi
> +++ b/arch/arm/dts/imx8qm-u-boot.dtsi
> @@ -50,7 +50,7 @@
> arch = "arm64";
> compression = "none";
> description = "U-Boot (64-bit)";
> - load = ;
> + load = ;
> type = "standalone";
>
> uboot-blob {
> diff --git a/arch/arm/dts/imx8qxp-u-boot.dtsi
> b/arch/arm/dts/imx8qxp-u-boot.dtsi
> index 01183f8ade63..e8df5bb8bfea 100644
> --- a/arch/arm/dts/imx8qxp-u-boot.dtsi
> +++ b/arch/arm/dts/imx8qxp-u-boot.dtsi
> @@ -50,7 +50,7 @@
> arch = "arm64";
> compression = "none";
> description = "U-Boot (64-bit)";
> - load = ;
> + load = ;
> type = "standalone";
>
> uboot-blob {
> diff --git a/configs/imx8qm_mek_defconfig b/configs/imx8qm_mek_defconfig
> index 4fc828681b6c..4c220188fbb6 100644
> --- a/configs/imx8qm_mek_defconfig
> +++ b/configs/imx8qm_mek_defconfig
> @@ -3,21 +3,12 @@ CONFIG_ARCH_IMX8=y
> CONFIG_TEXT_BASE=0x8002
> CONFIG_SYS_MALLOC_LEN=0x240
> CONFIG_SYS_MALLOC_F_LEN=0x8000
> -CONFIG_SPL_GPIO=y
> -CONFIG_SPL_LIBCOMMON_SUPPORT=y
> -CONFIG_SPL_LIBGENERIC_SUPPORT=y
> CONFIG_NR_DRAM_BANKS=3
> CONFIG_ENV_SIZE=0x1000
> CONFIG_ENV_OFFSET=0x40
> CONFIG_DM_GPIO=y
> CONFIG_DEFAULT_DEVICE_TREE="fsl-imx8qm-mek"
> -CONFIG_SPL_TEXT_BASE=0x10
> CONFIG_TARGET_IMX8QM_MEK=y
> -CONFIG_SPL_MMC=y
> -CONFIG_SPL_SERIAL=y
> -CONFIG_SPL_DRIVERS_MISC=y
> -CONFIG_SPL=y
> -CONFIG_SPL_LOAD_IMX_CONTAINER=y
> CONFIG_IMX_CONTAINER_CFG="board/freescale/imx8qm_mek/uboot-container.cfg"
> CONFIG_SYS_LOAD_ADDR=0x8028
> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> @@ -32,22 +23,8 @@ CONFIG_USE_BOOTCOMMAND=y
> CONFIG_BOOTCOMMAND="mmc dev ${mmcdev}; if mmc rescan; then if run
> loadbootscript; then run bootscript; else if test ${sec_boot} = yes;
> then if run loadcntr; then run mmcboot; else run netboot; fi; else if
> run loadimage; then run mmcboot; else run netboot; fi; fi; fi; else
> booti ${loadaddr} - ${fdt_addr}; fi"
> CONFIG_LOG=y
> CONFIG_BOARD_EARLY_INIT_F=y
> -CONFIG_SPL_MAX_SIZE=0x1f000
> -CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
> -CONFIG_SPL_BSS_START_ADDR=0x128000
> -CONFIG_SPL_BSS_MAX_SIZE=0x1000
> -CONFIG_SPL_BOARD_INIT=y
> -CONFIG_SPL_SYS_MALLOC_SIMPLE=y
> -# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
> -CONFIG_SPL_STACK=0x13e000
> -CONFIG_SYS_SPL_MALLOC=y
> -CONFIG_HAS_CUSTOM_SPL_MALLOC_START=y
> -CONFIG_CUSTOM_SYS_SPL_MALLOC_ADDR=0x12
> -CONFIG_SYS_SPL_MALLOC_SIZE=0x3000
> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x800
> -CONFIG_SPL_POWER_DOMAIN=y
> -CONFIG_SPL_WATCHDOG=y
> CONFIG_HUSH_PARSER=y
> CONFIG_SYS_CBSIZE=256
> CONFIG_SYS_PBSIZE=276
> @@ -66,14 +43,11 @@ CONFIG_CMD_MII=y
> CONFIG_CMD_PING=y
> CONFIG_CMD_CACHE=y
> CONFIG_CMD_FAT=y
> -CONFIG_SPL_OF_CONTROL=y
> CONFIG_ENV_OVERWRITE=y
> CONFIG_ENV_IS_IN_MMC=y
> CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> CONFIG_SYS_MMC_ENV_DEV=1
> CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
> -CONFIG_SPL_DM=y
> -CONFIG_SPL_CLK=y
> CONFIG_CLK_IMX8=y
> CONFIG_CPU=y
> CONFIG_MXC_GPIO=y
> @@ -93,16 +67,12 @@ CONFIG_FEC_MXC_MDIO_BASE=0x5B04
> CONFIG_FEC_MXC=y
> CONFIG_MII=y
> CONFIG_PINCTRL=y
> -CONFIG_SPL_PINCTRL=y
> CONFIG_PINCTRL_IMX8=y
> CONFIG_POWER_DOMAIN=y
> CONFIG_IMX8_POWER_DOMAIN=y
> CONFIG_DM_REGULATOR=y
> -CONFIG_SPL_DM_REGULATOR=y
> CONFIG_DM_REGULATOR_FIXED=y
> CONFIG_DM_REGULATOR_GPIO=y
> -CONFIG_SPL_DM_REGULATOR_GPIO=y
> CONFIG_DM_SERIAL=y
> CONFIG_FSL_LPUART=y
> -CONFIG_SPL_TINY_MEMSET=y
> # CONFIG_EFI_LOADER is not set
>
> I removed SPL support, which does not seems to be needed as the scufw
> handles DDR init.
I know that DDR init is done in scfw for imx8qm. Because I patched these
values gotten from a NXP Excel RPA sheet into our scfw a year ago as we
got our Micron LPDDR4. But I wasn't aware that then the SPL stuff is
superflous then. Is this really the case? Is there no other reason for
SPL? Can you confirm?
>
> I don't have access to an imx8qm/qxp board here.
>
> Could you try removing SPL support from your board and see if it boots
> with binman support?
Ok,
[PATCH] tool: ifwitool: Fix buffer overflow
An incorrect 1st parameter is passed to the fix_member()
function. Should use a pointer to the beginning of the parent structure
(bpdt or subpart_dir, because are boxed), not to their fields. Otherwise,
this leads to an overrun of the structure boundary, since in the
fix_member() function, an 'offset' is made, relative to the 1st argument,
which itself is an 'offset' from the beginning of the structure.
Signed-off-by: Mikhail Ilin
---
tools/ifwitool.c | 44 +++-
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/tools/ifwitool.c b/tools/ifwitool.c
index 543e9d4e70..5cc0981411 100644
--- a/tools/ifwitool.c
+++ b/tools/ifwitool.c
@@ -1443,23 +1443,20 @@ static void bpdt_fixup_write_buffer(struct buffer *buf)
size_t offset = 0;
- offset = fix_member(>signature, offset, sizeof(h->signature));
- offset = fix_member(>descriptor_count, offset,
- sizeof(h->descriptor_count));
- offset = fix_member(>bpdt_version, offset, sizeof(h->bpdt_version));
- offset = fix_member(>xor_redundant_block, offset,
- sizeof(h->xor_redundant_block));
- offset = fix_member(>ifwi_version, offset, sizeof(h->ifwi_version));
- offset = fix_member(>fit_tool_version, offset,
- sizeof(h->fit_tool_version));
+ offset = fix_member(, offset, sizeof(h->signature));
+ offset = fix_member(, offset, sizeof(h->descriptor_count));
+ offset = fix_member(, offset, sizeof(h->bpdt_version));
+ offset = fix_member(, offset, sizeof(h->xor_redundant_block));
+ offset = fix_member(, offset, sizeof(h->ifwi_version));
+ offset = fix_member(, offset, sizeof(h->fit_tool_version));
uint32_t i;
for (i = 0; i < count; i++) {
- offset = fix_member([i].type, offset, sizeof(e[i].type));
- offset = fix_member([i].flags, offset, sizeof(e[i].flags));
- offset = fix_member([i].offset, offset, sizeof(e[i].offset));
- offset = fix_member([i].size, offset, sizeof(e[i].size));
+ offset = fix_member(, offset, sizeof(e[i].type));
+ offset = fix_member(, offset, sizeof(e[i].flags));
+ offset = fix_member(, offset, sizeof(e[i].offset));
+ offset = fix_member(, offset, sizeof(e[i].size));
}
}
@@ -1657,24 +1654,21 @@ static void subpart_dir_fixup_write_buffer(struct
buffer *buf)
size_t count = h->num_entries;
size_t offset = 0;
- offset = fix_member(>marker, offset, sizeof(h->marker));
- offset = fix_member(>num_entries, offset, sizeof(h->num_entries));
- offset = fix_member(>header_version, offset,
- sizeof(h->header_version));
- offset = fix_member(>entry_version, offset,
- sizeof(h->entry_version));
- offset = fix_member(>header_length, offset,
- sizeof(h->header_length));
- offset = fix_member(>checksum, offset, sizeof(h->checksum));
+ offset = fix_member(, offset, sizeof(h->marker));
+ offset = fix_member(, offset, sizeof(h->num_entries));
+ offset = fix_member(, offset, sizeof(h->header_version));
+ offset = fix_member(, offset, sizeof(h->entry_version));
+ offset = fix_member(, offset, sizeof(h->header_length));
+ offset = fix_member(, offset, sizeof(h->checksum));
offset += sizeof(h->name);
uint32_t i;
for (i = 0; i < count; i++) {
offset += sizeof(e[i].name);
- offset = fix_member([i].offset, offset, sizeof(e[i].offset));
- offset = fix_member([i].length, offset, sizeof(e[i].length));
- offset = fix_member([i].rsvd, offset, sizeof(e[i].rsvd));
+ offset = fix_member(, offset, sizeof(e[i].offset));
+ offset = fix_member(, offset, sizeof(e[i].length));
+ offset = fix_member(, offset, sizeof(e[i].rsvd));
}
}
--
2.17.1
[PATCH v2020.01] tool: ifwitool: The function localtime() can return NULL.
This will cause the local_time pointer is passed as the 4th argument to function strftime() to also point to NULL. This result in a segmentation fault. Thus, it's necessary to add a check of the local_time pointer to NULL. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Mikhail Ilin --- tools/ifwitool.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/ifwitool.c b/tools/ifwitool.c index 543e9d4e70..b9873deb6b 100644 --- a/tools/ifwitool.c +++ b/tools/ifwitool.c @@ -1630,6 +1630,8 @@ static void init_manifest_header(struct manifest_header *hdr, size_t size) curr_time = time(NULL); local_time = localtime(_time); + assert(local_time != NULL); + strftime(buffer, sizeof(buffer), "0x%Y%m%d", local_time); hdr->date = strtoul(buffer, NULL, 16); -- 2.17.1
[PATCH] distro/pxeboot: Handle prompt variable
Regarding the documentation found here: https://github.com/u-boot/u-boot/blob/master/common/menu.c#L347 If both timeout and prompt is set to 0 the default entry shall be booted immediately. However the current behaviour is that the prompt is shown (tested with distroboot) until the user selects an entry (no timeout). This change implements a behaviour as documented. It was tested with distroboot. Signed-off-by: Manuel Traut --- boot/pxe_utils.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 8133006875..075a0f2830 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -1359,7 +1359,10 @@ static int parse_pxefile_top(struct pxe_context *ctx, char *p, unsigned long bas break; case T_PROMPT: - eol_or_eof(); + err = parse_integer(, >prompt); + // Do not fail if prompt configuration is undefined + if (err < 0) + eol_or_eof(); break; case T_EOL: -- 2.35.1
[PATCH] Revert "lib: sparse: Make CHUNK_TYPE_RAW buffer aligned"
This reverts commit 62649165cb02ab95b57360bb362886935f524f26.
The patch decreased the write performance quite a bit.
Here is an example on an i.MX 8M Quad platform.
- Before the revert:
Sending sparse 'vendor' 1/2 (516436 KB)OKAY [ 5.113s]
Writing 'vendor' OKAY [128.335s]
Sending sparse 'vendor' 2/2 (76100 KB) OKAY [ 0.802s]
Writing 'vendor' OKAY [ 27.902s]
- After the revert:
Sending sparse 'vendor' 1/2 (516436 KB)OKAY [ 5.310s]
Writing 'vendor' OKAY [ 18.041s]
Sending sparse 'vendor' 2/2 (76100 KB) OKAY [ 1.244s]
Writing 'vendor' OKAY [ 2.663s]
Considering that the patch only moves buffer around to avoid a warning
message about misaligned buffers, let's keep the best performances.
Signed-off-by: Gary Bisson
Signed-off-by: Troy Kisky
---
lib/image-sparse.c | 69 ++
1 file changed, 8 insertions(+), 61 deletions(-)
diff --git a/lib/image-sparse.c b/lib/image-sparse.c
index 5ec0f94ab3e..d80fdbbf58e 100644
--- a/lib/image-sparse.c
+++ b/lib/image-sparse.c
@@ -46,66 +46,9 @@
#include
#include
-#include
static void default_log(const char *ignored, char *response) {}
-static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info,
- lbaint_t blk, lbaint_t blkcnt,
- void *data,
- char *response)
-{
- lbaint_t n = blkcnt, write_blks, blks = 0, aligned_buf_blks = 100;
- uint32_t *aligned_buf = NULL;
-
- if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
- write_blks = info->write(info, blk, n, data);
- if (write_blks < n)
- goto write_fail;
-
- return write_blks;
- }
-
- aligned_buf = memalign(ARCH_DMA_MINALIGN, info->blksz *
aligned_buf_blks);
- if (!aligned_buf) {
- info->mssg("Malloc failed for: CHUNK_TYPE_RAW", response);
- return -ENOMEM;
- }
-
- while (blkcnt > 0) {
- n = min(aligned_buf_blks, blkcnt);
- memcpy(aligned_buf, data, n * info->blksz);
-
- /* write_blks might be > n due to NAND bad-blocks */
- write_blks = info->write(info, blk + blks, n, aligned_buf);
- if (write_blks < n) {
- free(aligned_buf);
- goto write_fail;
- }
-
- blks += write_blks;
- data += n * info->blksz;
- blkcnt -= n;
- }
-
- free(aligned_buf);
- return blks;
-
-write_fail:
- if (IS_ERR_VALUE(write_blks)) {
- printf("%s: Write failed, block #" LBAFU " [" LBAFU "]
(%lld)\n",
- __func__, blk + blks, n, (long long)write_blks);
- info->mssg("flash write failure", response);
- return write_blks;
- }
-
- /* write_blks < n */
- printf("%s: Write failed, block #" LBAFU " [" LBAFU "]\n",
- __func__, blk + blks, n);
- info->mssg("flash write failure(incomplete)", response);
- return -1;
-}
-
int write_sparse_image(struct sparse_storage *info,
const char *part_name, void *data, char *response)
{
@@ -209,11 +152,15 @@ int write_sparse_image(struct sparse_storage *info,
return -1;
}
- blks = write_sparse_chunk_raw(info, blk, blkcnt,
- data, response);
- if (blks < 0)
+ blks = info->write(info, blk, blkcnt, data);
+ /* blks might be > blkcnt (eg. NAND bad-blocks) */
+ if (blks < blkcnt) {
+ printf("%s: %s" LBAFU " [" LBAFU "]\n",
+ __func__, "Write failed, block #",
+ blk, blks);
+ info->mssg("flash write failure", response);
return -1;
-
+ }
blk += blks;
bytes_written += ((u64)blkcnt) * info->blksz;
total_blocks += chunk_header->chunk_sz;
--
2.35.1
Re: [v1] spl: nand: allow partial nand page reads during nand_spl_load_image
Hi Colin,
On Tue, Nov 15, 2022 at 5:35 PM Colin Foster
wrote:
>
> The nand_spl_load_image function was guaranteed to read an entire block
> into RAM, regardless of how many bytes were to be read. This is
> particularly problematic when spl_load_legacy_image is called, as this
> function attempts to load a struct image_header but gets surprised with
> a full flash sector.
>
> Allow partial reads to restore functionality to the code path
> spl_nand_load_element() -> spl_load_legacy_img() ->
> spl_nand_legacy_read(load, header, sizeof(hdr), header);
>
> Signed-off-by: Colin Foster
> ---
> drivers/mtd/nand/raw/nand_spl_loaders.c | 36 -
> 1 file changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_spl_loaders.c
> b/drivers/mtd/nand/raw/nand_spl_loaders.c
> index 4befc75c04..84ac482c06 100644
> --- a/drivers/mtd/nand/raw/nand_spl_loaders.c
> +++ b/drivers/mtd/nand/raw/nand_spl_loaders.c
> @@ -1,9 +1,18 @@
> +/*
> + * Temporary storage for non NAND page aligned and non NAND page sized
> + * reads. Note: This does not support runtime detected FLASH yet, but
> + * that should be reasonably easy to fix by making the buffer large
> + * enough :)
> + */
> +static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE];
> +
> int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
> {
> + unsigned int read_this_time = CONFIG_SYS_NAND_PAGE_SIZE;
> + unsigned int size_remaining = size;
> unsigned int block, lastblock;
> unsigned int page, page_offset;
>
> - /* offs has to be aligned to a page address! */
> block = offs / CONFIG_SYS_NAND_BLOCK_SIZE;
> lastblock = (offs + size - 1) / CONFIG_SYS_NAND_BLOCK_SIZE;
> page = (offs % CONFIG_SYS_NAND_BLOCK_SIZE) /
> CONFIG_SYS_NAND_PAGE_SIZE;
> @@ -12,8 +21,18 @@ int nand_spl_load_image(uint32_t offs, unsigned int size,
> void *dst)
> while (block <= lastblock) {
> if (!nand_is_bad_block(block)) {
> /* Skip bad blocks */
> - while (page < CONFIG_SYS_NAND_PAGE_COUNT) {
> - nand_read_page(block, page, dst);
> + while (page < CONFIG_SYS_NAND_PAGE_COUNT &&
> + size_remaining > 0) {
> + if (size_remaining <
> CONFIG_SYS_NAND_PAGE_SIZE)
> + {
> + nand_read_page(block, page,
> + scratch_buf);
> + memcpy(dst, scratch_buf,
> + size_remaining);
> + read_this_time = size_remaining;
> + } else {
> + nand_read_page(block, page, dst);
> + }
> /*
> * When offs is not aligned to page address
> the
> * extra offset is copied to dst as well. Copy
> @@ -26,7 +45,8 @@ int nand_spl_load_image(uint32_t offs, unsigned int size,
> void *dst)
> dst = (void *)((int)dst -
> page_offset);
> page_offset = 0;
> }
> - dst += CONFIG_SYS_NAND_PAGE_SIZE;
> + dst += read_this_time;
> + size_remaining -= read_this_time;
> page++;
> }
>
> @@ -70,14 +90,6 @@ u32 nand_spl_adjust_offset(u32 sector, u32 offs)
> }
>
> #ifdef CONFIG_SPL_UBI
> -/*
> - * Temporary storage for non NAND page aligned and non NAND page sized
> - * reads. Note: This does not support runtime detected FLASH yet, but
> - * that should be reasonably easy to fix by making the buffer large
> - * enough :)
> - */
> -static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE];
> -
> /**
> * nand_spl_read_block - Read data from physical eraseblock into a buffer
> * @block: Number of the physical eraseblock
> --
> 2.25.1
>
Reviewed-by: Dario Binacchi
Thanks and regards,
Dario
--
Dario Binacchi
Embedded Linux Developer
dario.binac...@amarulasolutions.com
__
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
i...@amarulasolutions.com
www.amarulasolutions.com
[PATCH] cli_hush: fix 'exit' cmd that was not exiting scripts
Commit 8c4e3b79bd0bb76eea16869e9666e19047c0d005 supposedly
passed one-level up the argument passed to 'exit' but it also
broke 'exit' purpose of stopping a script.
In reality, even if 'do_exit()' is capable of returning any
integer, the cli only admits '1' or '0' as return values.
This commit respects the current implementation to allow 'exit'
to at least return '1' for future processing, but returns
when the command being run is 'exit'.
Before this:
=> setenv foo 'echo bar ; exit 3 ; echo should not see this'; run foo;
echo $?
bar
should not see this
0
=> setenv foo 'echo bar ; exit 1 ; echo should not see this'; run foo;
echo $?
bar
should not see this
0
=> setenv foo 'echo bar ; exit 0 ; echo should not see this'; run foo;
echo $?
bar
should not see this
0
=> setenv foo 'echo bar ; exit -1 ; echo should not see this'; run foo;
echo $?
bar
should not see this
0
=> setenv foo 'echo bar ; exit -2 ; echo should not see this'; run foo;
echo $?
bar
should not see this
0
=> setenv foo 'echo bar ; exit ; echo should not see this'; run foo;
echo $?
bar
should not see this
0
After this:
=> setenv foo 'echo bar ; exit 3 ; echo should not see this'; run foo;
echo $?
bar
1
=> setenv foo 'echo bar ; exit 1 ; echo should not see this'; run foo;
echo $?
bar
1
=> setenv foo 'echo bar ; exit 0 ; echo should not see this'; run foo;
echo $?
bar
0
=> setenv foo 'echo bar ; exit -1 ; echo should not see this'; run foo;
echo $?
bar
0
=> setenv foo 'echo bar ; exit -2 ; echo should not see this'; run foo;
echo $?
bar
0
=> setenv foo 'echo bar ; exit ; echo should not see this'; run foo;
echo $?
bar
0
Reported-by: Adrian Vovk
Signed-off-by: Hector Palacios
---
common/cli_hush.c | 4
1 file changed, 4 insertions(+)
diff --git a/common/cli_hush.c b/common/cli_hush.c
index 1467ff81b35b..9fe8b87e02d7 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -1902,6 +1902,10 @@ static int run_list_real(struct pipe *pi)
last_return_code = -rcode - 2;
return -2; /* exit */
}
+ if (!strcmp(pi->progs->argv[0], "exit")) {
+ last_return_code = rcode;
+ return rcode; /* exit */
+ }
last_return_code=(rcode == 0) ? 0 : 1;
#endif
#ifndef __U_BOOT__
Re: [PATCH v9 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface
Hi Heinrhch,
On Fri, 18 Nov 2022 at 08:07, Heinrich Schuchardt wrote:
>
> On 11/16/22 11:28, Masahisa Kojima wrote:
> > This commit adds the menu-driven UEFI Secure Boot Key
> > enrollment interface. User can enroll PK, KEK, db
> > and dbx by selecting file.
> > Only the signed EFI Signature List(s) with an authenticated
> > header, typically '.auth' file, is accepted.
> >
> > To clear the PK, KEK, db and dbx, user needs to enroll the null key
> > signed by PK or KEK.
> >
> > Signed-off-by: Masahisa Kojima
> > Reviewed-by: Ilias Apalodimas
> > ---
> > Changes in v9:
> > - move file size check, set ret = EFI_INVALID_PARAMETER
> >
> > Changes in v8:
> > - fix missing efi_file_close_int() call
> >
> > Changes in v7:
> > - only accept .auth file.
> > - remove creating time based authenticated variable
> > - update commit message
> > - use efi_file_size()
> >
> > Changes in v6:
> > - use efi_secure_boot_enabled()
> > - replace with WIN_CERT_REVISION_2_0 from pe.h
> > - call efi_build_signature_store() to check the valid EFI Signature List
> > - update comment
> >
> > Changes in v4:
> > - add CONFIG_EFI_MM_COMM_TEE dependency
> > - fix error handling
> >
> > Changes in v3:
> > - fix error handling
> >
> > Changes in v2:
> > - allow to enroll .esl file
> > - fix typos
> > - add function comments
> >
> > cmd/Makefile | 5 +
> > cmd/eficonfig.c | 3 +
> > cmd/eficonfig_sbkey.c | 246 ++
> > include/efi_config.h | 5 +
> > 4 files changed, 259 insertions(+)
> > create mode 100644 cmd/eficonfig_sbkey.c
> >
> > diff --git a/cmd/Makefile b/cmd/Makefile
> > index 2444d116c0..0b6a96c1d9 100644
> > --- a/cmd/Makefile
> > +++ b/cmd/Makefile
> > @@ -66,6 +66,11 @@ obj-$(CONFIG_CMD_EEPROM) += eeprom.o
> > obj-$(CONFIG_EFI) += efi.o
> > obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o
> > obj-$(CONFIG_CMD_EFICONFIG) += eficonfig.o
> > +ifdef CONFIG_CMD_EFICONFIG
> > +ifdef CONFIG_EFI_MM_COMM_TEE
> > +obj-$(CONFIG_EFI_SECURE_BOOT) += eficonfig_sbkey.o
> > +endif
> > +endif
> > obj-$(CONFIG_CMD_ELF) += elf.o
> > obj-$(CONFIG_CMD_EROFS) += erofs.o
> > obj-$(CONFIG_HUSH_PARSER) += exit.o
> > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> > index 12babb76c2..d79194794b 100644
> > --- a/cmd/eficonfig.c
> > +++ b/cmd/eficonfig.c
> > @@ -2435,6 +2435,9 @@ static const struct eficonfig_item
> > maintenance_menu_items[] = {
> > {"Edit Boot Option", eficonfig_process_edit_boot_option},
> > {"Change Boot Order", eficonfig_process_change_boot_order},
> > {"Delete Boot Option", eficonfig_process_delete_boot_option},
> > +#if (CONFIG_IS_ENABLED(EFI_SECURE_BOOT) &&
> > CONFIG_IS_ENABLED(EFI_MM_COMM_TEE))
> > + {"Secure Boot Configuration", eficonfig_process_secure_boot_config},
> > +#endif
> > {"Quit", eficonfig_process_quit},
> > };
> >
> > diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c
> > new file mode 100644
> > index 00..e9aaf76bf8
> > --- /dev/null
> > +++ b/cmd/eficonfig_sbkey.c
> > @@ -0,0 +1,246 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Menu-driven UEFI Secure Boot Key Maintenance
> > + *
> > + * Copyright (c) 2022 Masahisa Kojima, Linaro Limited
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +enum efi_sbkey_signature_type {
> > + SIG_TYPE_X509 = 0,
> > + SIG_TYPE_HASH,
> > + SIG_TYPE_CRL,
> > + SIG_TYPE_RSA2048,
> > +};
> > +
> > +struct eficonfig_sigtype_to_str {
> > + efi_guid_t sig_type;
> > + char *str;
> > + enum efi_sbkey_signature_type type;
> > +};
> > +
> > +static const struct eficonfig_sigtype_to_str sigtype_to_str[] = {
> > + {EFI_CERT_X509_GUID,"X509",
> > SIG_TYPE_X509},
> > + {EFI_CERT_SHA256_GUID, "SHA256",
> > SIG_TYPE_HASH},
> > + {EFI_CERT_X509_SHA256_GUID, "X509_SHA256 CRL", SIG_TYPE_CRL},
> > + {EFI_CERT_X509_SHA384_GUID, "X509_SHA384 CRL", SIG_TYPE_CRL},
> > + {EFI_CERT_X509_SHA512_GUID, "X509_SHA512 CRL", SIG_TYPE_CRL},
> > + /* U-Boot does not support the following signature types */
> > +/* {EFI_CERT_RSA2048_GUID, "RSA2048",
> > SIG_TYPE_RSA2048}, */
> > +/* {EFI_CERT_RSA2048_SHA256_GUID, "RSA2048_SHA256",
> > SIG_TYPE_RSA2048}, */
> > +/* {EFI_CERT_SHA1_GUID,"SHA1",
> > SIG_TYPE_HASH}, */
> > +/* {EFI_CERT_RSA2048_SHA_GUID, "RSA2048_SHA",
> > SIG_TYPE_RSA2048 }, */
> > +/* {EFI_CERT_SHA224_GUID, "SHA224",
> > SIG_TYPE_HASH}, */
> > +/* {EFI_CERT_SHA384_GUID, "SHA384",
> > SIG_TYPE_HASH}, */
> > +/* {EFI_CERT_SHA512_GUID, "SHA512",
> > SIG_TYPE_HASH}, */
> > +};
> > +
> > +/**
> > + * file_have_auth_header() - check file has
