U-Boot Digest, Vol 165, Issue 29
Walter Heck
walterheck at gmail.com
Mon Feb 14 00:30:35 CET 2022
Unsubscribe
On Sun, Feb 13, 2022, 12:00 <u-boot-request at lists.denx.de> wrote:
> Send U-Boot mailing list submissions to
> u-boot at lists.denx.de
>
> To subscribe or unsubscribe via the World Wide Web, visit
> https://lists.denx.de/listinfo/u-boot
> or, via email, send a message with subject or body 'help' to
> u-boot-request at lists.denx.de
>
> You can reach the person managing the list at
> u-boot-owner at lists.denx.de
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of U-Boot digest..."
>
>
> Today's Topics:
>
> 1. Re: [PATCH 2/2] net: phy: mv88e6352: Fix
> miiphy_read/miiphy_write return value checks (Ramon Fried)
> 2. [PATCH 1/2] arm: omap3: Cleanup sys_info to fit OMAP3 booting
> with LTO (Adam Ford)
> 3. [PATCH 2/2] arm: omap3: Make some memory functions static and
> clean headers (Adam Ford)
> 4. [PATCH] tools/zynqmp_pm_cfg_obj_convert.py: fix build with
> Vivado 2021.x (Luca Ceresoli)
> 5. [PATCH] usb: ehci-omap: Drop dead code (Adam Ford)
> 6. Re: [ANN] U-Boot v2022.04-rc1 released (Andy Shevchenko)
> 7. Re: [ANN] U-Boot v2022.04-rc1 released (Michal Simek)
> 8. Re: [PATCH] image: Control FIT signature verification at
> runtime (Alex G.)
> 9. Re: Problem IMX8MN booting legacy kernel and mipi display
> (Michael Nazzareno Trimarchi)
> 10. [PATCH] tools: kwbimage: Fix dumping DATA registers for v0
> images (Pali Roh?r)
> 11. [PATCH] tools: mkimage/dumpimage: Allow to use -l with -T
> (Pali Roh?r)
> 12. [PATCH 1/7] microblaze: exception: move privileged
> instruction exception out of v5 ifdef (Ovidiu Panait)
> 13. [PATCH 5/7] microblaze: exception: move unaligned access
> printfs inside switch case (Ovidiu Panait)
> 14. [PATCH 3/7] microblaze: exception: fix delay slot exception
> handling (Ovidiu Panait)
> 15. [PATCH 7/7] microblaze: exception: drop user exception
> support (Ovidiu Panait)
> 16. [PATCH 6/7] microblaze: exception: fix unaligned data access
> register mask (Ovidiu Panait)
> 17. [PATCH 2/7] microblaze: exception: migrate MICROBLAZE_V5 to
> Kconfig (Ovidiu Panait)
> 18. [PATCH 4/7] microblaze: exception: fix return address for
> delay slot exceptions (Ovidiu Panait)
> 19. Re: [PATCH v4 1/2] efi_loader: use
> efi_update_capsule_firmware() for capsule on disk
> (Heinrich Schuchardt)
> 20. Re: [PATCH v4 2/2] efi_loader: Reset system after
> CapsuleUpdate on disk (Heinrich Schuchardt)
> 21. Re: [PATCH 3/3] efi_loader: add menu-driven UEFI Boot
> Variable maintenance (Heinrich Schuchardt)
> 22. Re: [PATCH 1/3] efi_loader: add menu-driven boot device
> selection (Heinrich Schuchardt)
> 23. Re: [PATCH 2/3] lib/charset: add u16_strcat() function
> (Heinrich Schuchardt)
> 24. Re: [PATCH v4 2/2] efi_loader: Reset system after
> CapsuleUpdate on disk (Heinrich Schuchardt)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Sat, 12 Feb 2022 13:50:05 +0200
> From: Ramon Fried <rfried.dev at gmail.com>
> To: Daniel Klauer <daniel.klauer at gin.de>
> Cc: U-Boot Mailing List <u-boot at lists.denx.de>
> Subject: Re: [PATCH 2/2] net: phy: mv88e6352: Fix
> miiphy_read/miiphy_write return value checks
> Message-ID:
> <
> CAGi-RU+mJcJ4aceUXQWUL-v80zwUdOaoZC--zeLL6C64SPcB9A at mail.gmail.com>
> Content-Type: text/plain; charset="UTF-8"
>
> On Wed, Feb 9, 2022 at 5:41 PM Daniel Klauer <daniel.klauer at gin.de> wrote:
> >
> > The miiphy_read/miiphy_write functions return 1 on error, not -errno.
> Why don't you just fix the miiphy_read/miiphy_write functions ?
>
>
> ------------------------------
>
> Message: 2
> Date: Sat, 12 Feb 2022 06:12:40 -0600
> From: Adam Ford <aford173 at gmail.com>
> To: u-boot at lists.denx.de
> Cc: trini at konsulko.com, aford at beaconembedded.com, Adam Ford
> <aford173 at gmail.com>
> Subject: [PATCH 1/2] arm: omap3: Cleanup sys_info to fit OMAP3 booting
> with LTO
> Message-ID: <20220212121241.1655425-1-aford173 at gmail.com>
>
> With LTO enabled, some functions appear to be optimized in a
> way that causes hanging on some OMAP3 boards after some
> unrelated patches were applied. The solution appears to make
> several functions __used. There also appears be to be some
> dead code, so remove it while cleaning this up.
>
> This has been tested on a general purpose OMAP3530, DM3730,
> and AM3517.
>
> Signed-off-by: Adam Ford <aford173 at gmail.com>
>
> diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h
> b/arch/arm/include/asm/arch-omap3/sys_proto.h
> index a6e9ff84aa..e7078a32db 100644
> --- a/arch/arm/include/asm/arch-omap3/sys_proto.h
> +++ b/arch/arm/include/asm/arch-omap3/sys_proto.h
> @@ -45,16 +45,12 @@ void gpmc_init(void);
> void enable_gpmc_cs_config(const u32 *gpmc_config, const struct gpmc_cs
> *cs,
> u32 base, u32 size);
> void set_gpmc_cs0(int flash_type);
> -
> void watchdog_init(void);
> void set_muxconf_regs(void);
> -
> u32 get_cpu_family(void);
> u32 get_cpu_rev(void);
> -u32 get_sku_id(void);
> u32 is_gpmc_muxed(void);
> u32 get_gpmc0_type(void);
> -u32 get_gpmc0_width(void);
> u32 is_running_in_sdram(void);
> u32 is_running_in_sram(void);
> u32 is_running_in_flash(void);
> diff --git a/arch/arm/mach-omap2/omap3/sys_info.c
> b/arch/arm/mach-omap2/omap3/sys_info.c
> index ac72633c20..5f535e2782 100644
> --- a/arch/arm/mach-omap2/omap3/sys_info.c
> +++ b/arch/arm/mach-omap2/omap3/sys_info.c
> @@ -55,7 +55,7 @@ void omap_die_id(unsigned int *die_id)
> /******************************************
> * get_cpu_type(void) - extract cpu info
> ******************************************/
> -u32 get_cpu_type(void)
> +static u32 get_cpu_type(void)
> {
> return readl(&ctrl_base->ctrl_omap_stat);
> }
> @@ -64,7 +64,7 @@ u32 get_cpu_type(void)
> * get_cpu_id(void) - extract cpu id
> * returns 0 for ES1.0, cpuid otherwise
> ******************************************/
> -u32 get_cpu_id(void)
> +static u32 get_cpu_id(void)
> {
> struct ctrl_id *id_base;
> u32 cpuid = 0;
> @@ -89,7 +89,7 @@ u32 get_cpu_id(void)
> /******************************************
> * get_cpu_family(void) - extract cpu info
> ******************************************/
> -u32 get_cpu_family(void)
> +__used u32 get_cpu_family(void)
> {
> u16 hawkeye;
> u32 cpu_family;
> @@ -119,7 +119,7 @@ u32 get_cpu_family(void)
> /******************************************
> * get_cpu_rev(void) - extract version info
> ******************************************/
> -u32 get_cpu_rev(void)
> +__used u32 get_cpu_rev(void)
> {
> u32 cpuid = get_cpu_id();
>
> @@ -132,41 +132,12 @@ u32 get_cpu_rev(void)
> /*****************************************************************
> * get_sku_id(void) - read sku_id to get info on max clock rate
> *****************************************************************/
> -u32 get_sku_id(void)
> +static u32 get_sku_id(void)
> {
> struct ctrl_id *id_base = (struct ctrl_id *)OMAP34XX_ID_L4_IO_BASE;
> return readl(&id_base->sku_id) & SKUID_CLK_MASK;
> }
>
>
> -/***************************************************************************
> - * get_gpmc0_base() - Return current address hardware will be
> - * fetching from. The below effectively gives what is correct, its a
> bit
> - * mis-leading compared to the TRM. For the most general case the mask
> - * needs to be also taken into account this does work in practice.
> - * - for u-boot we currently map:
> - * -- 0 to nothing,
> - * -- 4 to flash
> - * -- 8 to enent
> - * -- c to wifi
> -
> ****************************************************************************/
> -u32 get_gpmc0_base(void)
> -{
> - u32 b;
> -
> - b = readl(&gpmc_cfg->cs[0].config7);
> - b &= 0x1F; /* keep base [5:0] */
> - b = b << 24; /* ret 0x0b000000 */
> - return b;
> -}
> -
> -/*******************************************************************
> - * get_gpmc0_width() - See if bus is in x8 or x16 (mainly for nand)
> - *******************************************************************/
> -u32 get_gpmc0_width(void)
> -{
> - return WIDTH_16BIT;
> -}
> -
> /*************************************************************************
> * get_board_rev() - setup to pass kernel board revision information
> * returns:(bit[0-3] sub version, higher bit[7-4] is higher version)
> --
> 2.32.0
>
>
>
> ------------------------------
>
> Message: 3
> Date: Sat, 12 Feb 2022 06:12:41 -0600
> From: Adam Ford <aford173 at gmail.com>
> To: u-boot at lists.denx.de
> Cc: trini at konsulko.com, aford at beaconembedded.com, Adam Ford
> <aford173 at gmail.com>
> Subject: [PATCH 2/2] arm: omap3: Make some memory functions static and
> clean headers
> Message-ID: <20220212121241.1655425-2-aford173 at gmail.com>
>
> There are a few memory functions for both the emif4 (AM3517)
> and sdrc (OMAP35/DM37) code that can be defined as static,
> because those functions are not used externally. Make them
> static and clean up some of the corresponding headers.
>
> Signed-off-by: Adam Ford <aford173 at gmail.com>
>
> diff --git a/arch/arm/include/asm/arch-omap3/mem.h
> b/arch/arm/include/asm/arch-omap3/mem.h
> index 7adc134a75..569779c55e 100644
> --- a/arch/arm/include/asm/arch-omap3/mem.h
> +++ b/arch/arm/include/asm/arch-omap3/mem.h
> @@ -480,7 +480,6 @@ void mem_init(void);
> u32 is_mem_sdr(void);
> u32 mem_ok(u32 cs);
>
> -u32 get_sdr_cs_size(u32);
> u32 get_sdr_cs_offset(u32);
>
> #endif /* __ASSEMBLY__ */
> diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h
> b/arch/arm/include/asm/arch-omap3/sys_proto.h
> index e7078a32db..3e6335c5fa 100644
> --- a/arch/arm/include/asm/arch-omap3/sys_proto.h
> +++ b/arch/arm/include/asm/arch-omap3/sys_proto.h
> @@ -33,11 +33,8 @@ struct board_sdrc_timings {
> void prcm_init(void);
> void per_clocks_enable(void);
> void ehci_clocks_enable(void);
> -
> void memif_init(void);
> void sdrc_init(void);
> -void do_sdrc_init(u32, u32);
> -
> void get_board_mem_timings(struct board_sdrc_timings *timings);
> int identify_nand_chip(int *mfr, int *id);
> void emif4_init(void);
> @@ -60,12 +57,10 @@ void invalidate_dcache(u32);
> u32 wait_on_value(u32, u32, void *, u32);
> void cancel_out(u32 *num, u32 *den, u32 den_limit);
> void sdelay(unsigned long);
> -void make_cs1_contiguous(void);
> int omap_nand_switch_ecc(uint32_t, uint32_t);
> void power_init_r(void);
> void do_omap3_emu_romcode_call(u32 service_id, u32 parameters);
> void omap3_set_aux_cr_secure(u32 acr);
> u32 warm_reset(void);
> -
> void save_omap_boot_params(void);
> #endif
> diff --git a/arch/arm/mach-omap2/omap3/emif4.c
> b/arch/arm/mach-omap2/omap3/emif4.c
> index df6e9ce1d6..d7d779819b 100644
> --- a/arch/arm/mach-omap2/omap3/emif4.c
> +++ b/arch/arm/mach-omap2/omap3/emif4.c
> @@ -35,7 +35,7 @@ u32 is_mem_sdr(void)
> * get_sdr_cs_size -
> * - Get size of chip select 0/1
> */
> -u32 get_sdr_cs_size(u32 cs)
> +static u32 get_sdr_cs_size(u32 cs)
> {
> u32 size = 0;
>
> diff --git a/arch/arm/mach-omap2/omap3/sdrc.c
> b/arch/arm/mach-omap2/omap3/sdrc.c
> index 4d85b1dee9..07f534a60b 100644
> --- a/arch/arm/mach-omap2/omap3/sdrc.c
> +++ b/arch/arm/mach-omap2/omap3/sdrc.c
> @@ -44,13 +44,28 @@ u32 is_mem_sdr(void)
> return 0;
> }
>
> +/*
> + * get_sdr_cs_size -
> + * - Get size of chip select 0/1
> + */
> +static u32 get_sdr_cs_size(u32 cs)
> +{
> + u32 size;
> +
> + /* get ram size field */
> + size = readl(&sdrc_base->cs[cs].mcfg) >> 8;
> + size &= 0x3FF; /* remove unwanted bits */
> + size <<= 21; /* multiply by 2 MiB to find size in MB */
> + return size;
> +}
> +
> /*
> * make_cs1_contiguous -
> * - When we have CS1 populated we want to have it mapped after cs0 to
> allow
> * command line mem=xyz use all memory with out discontinuous support
> * compiled in. We could do it in the ATAG, but there really is two
> banks...
> */
> -void make_cs1_contiguous(void)
> +static void make_cs1_contiguous(void)
> {
> u32 size, a_add_low, a_add_high;
>
> @@ -62,22 +77,6 @@ void make_cs1_contiguous(void)
>
> }
>
> -
> -/*
> - * get_sdr_cs_size -
> - * - Get size of chip select 0/1
> - */
> -u32 get_sdr_cs_size(u32 cs)
> -{
> - u32 size;
> -
> - /* get ram size field */
> - size = readl(&sdrc_base->cs[cs].mcfg) >> 8;
> - size &= 0x3FF; /* remove unwanted bits */
> - size <<= 21; /* multiply by 2 MiB to find size in MB */
> - return size;
> -}
> -
> /*
> * get_sdr_cs_offset -
> * - Get offset of cs from cs0 start
> @@ -128,7 +127,7 @@ static void write_sdrc_timings(u32 cs, struct
> sdrc_actim *sdrc_actim_base,
> * true and a possible 2nd time depending on memory configuration from
> * stack+global context.
> */
> -void do_sdrc_init(u32 cs, u32 early)
> +static void do_sdrc_init(u32 cs, u32 early)
> {
> struct sdrc_actim *sdrc_actim_base0, *sdrc_actim_base1;
> struct board_sdrc_timings timings;
> --
> 2.32.0
>
>
>
> ------------------------------
>
> Message: 4
> Date: Sat, 12 Feb 2022 13:51:21 +0100
> From: Luca Ceresoli <luca at lucaceresoli.net>
> To: u-boot at lists.denx.de
> Cc: Luca Ceresoli <luca at lucaceresoli.net>, Michal Simek
> <michal.simek at xilinx.com>, Giulio Benetti
> <giulio.benetti at benettiengineering.com>, Neal Frager
> <nealf at xilinx.com>
> Subject: [PATCH] tools/zynqmp_pm_cfg_obj_convert.py: fix build with
> Vivado 2021.x
> Message-ID: <20220212125121.3398547-1-luca at lucaceresoli.net>
>
> This tool fails with a pm_cfg_obj.c file generated by Vitis 2021.2. This is
> because that version of Vitis added the PM_CONFIG_OBJECT_TYPE_BASE that was
> not previously generated, thus the script does not implement it.
>
> Reported-by: Neal Frager <nealf at xilinx.com>
> [report:
> https://lists.buildroot.org/pipermail/buildroot/2022-February/636639.html]
> Signed-off-by: Luca Ceresoli <luca at lucaceresoli.net>
> ---
> tools/zynqmp_pm_cfg_obj_convert.py | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/zynqmp_pm_cfg_obj_convert.py
> b/tools/zynqmp_pm_cfg_obj_convert.py
> index 0a44710e1e14..239991a5263c 100755
> --- a/tools/zynqmp_pm_cfg_obj_convert.py
> +++ b/tools/zynqmp_pm_cfg_obj_convert.py
> @@ -244,6 +244,8 @@ pm_define = {
>
> 'SUSPEND_TIMEOUT' : 0xFFFFFFFF,
>
> + 'PM_CONFIG_OBJECT_TYPE_BASE' : 0x1,
> +
> 'PM_CONFIG_IPI_PSU_CORTEXA53_0_MASK' : 0x00000001,
> 'PM_CONFIG_IPI_PSU_CORTEXR5_0_MASK' : 0x00000100,
> 'PM_CONFIG_IPI_PSU_CORTEXR5_1_MASK' : 0x00000200,
> --
> 2.25.1
>
>
>
> ------------------------------
>
> Message: 5
> Date: Sat, 12 Feb 2022 08:26:40 -0600
> From: Adam Ford <aford173 at gmail.com>
> To: u-boot at lists.denx.de
> Cc: marex at denx.de, trini at konsulko.com, Adam Ford <
> aford173 at gmail.com>
> Subject: [PATCH] usb: ehci-omap: Drop dead code
> Message-ID: <20220212142640.1669851-1-aford173 at gmail.com>
>
> omap_ehci_hcd_stop appears to be dead code, and omap_ehci_hcd_init
> is only called by the probe function, so it can be static to that
> function. Remove both from the header along with some additional
> checking for DM_USB.
>
> Signed-off-by: Adam Ford <aford173 at gmail.com>
>
> diff --git a/arch/arm/include/asm/ehci-omap.h
> b/arch/arm/include/asm/ehci-omap.h
> index f970bba937..2b51b5eb99 100644
> --- a/arch/arm/include/asm/ehci-omap.h
> +++ b/arch/arm/include/asm/ehci-omap.h
> @@ -123,17 +123,4 @@ struct omap_ehci {
> u32 insreg08; /* 0xb0 */
> };
>
> -#if !CONFIG_IS_ENABLED(DM_USB) || !CONFIG_IS_ENABLED(OF_CONTROL)
> -/*
> - * FIXME: forward declaration of this structs needed because omap got the
> - * ehci implementation backwards. move out ehci_hcd_x from board files
> - */
> -struct ehci_hccr;
> -struct ehci_hcor;
> -
> -int omap_ehci_hcd_init(int index, struct omap_usbhs_board_data
> *usbhs_pdata,
> - struct ehci_hccr **hccr, struct ehci_hcor **hcor);
> -int omap_ehci_hcd_stop(void);
> -#endif
> -
> #endif /* _OMAP_COMMON_EHCI_H_ */
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index d5facf10e1..d34c0add4a 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -163,27 +163,12 @@ static inline void omap_ehci_phy_reset(int on, int
> delay)
> #define omap_ehci_phy_reset(on, delay) do {} while (0)
> #endif
>
> -/* Reset is needed otherwise the kernel-driver will throw an error. */
> -int omap_ehci_hcd_stop(void)
> -{
> - debug("Resetting OMAP EHCI\n");
> - omap_ehci_phy_reset(1, 0);
> -
> - if (omap_uhh_reset() < 0)
> - return -1;
> -
> - if (omap_ehci_tll_reset() < 0)
> - return -1;
> -
> - return 0;
> -}
> -
> /*
> * Initialize the OMAP EHCI controller and PHY.
> * Based on "drivers/usb/host/ehci-omap.c" from Linux 3.1
> * See there for additional Copyrights.
> */
> -int omap_ehci_hcd_init(int index, struct omap_usbhs_board_data
> *usbhs_pdata)
> +static int omap_ehci_hcd_init(int index, struct omap_usbhs_board_data
> *usbhs_pdata)
> {
> int ret;
> unsigned int i, reg = 0, rev = 0;
> --
> 2.32.0
>
>
>
> ------------------------------
>
> Message: 6
> Date: Sat, 12 Feb 2022 17:02:00 +0200
> From: Andy Shevchenko <andy.shevchenko at gmail.com>
> To: Andy Shevchenko <andriy.shevchenko at intel.com>
> Cc: Tom Rini <trini at konsulko.com>, Simon Glass <sjg at chromium.org>, Bin
> Meng <bmeng.cn at gmail.com>, U-Boot Mailing List
> <u-boot at lists.denx.de>, u-boot-custodians at lists.denx.de,
> u-boot-board-maintainers at lists.denx.de
> Subject: Re: [ANN] U-Boot v2022.04-rc1 released
> Message-ID:
> <
> CAHp75VfgWnGTjOUsJ05DfMU0qFQtrwr9oBQCBC9TLi8h3LHN+g at mail.gmail.com>
> Content-Type: text/plain; charset="UTF-8"
>
> On Fri, Feb 11, 2022 at 9:15 PM Andy Shevchenko
> <andy.shevchenko at gmail.com> wrote:
> > On Fri, Feb 11, 2022 at 8:46 PM Andy Shevchenko
> > <andy.shevchenko at gmail.com> wrote:
> > > On Fri, Feb 11, 2022 at 8:31 PM Andy Shevchenko
> > > <andriy.shevchenko at intel.com> wrote:
> > > > On Mon, Jan 31, 2022 at 05:59:30PM -0500, Tom Rini wrote:
>
> ...
>
> > > git bisect bad 379d3c1fd6aa490b1ad5697525cfc89b615cf25a
> > > # first bad commit: [379d3c1fd6aa490b1ad5697525cfc89b615cf25a] x86:
> > > Move FACP table into separate functions
> > > u-boot((379d3c1fd6aa...)|BISECTING)$
> >
> > For the record, these two
> > acpi: Move MCFG implementation to common lib
> > arch: x86: lib: acpi_table: Fix MCFG entries
> >
> > do not help.
> >
> > > Irony is that I have reviewed it, but that time I was busy and
> couldn't test.
> > >
> > > > Simon, can you prioritize setting up Edison to make it available for
> tests?
> >
> > So, I'm open to testing any other suggestions.
>
> Meanwhile I will try to revert and if it works and no other solution
> comes, I will send it out.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
> ------------------------------
>
> Message: 7
> Date: Sat, 12 Feb 2022 17:58:29 +0100
> From: Michal Simek <monstr at monstr.eu>
> To: Tom Rini <trini at konsulko.com>, u-boot at lists.denx.de, Simon Glass
> <sjg at chromium.org>
> Cc: u-boot-custodians at lists.denx.de,
> u-boot-board-maintainers at lists.denx.de, Michal Simek
> <michal.simek at xilinx.com>
> Subject: Re: [ANN] U-Boot v2022.04-rc1 released
> Message-ID: <b7383793-f671-0766-1309-14b75f031ac0 at monstr.eu>
> Content-Type: text/plain; charset=UTF-8; format=flowed
>
> Hi,
>
> On 1/31/22 23:59, Tom Rini wrote:
> > Hey all,
> >
> > It's release day and so here's v2022.04-rc1. While there's much in here
> > that needed to come in, there's a few big things outstanding, including
> > but not limited to i.MX and layerscape syncs and further sunxi changes.
> >
> > In terms of a changelog,
> > git log --merges v2022.01..v2022.04-rc1
> > contains what I've pulled but as always, better PR messages and tags
> > will provide better results here.
> >
> > So we're now looking at regular releases every other Monday, and with
> > final release on April 4th, 2022. Thanks all!
> >
>
> I also found that this patch break ZynqMP boards. I didn't have a time to
> dig
> into it more. Just did a bisect to find out what's going on.
> 985503439762 ("fdt: Don't call board_fdt_blob_setup() without OF_BOARD")
>
> Thanks,
> Michal
>
> --
> Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel - Xilinx Microblaze
> Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
> U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
>
>
>
> ------------------------------
>
> Message: 8
> Date: Sat, 12 Feb 2022 12:55:24 -0600
> From: "Alex G." <mr.nuke.me at gmail.com>
> To: Andrew Jeffery <andrew at aj.id.au>, u-boot at lists.denx.de
> Cc: sjg at chromium.org, chiawei_wang at aspeedtech.com, joel at jms.id.au,
> openbmc at lists.ozlabs.org
> Subject: Re: [PATCH] image: Control FIT signature verification at
> runtime
> Message-ID: <97430094-7d2a-432b-a121-96812fac87f9 at gmail.com>
> Content-Type: text/plain; charset=UTF-8; format=flowed
>
> On 1/30/22 21:41, Andrew Jeffery wrote:
> > Some platform designs include support for disabling secure-boot via a
> > jumper on the board. Sometimes this control can be separate from the
> > mechanism enabling the root-of-trust for the platform. Add support for
> > this latter scenario by allowing boards to implement
> > board_fit_image_require_verfied(), which is then invoked in the usual
> > FIT verification paths.
> >
> > Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> > ---
> > Hi,
> >
> > This patch is extracted from and motivated by a series adding run-time
> > control of FIT signature verification to u-boot in OpenBMC:
> >
> > https://lore.kernel.org/openbmc/20220131012538.73021-1-andrew@aj.id.au/
> >
> > Unfortunately the OpenBMC u-boot tree is quite a way behind on tracking
> > upstream and contains a bunch of out-of-tree work as well. As such I'm
> > looking to upstream the couple of changes that make sense against
> > master.
>
> I don't understand the practical use of a mechanism to disable security
> if secure boot was enabled in the first place. Sure it can be
> technically done, but why is this not a bad idea?
>
> > Please take a look!
> >
> > Andrew
> >
> > boot/Kconfig | 8 ++++++++
> > boot/image-fit.c | 21 +++++++++++++++++----
> > include/image.h | 9 +++++++++
> > 3 files changed, 34 insertions(+), 4 deletions(-)
> >
> > diff --git a/boot/Kconfig b/boot/Kconfig
> > index c8d5906cd304..ec413151fd5a 100644
> > --- a/boot/Kconfig
> > +++ b/boot/Kconfig
> > @@ -78,6 +78,14 @@ config FIT_SIGNATURE
> > format support in this case, enable it using
> > CONFIG_LEGACY_IMAGE_FORMAT.
> >
> > +if FIT_SIGNATURE
> > +config FIT_RUNTIME_SIGNATURE
> > + bool "Control verification of FIT uImages at runtime"
> > + help
> > + This option allows board support to disable verification of
> > + signatures at runtime, for example through the state of a GPIO.
>
> The commit message does a much nicer job explaining this option, even
> though it is this kconfig help text that almost all users will ever see.
>
> > +endif # FIT_SIGNATURE
> > +
> > config FIT_SIGNATURE_MAX_SIZE
> > hex "Max size of signed FIT structures"
> > depends on FIT_SIGNATURE
> > diff --git a/boot/image-fit.c b/boot/image-fit.c
> > index f01cafe4e277..919dbfa4ee1d 100644
> > --- a/boot/image-fit.c
> > +++ b/boot/image-fit.c
> > @@ -1308,6 +1308,14 @@ static int fit_image_check_hash(const void *fit,
> int noffset, const void *data,
> > return 0;
> > }
> >
> > +#ifndef __weak
> > +#define __weak
> > +#endif
>
> What?
>
> > +__weak int board_fit_image_require_verified(void)
> > +{
> > + return 1;
> > +}
> > +
>
> I caution against having any platform security related functionality as
> a weak function. Did I get the right function, or something else with
> the same name was compiled that returns 0 and silently disables security
> in my platform?
>
> I think a weak function in this context is a source of confusion and
> future bugs. You could instead require the symbol to be defined if and
> only if the appropriate kconfig is selected. Symbol not defined ->
> compiler error. Symbol defined twice -> compiler error. Solves the
> issues in the preceding paragraph.
>
> [snip]
>
> > diff --git a/include/image.h b/include/image.h
> > index 97e5f2eb24d6..98900c2e839b 100644
> > --- a/include/image.h
> > +++ b/include/image.h
> > @@ -1173,6 +1173,15 @@ int calculate_hash(const void *data, int
> data_len, const char *algo,
> > # define FIT_IMAGE_ENABLE_VERIFY CONFIG_IS_ENABLED(FIT_SIGNATURE)
> > #endif
> >
> > +/*
> > + * Further, allow run-time control of verification, e.g. via a jumper
> > + */
> > +#if defined(CONFIG_FIT_RUNTIME_SIGNATURE)
> > +# define fit_image_require_verified()
> board_fit_image_require_verified()
> > +#else
> > +# define fit_image_require_verified() FIT_IMAGE_ENABLE_VERIFY
> > +#endif
> > +
>
> image.h is also used for host code. When only changing target code
> behavior, there should be a very good reason to modify common code. I'm
> not convinced that threshold has been met.
>
> My second concern here is subjective: I don't like functions defined as
> macros, further depending on config selections. It makes many code
> parsers and IDEs poop their pantaloons. It makes u-boot harder to work
> with as a result. I suggest finding a way to turn this into a static
> inline.
>
> Alex
>
>
> ------------------------------
>
> Message: 9
> Date: Sat, 12 Feb 2022 20:27:17 +0100
> From: Michael Nazzareno Trimarchi <michael at amarulasolutions.com>
> To: Tim Harvey <tharvey at gateworks.com>
> Cc: Adam Ford <aford173 at gmail.com>, Fabio Estevam
> <festevam at gmail.com>, dl-uboot-imx <uboot-imx at nxp.com>,
> U-Boot-Denx
> <u-boot at lists.denx.de>, Stefano Babic <sbabic at denx.de>
> Subject: Re: Problem IMX8MN booting legacy kernel and mipi display
> Message-ID:
> <CAOf5uwkKG6mc0ek+H5vLn=
> KwWU0gghgDYGjV0beMJ1+8dfL04Q at mail.gmail.com>
> Content-Type: text/plain; charset="UTF-8"
>
> Hi Tim
>
> You are using the imx8mm. I have problems on imx8mn. It's like some
> part is not implemented. Time to time power domain timeout if I
> enabled
> mipi pipeline
>
> Michael
>
> On Wed, Feb 2, 2022 at 4:54 PM Tim Harvey <tharvey at gateworks.com> wrote:
> >
> > On Wed, Feb 2, 2022 at 5:04 AM Michael Nazzareno Trimarchi
> > <michael at amarulasolutions.com> wrote:
> > >
> > > Hi Adam
> > >
> > > On Wed, Feb 2, 2022 at 2:02 PM Adam Ford <aford173 at gmail.com> wrote:
> > > >
> > > > On Wed, Feb 2, 2022 at 6:24 AM Michael Nazzareno Trimarchi
> > > > <michael at amarulasolutions.com> wrote:
> > > > >
> > > > > Hi all
> > > > >
> > > > > I have now able to pack those combination and all of them look ok
> > > > >
> > > > > - mainline uboot
> > > > > - optee 3.15.0
> > > > > - atf 2.5
> > > > > - mainline kernel
> > > > >
> > > > > - mainline uboot
> > > > > - optee 3.15.0
> > > > > - imx-atf rel_imx_5.4.70_2.3.2
> > > > > - Linux rel_imx_5.4.70_2.3.2
> > > > >
> > > > > I have a setup where I can test some peripherals. I can run an
> optee
> > > > > test on both combinations
> > > > > The big issue I have it's the DSIM display it's not working and
> > > > > crashes the kernel using the rel_imx_5.4.70_2.3.2 nxp version. The
> > > > > mipi
> > > > > randomly deadlock the booting. If I don't register the mipi part
> it's
> > > > > ok. Can someone have an idea?
> > > >
> > > > Are you using the Linux rel_imx_5.4.70_2.3.2 with imx-atf
> rel_imx_5.4.70_2.3.2?
> > > >
> > > > The NXP kernel expects the NXP ATF code. The power-domain drivers in
> > > > the NXP branch make secure calls to ATF which enable/disable the
> > > > power-domains which control a bunch of peripherals like DSI, CSI, USB
> > > > and GPU. If you're using the mainline ATF, I don't believe it
> > > > contains the power-domain controllers, so when the Linux power-domain
> > > > drivers make the secure calls, it's likely the power-domains won't
> > > > change.
> > >
> > > - mainline uboot
> > > - optee 3.15.0
> > > - imx-atf rel_imx_5.4.70_2.3.2
> > > - Linux rel_imx_5.4.70_2.3.2
> > >
> > > I have tried even to not load optee in the image but still the same
> > >
> > > Michael
> > > >
> > > > adam
> > > > >
> > > > > All of them can be test using a buildroot setup
> > > > >
> > > > > Michael
> > >
> >
> > Michael,
> >
> > I have not bothered with NXP's kernel, but I do have DSIM working on
> > 5.15 with a set of patches here
> > https://github.com/Gateworks/linux-venice if it is helpful. I've been
> > using this with a mainline U-Boot, imx-atf lf_v2.4.
> >
> > Tim
>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael at amarulasolutions.com
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info at amarulasolutions.com
> www.amarulasolutions.com
>
>
> ------------------------------
>
> Message: 10
> Date: Sun, 13 Feb 2022 01:04:33 +0100
> From: Pali Roh?r <pali at kernel.org>
> To: Stefan Roese <sr at denx.de>
> Cc: Marek Beh?n <marek.behun at nic.cz>, Tony Dinh <mibodhi at gmail.com>,
> u-boot at lists.denx.de
> Subject: [PATCH] tools: kwbimage: Fix dumping DATA registers for v0
> images
> Message-ID: <20220213000433.16131-1-pali at kernel.org>
> Content-Type: text/plain; charset=UTF-8
>
> End of DATA register section is indicated by zero value in both raddr and
> rdata.
>
> So do not stop dumping registers with non-zero address and zero value.
> And also print end of DATA registers section.
>
> Fixes: 1a8e6b63e24f ("tools: kwbimage: Dump kwbimage config file on '-p
> -1' option")
> Signed-off-by: Pali Roh?r <pali at kernel.org>
> Reported-by: Tony Dinh <mibodhi at gmail.com>
> Tested-by: Tony Dinh <mibodhi at gmail.com>
> ---
> tools/kwbimage.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/kwbimage.c b/tools/kwbimage.c
> index 9b63ce80ff4e..99d38cd1cfb2 100644
> --- a/tools/kwbimage.c
> +++ b/tools/kwbimage.c
> @@ -2226,11 +2226,13 @@ static int kwbimage_generate_config(void *ptr,
> struct image_tool_params *params)
> ehdr0 = (struct ext_hdr_v0 *)(mhdr0 + 1);
> if (ehdr0->offset) {
> for (regdata = (struct ext_hdr_v0_reg *)((uint8_t
> *)ptr + ehdr0->offset);
> - (uint8_t *)regdata < (uint8_t *)ptr +
> header_size && regdata->raddr &&
> - regdata->rdata;
> + (uint8_t *)regdata < (uint8_t *)ptr +
> header_size &&
> + (regdata->raddr || regdata->rdata);
> regdata++)
> fprintf(f, "DATA 0x%08x 0x%08x\n",
> le32_to_cpu(regdata->raddr),
> le32_to_cpu(regdata->rdata));
> + if ((uint8_t *)regdata != (uint8_t *)ptr +
> ehdr0->offset)
> + fprintf(f, "DATA 0x0 0x0\n");
> }
> }
>
> --
> 2.20.1
>
>
>
> ------------------------------
>
> Message: 11
> Date: Sun, 13 Feb 2022 01:09:46 +0100
> From: Pali Roh?r <pali at kernel.org>
> To: Simon Glass <sjg at chromium.org>, Jan Kiszka
> <jan.kiszka at siemens.com>, Heinrich Schuchardt <xypron.glpk at gmx.de>
> Cc: u-boot at lists.denx.de
> Subject: [PATCH] tools: mkimage/dumpimage: Allow to use -l with -T
> Message-ID: <20220213000946.16236-1-pali at kernel.org>
> Content-Type: text/plain; charset=UTF-8
>
> Currently -l option for mkimage and dumpimage ignores option -T and always
> tries to autodetect image type.
>
> With this change it is possible to tell mkimage and dumpimage to parse
> image file as specific type (and not random autodetected type). This allows
> to use mkimage -l or dumpimage -l as tool for validating image.
>
> params.type for -l option is now by default initialized to zero
> (IH_TYPE_INVALID) instead of IH_TYPE_KERNEL. imagetool_get_type() for
> IH_TYPE_INVALID returns NULL, which is assigned to tparams. mkimage and
> dumpimage code is extended to handle tparams with NULL for -l option. And
> imagetool_verify_print_header() is extended to do validation via tparams if
> is not NULL.
>
> Signed-off-by: Pali Roh?r <pali at kernel.org>
> ---
> doc/mkimage.1 | 10 ++++++++--
> tools/dumpimage.c | 17 ++++++++---------
> tools/imagetool.c | 11 ++++++++++-
> tools/imagetool.h | 22 ++++------------------
> tools/mkimage.c | 36 ++++++++++++++----------------------
> 5 files changed, 44 insertions(+), 52 deletions(-)
>
> diff --git a/doc/mkimage.1 b/doc/mkimage.1
> index fc84cca066b0..287006279f71 100644
> --- a/doc/mkimage.1
> +++ b/doc/mkimage.1
> @@ -1,10 +1,10 @@
> -.TH MKIMAGE 1 "2010-05-16"
> +.TH MKIMAGE 1 "2022-02-07"
>
> .SH NAME
> mkimage \- Generate image for U-Boot
> .SH SYNOPSIS
> .B mkimage
> -.RB "\-l [" "uimage file name" "]"
> +.RB [ \-T " \fItype\fP] " \-l " [\fIuimage file name\fP]"
>
> .B mkimage
> .RB [\fIoptions\fP] " \-f [" "image tree source file" "]" " [" "uimage
> file name" "]"
> @@ -47,6 +47,12 @@ supports verified boot.
> .BI "\-l [" "uimage file name" "]"
> mkimage lists the information contained in the header of an existing
> U-Boot image.
>
> +.TP
> +.BI "\-T [" "image type" "]"
> +Parse image file as type.
> +Pass \-h as the image to see the list of supported image type.
> +Without this option image type is autodetected.
> +
> .P
> .B Create old legacy image:
>
> diff --git a/tools/dumpimage.c b/tools/dumpimage.c
> index e5481435a764..4791dd0dfe18 100644
> --- a/tools/dumpimage.c
> +++ b/tools/dumpimage.c
> @@ -12,9 +12,7 @@
> static void usage(void);
>
> /* parameters initialized by core will be used by the image type code */
> -static struct image_tool_params params = {
> - .type = IH_TYPE_KERNEL,
> -};
> +static struct image_tool_params params;
>
> /*
> * dumpimage_extract_subimage -
> @@ -110,7 +108,7 @@ int main(int argc, char **argv)
> }
> }
>
> - if (argc < 2)
> + if (argc < 2 || (params.iflag && params.lflag))
> usage();
>
> if (optind >= argc) {
> @@ -122,7 +120,7 @@ int main(int argc, char **argv)
>
> /* set tparams as per input type_id */
> tparams = imagetool_get_type(params.type);
> - if (tparams == NULL) {
> + if (!params.lflag && tparams == NULL) {
> fprintf(stderr, "%s: unsupported type: %s\n",
> params.cmdname, genimg_get_type_name(params.type));
> exit(EXIT_FAILURE);
> @@ -132,7 +130,7 @@ int main(int argc, char **argv)
> * check the passed arguments parameters meets the requirements
> * as per image type to be generated/listed
> */
> - if (tparams->check_params) {
> + if (tparams && tparams->check_params) {
> if (tparams->check_params(¶ms)) {
> fprintf(stderr, "%s: Parameter check failed\n",
> params.cmdname);
> @@ -159,7 +157,7 @@ int main(int argc, char **argv)
> exit(EXIT_FAILURE);
> }
>
> - if ((uint32_t)sbuf.st_size < tparams->header_size) {
> + if (tparams && (uint32_t)sbuf.st_size < tparams->header_size) {
> fprintf(stderr, "%s: Bad size: \"%s\" is not valid
> image\n",
> params.cmdname, params.imagefile);
> exit(EXIT_FAILURE);
> @@ -203,8 +201,9 @@ int main(int argc, char **argv)
>
> static void usage(void)
> {
> - fprintf(stderr, "Usage: %s -l image\n"
> - " -l ==> list image header information\n",
> + fprintf(stderr, "Usage: %s [-T type] -l image\n"
> + " -l ==> list image header information\n"
> + " -T ==> parse image file as 'type'\n",
> params.cmdname);
> fprintf(stderr,
> " %s [-T type] [-p position] [-o outfile] image\n"
> diff --git a/tools/imagetool.c b/tools/imagetool.c
> index ba1f64aa377a..5ad6d7413fec 100644
> --- a/tools/imagetool.c
> +++ b/tools/imagetool.c
> @@ -26,6 +26,12 @@ struct image_type_params *imagetool_get_type(int type)
> return NULL;
> }
>
> +static int imagetool_verify_print_header_by_type(
> + void *ptr,
> + struct stat *sbuf,
> + struct image_type_params *tparams,
> + struct image_tool_params *params);
> +
> int imagetool_verify_print_header(
> void *ptr,
> struct stat *sbuf,
> @@ -39,6 +45,9 @@ int imagetool_verify_print_header(
> struct image_type_params **start = __start_image_type;
> struct image_type_params **end = __stop_image_type;
>
> + if (tparams)
> + return imagetool_verify_print_header_by_type(ptr, sbuf,
> tparams, params);
> +
> for (curr = start; curr != end; curr++) {
> if ((*curr)->verify_header) {
> retval = (*curr)->verify_header((unsigned char
> *)ptr,
> @@ -65,7 +74,7 @@ int imagetool_verify_print_header(
> return retval;
> }
>
> -int imagetool_verify_print_header_by_type(
> +static int imagetool_verify_print_header_by_type(
> void *ptr,
> struct stat *sbuf,
> struct image_type_params *tparams,
> diff --git a/tools/imagetool.h b/tools/imagetool.h
> index c3f80fc64e84..5169b0245dad 100644
> --- a/tools/imagetool.h
> +++ b/tools/imagetool.h
> @@ -178,33 +178,19 @@ struct image_type_params *imagetool_get_type(int
> type);
> /*
> * imagetool_verify_print_header() - verifies the image header
> *
> - * Scan registered image types and verify the image_header for each
> - * supported image type. If verification is successful, this prints
> - * the respective header.
> - *
> - * Return: 0 on success, negative if input image format does not match
> with
> - * any of supported image types
> - */
> -int imagetool_verify_print_header(
> - void *ptr,
> - struct stat *sbuf,
> - struct image_type_params *tparams,
> - struct image_tool_params *params);
> -
> -/*
> - * imagetool_verify_print_header_by_type() - verifies the image header
> - *
> * Verify the image_header for the image type given by tparams.
> + * If tparams is NULL then scan registered image types and verify the
> + * image_header for each supported image type.
> * If verification is successful, this prints the respective header.
> * @ptr: pointer the the image header
> * @sbuf: stat information about the file pointed to by ptr
> - * @tparams: image type parameters
> + * @tparams: image type parameters or NULL
> * @params: mkimage parameters
> *
> * Return: 0 on success, negative if input image format does not match
> with
> * the given image type
> */
> -int imagetool_verify_print_header_by_type(
> +int imagetool_verify_print_header(
> void *ptr,
> struct stat *sbuf,
> struct image_type_params *tparams,
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index f8a9899a51f6..d24b3bc002bf 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -82,8 +82,9 @@ static int show_valid_options(enum ih_category category)
> static void usage(const char *msg)
> {
> fprintf(stderr, "Error: %s\n", msg);
> - fprintf(stderr, "Usage: %s -l image\n"
> - " -l ==> list image header
> information\n",
> + fprintf(stderr, "Usage: %s [-T type] -l image\n"
> + " -l ==> list image header information\n"
> + " -T ==> parse image file as 'type'\n",
> params.cmdname);
> fprintf(stderr,
> " %s [-x] -A arch -O os -T type -C comp -a addr -e
> ep -n name -d data_file[:data_file...] image\n"
> @@ -329,7 +330,7 @@ static void process_args(int argc, char **argv)
> params.datafile = datafile;
> else if (!params.datafile)
> usage("Missing data file for auto-FIT (use -d)");
> - } else if (type != IH_TYPE_INVALID) {
> + } else if (params.lflag || type != IH_TYPE_INVALID) {
> if (type == IH_TYPE_SCRIPT && !params.datafile)
> usage("Missing data file for script (use -d)");
> params.type = type;
> @@ -396,7 +397,7 @@ int main(int argc, char **argv)
>
> /* set tparams as per input type_id */
> tparams = imagetool_get_type(params.type);
> - if (tparams == NULL) {
> + if (tparams == NULL && !params.lflag) {
> fprintf (stderr, "%s: unsupported type %s\n",
> params.cmdname, genimg_get_type_name(params.type));
> exit (EXIT_FAILURE);
> @@ -406,14 +407,14 @@ int main(int argc, char **argv)
> * check the passed arguments parameters meets the requirements
> * as per image type to be generated/listed
> */
> - if (tparams->check_params)
> + if (tparams && tparams->check_params)
> if (tparams->check_params (¶ms))
> usage("Bad parameters for image type");
>
> if (!params.eflag) {
> params.ep = params.addr;
> /* If XIP, entry point must be after the U-Boot header */
> - if (params.xflag)
> + if (params.xflag && tparams)
> params.ep += tparams->header_size;
> }
>
> @@ -474,7 +475,7 @@ int main(int argc, char **argv)
> params.cmdname, params.imagefile);
> exit (EXIT_FAILURE);
> #endif
> - } else if (sbuf.st_size < (off_t)tparams->header_size) {
> + } else if (tparams && sbuf.st_size <
> (off_t)tparams->header_size) {
> fprintf (stderr,
> "%s: Bad size: \"%s\" is not valid image:
> size %llu < %u\n",
> params.cmdname, params.imagefile,
> @@ -493,21 +494,12 @@ int main(int argc, char **argv)
> exit (EXIT_FAILURE);
> }
>
> - if (params.fflag) {
> - /*
> - * Verifies the header format based on the
> expected header for image
> - * type in tparams
> - */
> - retval =
> imagetool_verify_print_header_by_type(ptr, &sbuf,
> - tparams, ¶ms);
> - } else {
> - /**
> - * When listing the image, we are not given the
> image type. Simply check all
> - * image types to find one that matches our header
> - */
> - retval = imagetool_verify_print_header(ptr, &sbuf,
> - tparams, ¶ms);
> - }
> + /*
> + * Verifies the header format based on the expected header
> for image
> + * type in tparams. If tparams is NULL simply check all
> image types
> + * to find one that matches our header.
> + */
> + retval = imagetool_verify_print_header(ptr, &sbuf,
> tparams, ¶ms);
>
> (void) munmap((void *)ptr, sbuf.st_size);
> (void) close (ifd);
> --
> 2.20.1
>
>
>
> ------------------------------
>
> Message: 12
> Date: Sun, 13 Feb 2022 10:09:19 +0200
> From: Ovidiu Panait <ovidiu.panait at windriver.com>
> To: u-boot at lists.denx.de
> Cc: Ovidiu Panait <ovidiu.panait at windriver.com>, Michal Simek
> <monstr at monstr.eu>
> Subject: [PATCH 1/7] microblaze: exception: move privileged
> instruction exception out of v5 ifdef
> Message-ID: <20220213080925.1548411-1-ovidiu.panait at windriver.com>
> Content-Type: text/plain
>
> The privileged instruction exception seems to have been introduced in
> microblaze v7.00 along with MMU support, so having it wrapped in
> MICROBLAZE_v5 ifdefs seems incorrect. Move it out of the ifdef, since all
> recent microblaze versions support it.
>
> Signed-off-by: Ovidiu Panait <ovidiu.panait at windriver.com>
> ---
>
> arch/microblaze/cpu/exception.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/microblaze/cpu/exception.c
> b/arch/microblaze/cpu/exception.c
> index e9476abedb..f60f1fc693 100644
> --- a/arch/microblaze/cpu/exception.c
> +++ b/arch/microblaze/cpu/exception.c
> @@ -37,10 +37,10 @@ void _hw_exception_handler (void)
> case 0x5:
> puts("Divide by zero exception\n");
> break;
> -#ifdef MICROBLAZE_V5
> case 0x7:
> puts("Priviledged or stack protection violation
> exception\n");
> break;
> +#ifdef MICROBLAZE_V5
> case 0x1000:
> puts("Exception in delay slot\n");
> break;
> --
> 2.25.1
>
>
>
> ------------------------------
>
> Message: 13
> Date: Sun, 13 Feb 2022 10:09:23 +0200
> From: Ovidiu Panait <ovidiu.panait at windriver.com>
> To: u-boot at lists.denx.de
> Cc: Ovidiu Panait <ovidiu.panait at windriver.com>, Michal Simek
> <monstr at monstr.eu>
> Subject: [PATCH 5/7] microblaze: exception: move unaligned access
> printfs inside switch case
> Message-ID: <20220213080925.1548411-5-ovidiu.panait at windriver.com>
> Content-Type: text/plain
>
> The unaligned access messages are only valid in the case of an unaligned
> data access exception. Do not print them for other types of hw exceptions.
>
> Signed-off-by: Ovidiu Panait <ovidiu.panait at windriver.com>
> ---
>
> arch/microblaze/cpu/exception.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/microblaze/cpu/exception.c
> b/arch/microblaze/cpu/exception.c
> index f79e465e1f..d37f04364a 100644
> --- a/arch/microblaze/cpu/exception.c
> +++ b/arch/microblaze/cpu/exception.c
> @@ -35,6 +35,10 @@ void _hw_exception_handler (void)
> switch (state & 0x1f) { /* mask on exception cause */
> case 0x1:
> puts("Unaligned data access exception\n");
> +
> + printf("Unaligned %sword access\n", ((state & 0x800) ? ""
> : "half"));
> + printf("Unaligned %s access\n", ((state & 0x400) ? "store"
> : "load"));
> + printf("Register R%x\n", (state & 0x3E) >> 5);
> break;
> case 0x2:
> puts("Illegal op-code exception\n");
> @@ -57,9 +61,6 @@ void _hw_exception_handler (void)
> }
>
> printf("Return address from exception 0x%x\n", address);
> - printf("Unaligned %sword access\n", ((state & 0x800) ? "" :
> "half"));
> - printf("Unaligned %s access\n", ((state & 0x400) ? "store" :
> "load"));
> - printf("Register R%x\n", (state & 0x3E) >> 5);
> hang();
> }
>
> --
> 2.25.1
>
>
>
> ------------------------------
>
> Message: 14
> Date: Sun, 13 Feb 2022 10:09:21 +0200
> From: Ovidiu Panait <ovidiu.panait at windriver.com>
> To: u-boot at lists.denx.de
> Cc: Ovidiu Panait <ovidiu.panait at windriver.com>, Michal Simek
> <monstr at monstr.eu>
> Subject: [PATCH 3/7] microblaze: exception: fix delay slot exception
> handling
> Message-ID: <20220213080925.1548411-3-ovidiu.panait at windriver.com>
> Content-Type: text/plain
>
> The switch statement in _hw_exception_handler() only covers the
> rightmost 5 bits that encode the exception cause:
> switch (state & 0x1f)
> {
> ...
> }
>
> For this reason, the "0x1000" case will never be reached, because the 13th
> bit was zeroed out. To fix this, move delay slot exception handling before
> the switch statement (delay slot (DS) bit in Exception Status Register is
> independent of the exception cause (EC)).
>
> Signed-off-by: Ovidiu Panait <ovidiu.panait at windriver.com>
> ---
>
> arch/microblaze/cpu/exception.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/microblaze/cpu/exception.c
> b/arch/microblaze/cpu/exception.c
> index 5601dde5b4..64d5fe4a80 100644
> --- a/arch/microblaze/cpu/exception.c
> +++ b/arch/microblaze/cpu/exception.c
> @@ -21,6 +21,11 @@ void _hw_exception_handler (void)
> printf("Hardware exception at 0x%x address\n", address);
> R17(address);
> printf("Return address from exception 0x%x\n", address);
> +
> + if (CONFIG_IS_ENABLED(XILINX_MICROBLAZE0_DELAY_SLOT_EXCEP) &&
> + (state & 0x1000))
> + puts("Exception in delay slot\n");
> +
> switch (state & 0x1f) { /* mask on exception cause */
> case 0x1:
> puts("Unaligned data access exception\n");
> @@ -40,11 +45,6 @@ void _hw_exception_handler (void)
> case 0x7:
> puts("Priviledged or stack protection violation
> exception\n");
> break;
> -#if CONFIG_IS_ENABLED(XILINX_MICROBLAZE0_DELAY_SLOT_EXCEP)
> - case 0x1000:
> - puts("Exception in delay slot\n");
> - break;
> -#endif
> default:
> puts("Undefined cause\n");
> break;
> --
> 2.25.1
>
>
>
> ------------------------------
>
> Message: 15
> Date: Sun, 13 Feb 2022 10:09:25 +0200
> From: Ovidiu Panait <ovidiu.panait at windriver.com>
> To: u-boot at lists.denx.de
> Cc: Ovidiu Panait <ovidiu.panait at windriver.com>, Michal Simek
> <monstr at monstr.eu>
> Subject: [PATCH 7/7] microblaze: exception: drop user exception
> support
> Message-ID: <20220213080925.1548411-7-ovidiu.panait at windriver.com>
> Content-Type: text/plain
>
> A user exception is triggered by inserting a bralid/brki jump to
> "C_BASE_VECTORS+0x8" in the software flow. Because u-boot microblaze code
> does not deal with MMU-related features such as user-mode/privileged-mode
> separation, there are no code sequences that call into the user exception
> handler.
>
> It seems there is no real usecase for having user exception support in
> u-boot, so drop the code that installs the nop handler.
>
> Signed-off-by: Ovidiu Panait <ovidiu.panait at windriver.com>
> ---
>
> arch/microblaze/cpu/exception.c | 8 -----
> arch/microblaze/cpu/start.S | 40 ++-----------------------
> board/xilinx/microblaze-generic/Kconfig | 9 ------
> 3 files changed, 3 insertions(+), 54 deletions(-)
>
> diff --git a/arch/microblaze/cpu/exception.c
> b/arch/microblaze/cpu/exception.c
> index d3640d3903..1f7c44d1f3 100644
> --- a/arch/microblaze/cpu/exception.c
> +++ b/arch/microblaze/cpu/exception.c
> @@ -63,11 +63,3 @@ void _hw_exception_handler (void)
> printf("Return address from exception 0x%x\n", address);
> hang();
> }
> -
> -#if CONFIG_IS_ENABLED(XILINX_MICROBLAZE0_USR_EXCEP)
> -void _exception_handler (void)
> -{
> - puts("User vector_exception\n");
> - hang();
> -}
> -#endif
> diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
> index 645f7cb038..0ea0b78da9 100644
> --- a/arch/microblaze/cpu/start.S
> +++ b/arch/microblaze/cpu/start.S
> @@ -97,9 +97,9 @@ clear_bss:
> * r5 - relocation offset (zero when setting up vectors before
> * relocation, and gd->reloc_off when setting up vectors after
> * relocation)
> - * - the relocation offset is added to the _exception_handler,
> - * _interrupt_handler and _hw_exception_handler symbols to reflect
> the
> - * post-relocation memory addresses
> + * - the relocation offset is added to the _interrupt_handler and
> + * _hw_exception_handler symbols to reflect the post-relocation
> memory
> + * addresses
> *
> * Reserve registers:
> * r10: Stores little/big endian offset for vectors
> @@ -149,40 +149,6 @@ __setup_exceptions:
> rsubi r8, r10, 0x6
> sh r6, r4, r8
>
> -#if CONFIG_IS_ENABLED(XILINX_MICROBLAZE0_USR_EXCEP)
> - /* user_vector_exception */
> - swi r2, r4, 0x8 /* user vector exception - imm opcode */
> - swi r3, r4, 0xC /* user vector exception - brai opcode */
> -
> - addik r6, r5, _exception_handler
> - sw r6, r1, r0
> - /*
> - * BIG ENDIAN memory map for user exception
> - * 0x8: 0xB000XXXX
> - * 0xC: 0xB808XXXX
> - *
> - * then it is necessary to count address for storing the most
> significant
> - * 16bits from _exception_handler address and copy it to
> - * 0xa address. Big endian use offset in r10=0 that's why is it
> just
> - * 0xa address. The same is done for the least significant 16 bits
> - * for 0xe address.
> - *
> - * LITTLE ENDIAN memory map for user exception
> - * 0x8: 0xXXXX00B0
> - * 0xC: 0xXXXX08B8
> - *
> - * Offset is for little endian setup to 0x2. rsubi instruction
> decrease
> - * address value to ensure that points to proper place which is
> - * 0x8 for the most significant 16 bits and
> - * 0xC for the least significant 16 bits
> - */
> - lhu r7, r1, r10
> - rsubi r8, r10, 0xa
> - sh r7, r4, r8
> - rsubi r8, r10, 0xe
> - sh r6, r4, r8
> -#endif
> -
> /* interrupt_handler */
> swi r2, r4, 0x10 /* interrupt - imm opcode */
> swi r3, r4, 0x14 /* interrupt - brai opcode */
> diff --git a/board/xilinx/microblaze-generic/Kconfig
> b/board/xilinx/microblaze-generic/Kconfig
> index 117b476f3f..a0af2e9abd 100644
> --- a/board/xilinx/microblaze-generic/Kconfig
> +++ b/board/xilinx/microblaze-generic/Kconfig
> @@ -38,15 +38,6 @@ config XILINX_MICROBLAZE0_HW_VER
> string "Core version number"
> default "7.10.d"
>
> -config XILINX_MICROBLAZE0_USR_EXCEP
> - bool "MicroBlaze user exception support"
> - default y
> - help
> - Enable this option in order to install the user exception handler
> - (_exception_handler routine from
> arch/microblaze/cpu/exception.c) in
> - the exception vector table. The user exception vector is located
> at
> - C_BASE_VECTORS + 0x8 address.
> -
> config XILINX_MICROBLAZE0_DELAY_SLOT_EXCEP
> bool "MicroBlaze delay slot exception support"
> default y
> --
> 2.25.1
>
>
>
> ------------------------------
>
> Message: 16
> Date: Sun, 13 Feb 2022 10:09:24 +0200
> From: Ovidiu Panait <ovidiu.panait at windriver.com>
> To: u-boot at lists.denx.de
> Cc: Ovidiu Panait <ovidiu.panait at windriver.com>, Michal Simek
> <monstr at monstr.eu>
> Subject: [PATCH 6/7] microblaze: exception: fix unaligned data access
> register mask
> Message-ID: <20220213080925.1548411-6-ovidiu.panait at windriver.com>
> Content-Type: text/plain
>
> The correct mask for getting the source/destination register from ESR in
> the case of an unaligned access exception is 0x3E0. With this change, a
> dummy unaligned store produces the expected info:
> """
> >> swi r5, r0, 0x111
>
> ...
> Hardware exception at 0x111 address
> Unaligned data access exception
> Unaligned word access
> Unaligned store access
> Register R5
> Return address from exception 0x7f99dfc
> ### ERROR ### Please RESET the board ###
> """
>
> Signed-off-by: Ovidiu Panait <ovidiu.panait at windriver.com>
> ---
>
> arch/microblaze/cpu/exception.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/microblaze/cpu/exception.c
> b/arch/microblaze/cpu/exception.c
> index d37f04364a..d3640d3903 100644
> --- a/arch/microblaze/cpu/exception.c
> +++ b/arch/microblaze/cpu/exception.c
> @@ -38,7 +38,7 @@ void _hw_exception_handler (void)
>
> printf("Unaligned %sword access\n", ((state & 0x800) ? ""
> : "half"));
> printf("Unaligned %s access\n", ((state & 0x400) ? "store"
> : "load"));
> - printf("Register R%x\n", (state & 0x3E) >> 5);
> + printf("Register R%x\n", (state & 0x3E0) >> 5);
> break;
> case 0x2:
> puts("Illegal op-code exception\n");
> --
> 2.25.1
>
>
>
> ------------------------------
>
> Message: 17
> Date: Sun, 13 Feb 2022 10:09:20 +0200
> From: Ovidiu Panait <ovidiu.panait at windriver.com>
> To: u-boot at lists.denx.de
> Cc: Ovidiu Panait <ovidiu.panait at windriver.com>, Michal Simek
> <monstr at monstr.eu>
> Subject: [PATCH 2/7] microblaze: exception: migrate MICROBLAZE_V5 to
> Kconfig
> Message-ID: <20220213080925.1548411-2-ovidiu.panait at windriver.com>
> Content-Type: text/plain
>
> Also, rename it to XILINX_MICROBLAZE0_DELAY_SLOT_EXCEP, since it only
> covers delay slot exception support.
>
> Signed-off-by: Ovidiu Panait <ovidiu.panait at windriver.com>
> ---
>
> arch/microblaze/cpu/exception.c | 2 +-
> board/xilinx/microblaze-generic/Kconfig | 9 +++++++++
> include/configs/microblaze-generic.h | 3 ---
> 3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/arch/microblaze/cpu/exception.c
> b/arch/microblaze/cpu/exception.c
> index f60f1fc693..5601dde5b4 100644
> --- a/arch/microblaze/cpu/exception.c
> +++ b/arch/microblaze/cpu/exception.c
> @@ -40,7 +40,7 @@ void _hw_exception_handler (void)
> case 0x7:
> puts("Priviledged or stack protection violation
> exception\n");
> break;
> -#ifdef MICROBLAZE_V5
> +#if CONFIG_IS_ENABLED(XILINX_MICROBLAZE0_DELAY_SLOT_EXCEP)
> case 0x1000:
> puts("Exception in delay slot\n");
> break;
> diff --git a/board/xilinx/microblaze-generic/Kconfig
> b/board/xilinx/microblaze-generic/Kconfig
> index e31257d335..117b476f3f 100644
> --- a/board/xilinx/microblaze-generic/Kconfig
> +++ b/board/xilinx/microblaze-generic/Kconfig
> @@ -47,6 +47,15 @@ config XILINX_MICROBLAZE0_USR_EXCEP
> the exception vector table. The user exception vector is located
> at
> C_BASE_VECTORS + 0x8 address.
>
> +config XILINX_MICROBLAZE0_DELAY_SLOT_EXCEP
> + bool "MicroBlaze delay slot exception support"
> + default y
> + help
> + Enable this option if the MicroBlaze processor supports
> exceptions
> + caused by delay slot instructions (processor version >= v5.00).
> When
> + enabled, the hw exception handler will print a message indicating
> + whether the exception was triggered by a delay slot instruction.
> +
> config XILINX_MICROBLAZE0_VECTOR_BASE_ADDR
> hex "Location of MicroBlaze vectors"
> default 0x0
> diff --git a/include/configs/microblaze-generic.h
> b/include/configs/microblaze-generic.h
> index ca749ed18a..fd5a9cf8b8 100644
> --- a/include/configs/microblaze-generic.h
> +++ b/include/configs/microblaze-generic.h
> @@ -11,9 +11,6 @@
> /* Microblaze is microblaze_0 */
> #define XILINX_FSL_NUMBER 3
>
> -/* MicroBlaze CPU */
> -#define MICROBLAZE_V5 1
> -
> #define CONFIG_SYS_BOOTM_LEN (64 * 1024 * 1024)
>
> /* uart */
> --
> 2.25.1
>
>
>
> ------------------------------
>
> Message: 18
> Date: Sun, 13 Feb 2022 10:09:22 +0200
> From: Ovidiu Panait <ovidiu.panait at windriver.com>
> To: u-boot at lists.denx.de
> Cc: Ovidiu Panait <ovidiu.panait at windriver.com>, Michal Simek
> <monstr at monstr.eu>
> Subject: [PATCH 4/7] microblaze: exception: fix return address for
> delay slot exceptions
> Message-ID: <20220213080925.1548411-4-ovidiu.panait at windriver.com>
> Content-Type: text/plain
>
> According to the MicroBlaze reference manual (xilinx2021.2/ug984/page-37):
> """
> If an exception is caused by an instruction in a delay slot (that is,
> ESR[DS]=1), the exception handler should return execution to
> the address stored in BTR instead of the normal exception return
> address stored in R17.
> """
>
> Adjust the code to print the proper return address for delay slot
> exceptions.
>
> Signed-off-by: Ovidiu Panait <ovidiu.panait at windriver.com>
> ---
>
> arch/microblaze/cpu/exception.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/microblaze/cpu/exception.c
> b/arch/microblaze/cpu/exception.c
> index 64d5fe4a80..f79e465e1f 100644
> --- a/arch/microblaze/cpu/exception.c
> +++ b/arch/microblaze/cpu/exception.c
> @@ -20,11 +20,17 @@ void _hw_exception_handler (void)
> MFS(state, resr);
> printf("Hardware exception at 0x%x address\n", address);
> R17(address);
> - printf("Return address from exception 0x%x\n", address);
>
> if (CONFIG_IS_ENABLED(XILINX_MICROBLAZE0_DELAY_SLOT_EXCEP) &&
> - (state & 0x1000))
> + (state & 0x1000)) {
> + /*
> + * For exceptions in delay slots, the return address is
> stored
> + * in the Branch Target Register (BTR), rather than R17.
> + */
> + MFS(address, rbtr);
> +
> puts("Exception in delay slot\n");
> + }
>
> switch (state & 0x1f) { /* mask on exception cause */
> case 0x1:
> @@ -49,6 +55,8 @@ void _hw_exception_handler (void)
> puts("Undefined cause\n");
> break;
> }
> +
> + printf("Return address from exception 0x%x\n", address);
> printf("Unaligned %sword access\n", ((state & 0x800) ? "" :
> "half"));
> printf("Unaligned %s access\n", ((state & 0x400) ? "store" :
> "load"));
> printf("Register R%x\n", (state & 0x3E) >> 5);
> --
> 2.25.1
>
>
>
> ------------------------------
>
> Message: 19
> Date: Sun, 13 Feb 2022 09:58:10 +0100
> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
> To: Masami Hiramatsu <masami.hiramatsu at linaro.org>,
> u-boot at lists.denx.de
> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>, Patrice Chotard
> <patrice.chotard at foss.st.com>, Alexander Graf <agraf at csgraf.de>,
> AKASHI Takahiro <takahiro.akashi at linaro.org>, Simon Glass
> <sjg at chromium.org>, Bin Meng <bmeng.cn at gmail.com>, Ilias
> Apalodimas
> <ilias.apalodimas at linaro.org>, Jose Marinho <jose.marinho at arm.com
> >,
> Grant Likely <grant.likely at arm.com>, Tom Rini <trini at konsulko.com
> >,
> Etienne Carriere <etienne.carriere at linaro.org>, Sughosh Ganu
> <sughosh.ganu at linaro.org>, Paul Liu <paul.liu at linaro.org>
> Subject: Re: [PATCH v4 1/2] efi_loader: use
> efi_update_capsule_firmware() for capsule on disk
> Message-ID: <cfd78afc-8b23-0dd7-98aa-4f9477a08c93 at gmx.de>
> Content-Type: text/plain; charset=UTF-8; format=flowed
>
> On 2/3/22 10:23, Masami Hiramatsu wrote:
> > Since the efi_update_capsule() represents the UpdateCapsule() runtime
> > service, it has to handle the capsule flags and update ESRT. However
> > the capsule-on-disk doesn't need to care about such things.
> >
> > Thus, the capsule-on-disk should use the efi_capsule_update_firmware()
> > directly instead of calling efi_update_capsule().
> >
> > This means the roles of the efi_update_capsule() and capsule-on-disk
> > are different. We have to keep the efi_update_capsule() for providing
> > runtime service API at boot time.
> >
> > Suggested-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>
>
> ------------------------------
>
> Message: 20
> Date: Sun, 13 Feb 2022 10:01:35 +0100
> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
> To: Masami Hiramatsu <masami.hiramatsu at linaro.org>,
> u-boot at lists.denx.de
> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>, Patrice Chotard
> <patrice.chotard at foss.st.com>, Heinrich Schuchardt
> <xypron.glpk at gmx.de>, Alexander Graf <agraf at csgraf.de>, AKASHI
> Takahiro <takahiro.akashi at linaro.org>, Simon Glass <
> sjg at chromium.org>,
> Bin Meng <bmeng.cn at gmail.com>, Ilias Apalodimas
> <ilias.apalodimas at linaro.org>, Jose Marinho <jose.marinho at arm.com
> >,
> Grant Likely <grant.likely at arm.com>, Tom Rini <trini at konsulko.com
> >,
> Etienne Carriere <etienne.carriere at linaro.org>, Sughosh Ganu
> <sughosh.ganu at linaro.org>, Paul Liu <paul.liu at linaro.org>
> Subject: Re: [PATCH v4 2/2] efi_loader: Reset system after
> CapsuleUpdate on disk
> Message-ID: <b69b5a40-14a2-6791-b63f-925b9b1645ff at gmx.de>
> Content-Type: text/plain; charset=UTF-8; format=flowed
>
> On 2/3/22 10:23, Masami Hiramatsu wrote:
> > Add a cold reset soon after processing capsule update on disk.
> > This is required in UEFI specification 2.9 Section 8.5.5
> > "Delivery of Capsules via file on Mass Storage device" as;
> >
> > In all cases that a capsule is identified for processing the system
> is
> > restarted after capsule processing is completed.
> >
> > This also reports the result of each capsule update so that the user can
> > notice that the capsule update has been succeeded or not from console
> log.
> >
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>
>
> ------------------------------
>
> Message: 21
> Date: Sun, 13 Feb 2022 10:58:17 +0100
> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
> To: Masahisa Kojima <masahisa.kojima at linaro.org>
> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>, Simon Glass
> <sjg at chromium.org>, Takahiro Akashi <takahiro.akashi at linaro.org>,
> Francois Ozog <francois.ozog at linaro.org>, Mark Kettenis
> <mark.kettenis at xs4all.nl>, u-boot at lists.denx.de
> Subject: Re: [PATCH 3/3] efi_loader: add menu-driven UEFI Boot
> Variable maintenance
> Message-ID: <9e88e71a-ea98-98ad-e0db-1686c2045d95 at gmx.de>
> Content-Type: text/plain; charset=UTF-8; format=flowed
>
> On 2/10/22 08:05, Masahisa Kojima wrote:
> > This commit adds the menu-driven UEFI Boot Variable maintenance.
> > User can add and delete the Boot#### variable, and update the
> > BootOrder variable through menu operation.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > ---
> > lib/efi_loader/efi_bootmgr.c | 720 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 720 insertions(+)
> >
> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > index 013d868f23..739140f742 100644
> > --- a/lib/efi_loader/efi_bootmgr.c
> > +++ b/lib/efi_loader/efi_bootmgr.c
> > @@ -32,6 +32,8 @@ static const struct efi_runtime_services *rs;
>
> Where is the Kconfig entry to disable this code?
>
> > */
> >
> > #define EFI_BOOTMGR_MENU_ENTRY_NUM_MAX 1024
> > +#define EFI_BOOTMGR_FILE_PATH_MAX 512
> > +#define EFI_BOOTMGR_BOOT_NAME_MAX 64
> >
> > typedef efi_status_t (*efi_bootmenu_entry_func)(void *data, bool
> *exit);
> >
> > @@ -95,12 +97,49 @@ struct efi_bootmgr_boot_selection_data {
> >
> > static efi_status_t efi_bootmgr_process_boot_selected(void *data, bool
> *exit);
> > static efi_status_t efi_bootmgr_process_boot_selection(void *data,
> bool *exit);
> > +static efi_status_t efi_bootmgr_process_maintenance(void *data, bool
> *exit);
> > +static efi_status_t efi_bootmgr_process_add_boot_option(void *data,
> bool *exit);
> > +static efi_status_t efi_bootmgr_process_delete_boot_option(void *data,
> bool *exit);
> > +static efi_status_t efi_bootmgr_process_change_boot_order(void *data,
> bool *exit);
> >
> > static struct efi_bootmgr_menu_item bootmgr_menu_items[] = {
> > {u"Boot Manager", efi_bootmgr_process_boot_selection},
> > + {u"Boot Manager maintenance", efi_bootmgr_process_maintenance},
> > {u"Quit", NULL},
> > };
> >
> > +static struct efi_bootmgr_menu_item maintenance_menu_items[] = {
> > + {u"Add Boot Option", efi_bootmgr_process_add_boot_option},
> > + {u"Delete Boot Option", efi_bootmgr_process_delete_boot_option},
> > + {u"Change Boot Order", efi_bootmgr_process_change_boot_order},
> > + {u"Quit", NULL},
> > +};
> > +
> > +struct efi_bootmgr_boot_option {
> > + struct efi_simple_file_system_protocol *current_volume;
> > + struct efi_device_path *dp_volume;
> > + u16 *current_path;
> > + u16 *boot_name;
> > + bool file_selected;
> > +};
> > +
> > +static const struct efi_device_path END = {
> > + .type = DEVICE_PATH_TYPE_END,
> > + .sub_type = DEVICE_PATH_SUB_TYPE_END,
> > + .length = sizeof(END),
> > +};
> > +
> > +struct efi_bootmgr_volume_entry_data {
> > + struct efi_bootmgr_boot_option *bo;
> > + struct efi_simple_file_system_protocol *v;
> > + struct efi_device_path *dp;
> > +};
> > +
> > +struct efi_bootmgr_file_entry_data {
> > + struct efi_bootmgr_boot_option *bo;
> > + struct efi_file_info *f;
> > +};
> > +
> > static void efi_bootmgr_menu_print_entry(void *data)
> > {
> > struct efi_bootmgr_menu_entry *entry = data;
> > @@ -558,6 +597,687 @@ static efi_status_t
> efi_bootmgr_process_boot_selection(void *data, bool *exit)
> > return ret;
> > }
> >
> > +static efi_status_t efi_bootmgr_volume_selected(void *data, bool *exit)
> > +{
> > + struct efi_bootmgr_volume_entry_data *info = data;
> > +
> > + *exit = true;
> > +
> > + if (info) {
> > + info->bo->current_volume = info->v;
> > + info->bo->dp_volume = info->dp;
> > + }
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +static efi_status_t efi_bootmgr_file_selected(void *data, bool *exit)
> > +{
> > + struct efi_bootmgr_file_entry_data *info = data;
> > +
> > + *exit = true;
> > +
> > + if (!info)
> > + return EFI_INVALID_PARAMETER;
> > +
> > + if (u16_strncmp(info->f->file_name, u".", 1) == 0 &&
> > + u16_strlen(info->f->file_name) == 1) {
> > + /* stay current path */
> > + } else if (u16_strncmp(info->f->file_name, u"..", 2) == 0 &&
> > + u16_strlen(info->f->file_name) == 2) {
> > + u32 i;
> > + int len = u16_strlen(info->bo->current_path);
> > +
> > + for (i = len - 2; i > 0; i--) {
> > + if (info->bo->current_path[i] == u'\\')
> > + break;
> > + }
> > +
> > + if (i == 0)
> > + info->bo->current_path[0] = u'\0';
> > + else
> > + info->bo->current_path[i + 1] = u'\0';
> > + } else {
> > + size_t new_len;
> > +
> > + new_len = u16_strlen(info->bo->current_path) +
> > + u16_strlen(info->f->file_name) + 1;
> > + if (new_len >= EFI_BOOTMGR_FILE_PATH_MAX) {
>
> Why do we need such an arbitrary limitation? Please, allocate a buffer
> of adequate size.
>
> > + /* TODO: show error notification to user */
> > + log_err("file path is too long\n");
> > + return EFI_INVALID_PARAMETER;
> > + }
> > + u16_strcat(info->bo->current_path, info->f->file_name);
>
> I would prefer to use a safe function here where the destination buffer
> length is an argument.
>
>
> > + if (info->f->attribute & EFI_FILE_DIRECTORY) {
> > + if (new_len + 1 >= EFI_BOOTMGR_FILE_PATH_MAX) {
>
> Please, remove this duplicate test and fix the test above.
>
> > + log_err("file path is too long\n");
> > + return EFI_INVALID_PARAMETER;
> > + }
> > + u16_strcat(info->bo->current_path, u"\\");
> > + } else {
> > + info->bo->file_selected = true;
> > + }
> > + }
> > + return EFI_SUCCESS;
> > +}
> > +
> > +static efi_status_t efi_bootmgr_select_volume(struct
> efi_bootmgr_boot_option *bo)
> > +{
> > + u16 *name;
> > + u32 i;
> > + efi_status_t ret;
> > + efi_uintn_t count;
> > + struct efi_device_path *device_path;
> > + efi_handle_t *volume_handles = NULL;
> > + struct efi_simple_file_system_protocol *v;
> > + struct efi_device_path_to_text_protocol *text;
> > + struct efi_bootmgr_menu_item *menu_item, *iter;
> > +
> > + ret = EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL,
> &efi_system_partition_guid,
>
> We have too many EFI_CALLs. Factor out a function
> efi_locate_handle_buffer_int() which you can call without EFI_CALL and
> remove all of the existing EFI_CALLs.
>
> > + NULL, &count,
> > + (efi_handle_t
> **)&volume_handles));
> > + if (ret != EFI_SUCCESS)
> > + return ret;
>
> What will you do if you get multiple results?
>
> > +
> > + ret =
> EFI_CALL(systab.boottime->locate_protocol(&efi_guid_device_path_to_text_protocol,
> > + NULL, (void
> **)&text));
>
> This will give you a random instance of a device path and not the one
> related to the ESP. You just called LocateHandleBuffer() for good
> reason. Please, use efi_search_protocol().
>
> > + if (ret != EFI_SUCCESS)
> > + goto out1;
> > +
> > + menu_item = calloc(count + 1, sizeof(struct
> efi_bootmgr_menu_item));
> > + if (!menu_item) {
> > + ret = EFI_OUT_OF_RESOURCES;
> > + goto out1;
> > + }
> > +
> > + iter = menu_item;
> > + for (i = 0; i < count; i++) {
> > + struct efi_bootmgr_volume_entry_data *info;
> > +
> > + ret =
> EFI_CALL(systab.boottime->open_protocol(volume_handles[i],
> > +
> &efi_simple_file_system_protocol_guid,
> > + (void **)&v, efi_root,
> NULL,
> > +
> EFI_OPEN_PROTOCOL_GET_PROTOCOL));
> > + if (ret != EFI_SUCCESS)
> > + continue;
> > +
> > + ret =
> EFI_CALL(systab.boottime->open_protocol(volume_handles[i],
> > + &efi_guid_device_path,
> > + (void **)&device_path,
> efi_root, NULL,
> > +
> EFI_OPEN_PROTOCOL_GET_PROTOCOL));
> > + if (ret != EFI_SUCCESS)
> > + continue;
> > +
> > + name = text->convert_device_path_to_text(device_path,
> true, true);
> > + if (!name) {
> > + ret = EFI_OUT_OF_RESOURCES;
> > + goto out2;
> > + }
> > +
> > + info = calloc(1, sizeof(struct
> efi_bootmgr_volume_entry_data));
> > + if (!info) {
> > + ret = EFI_OUT_OF_RESOURCES;
> > + goto out2;
> > + }
> > +
> > + info->v = v;
> > + info->dp = device_path;
> > + info->bo = bo;
> > + iter->title = name;
> > + iter->func = efi_bootmgr_volume_selected;
> > + iter->data = info;
> > + iter++;
> > + }
> > +
> > + iter->title = u"Quit";
> > + iter->func = NULL;
> > + iter->data = NULL;
> > + count += 1;
> > +
> > + ret = efi_bootmgr_process_common(menu_item, count, false);
> > +
> > +out2:
> > + iter = menu_item;
> > + for (i = 0; i < count - 1; i++) {
> > + struct efi_bootmgr_volume_entry_data *p;
> > +
> > + p = (struct efi_bootmgr_volume_entry_data *)
More information about the U-Boot
mailing list