Unsubscribe On Sun, Feb 13, 2022, 12:00 <u-boot-requ...@lists.denx.de> wrote:
> Send U-Boot mailing list submissions to > u-boot@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-requ...@lists.denx.de > > You can reach the person managing the list at > u-boot-ow...@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....@gmail.com> > To: Daniel Klauer <daniel.kla...@gin.de> > Cc: U-Boot Mailing List <u-boot@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--zell6c64spc...@mail.gmail.com> > Content-Type: text/plain; charset="UTF-8" > > On Wed, Feb 9, 2022 at 5:41 PM Daniel Klauer <daniel.kla...@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 <aford...@gmail.com> > To: u-boot@lists.denx.de > Cc: tr...@konsulko.com, af...@beaconembedded.com, Adam Ford > <aford...@gmail.com> > Subject: [PATCH 1/2] arm: omap3: Cleanup sys_info to fit OMAP3 booting > with LTO > Message-ID: <20220212121241.1655425-1-aford...@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 <aford...@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 <aford...@gmail.com> > To: u-boot@lists.denx.de > Cc: tr...@konsulko.com, af...@beaconembedded.com, Adam Ford > <aford...@gmail.com> > Subject: [PATCH 2/2] arm: omap3: Make some memory functions static and > clean headers > Message-ID: <20220212121241.1655425-2-aford...@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 <aford...@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 <l...@lucaceresoli.net> > To: u-boot@lists.denx.de > Cc: Luca Ceresoli <l...@lucaceresoli.net>, Michal Simek > <michal.si...@xilinx.com>, Giulio Benetti > <giulio.bene...@benettiengineering.com>, Neal Frager > <ne...@xilinx.com> > Subject: [PATCH] tools/zynqmp_pm_cfg_obj_convert.py: fix build with > Vivado 2021.x > Message-ID: <20220212125121.3398547-1-l...@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 <ne...@xilinx.com> > [report: > https://lists.buildroot.org/pipermail/buildroot/2022-February/636639.html] > Signed-off-by: Luca Ceresoli <l...@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 <aford...@gmail.com> > To: u-boot@lists.denx.de > Cc: ma...@denx.de, tr...@konsulko.com, Adam Ford < > aford...@gmail.com> > Subject: [PATCH] usb: ehci-omap: Drop dead code > Message-ID: <20220212142640.1669851-1-aford...@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 <aford...@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.shevche...@gmail.com> > To: Andy Shevchenko <andriy.shevche...@intel.com> > Cc: Tom Rini <tr...@konsulko.com>, Simon Glass <s...@chromium.org>, Bin > Meng <bmeng...@gmail.com>, U-Boot Mailing List > <u-boot@lists.denx.de>, u-boot-custodi...@lists.denx.de, > u-boot-board-maintain...@lists.denx.de > Subject: Re: [ANN] U-Boot v2022.04-rc1 released > Message-ID: > < > cahp75vfgwngtjousj05dfmu0qfqtrwr9obqcbc9tli8h3lh...@mail.gmail.com> > Content-Type: text/plain; charset="UTF-8" > > On Fri, Feb 11, 2022 at 9:15 PM Andy Shevchenko > <andy.shevche...@gmail.com> wrote: > > On Fri, Feb 11, 2022 at 8:46 PM Andy Shevchenko > > <andy.shevche...@gmail.com> wrote: > > > On Fri, Feb 11, 2022 at 8:31 PM Andy Shevchenko > > > <andriy.shevche...@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 <mon...@monstr.eu> > To: Tom Rini <tr...@konsulko.com>, u-boot@lists.denx.de, Simon Glass > <s...@chromium.org> > Cc: u-boot-custodi...@lists.denx.de, > u-boot-board-maintain...@lists.denx.de, Michal Simek > <michal.si...@xilinx.com> > Subject: Re: [ANN] U-Boot v2022.04-rc1 released > Message-ID: <b7383793-f671-0766-1309-14b75f031...@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...@gmail.com> > To: Andrew Jeffery <and...@aj.id.au>, u-boot@lists.denx.de > Cc: s...@chromium.org, chiawei_w...@aspeedtech.com, j...@jms.id.au, > open...@lists.ozlabs.org > Subject: Re: [PATCH] image: Control FIT signature verification at > runtime > Message-ID: <97430094-7d2a-432b-a121-96812fac8...@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 <and...@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-and...@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 <mich...@amarulasolutions.com> > To: Tim Harvey <thar...@gateworks.com> > Cc: Adam Ford <aford...@gmail.com>, Fabio Estevam > <feste...@gmail.com>, dl-uboot-imx <uboot-...@nxp.com>, > U-Boot-Denx > <u-boot@lists.denx.de>, Stefano Babic <sba...@denx.de> > Subject: Re: Problem IMX8MN booting legacy kernel and mipi display > Message-ID: > <CAOf5uwkKG6mc0ek+H5vLn= > kwwu0gghgdygjv0bemj1+8dfl...@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 <thar...@gateworks.com> wrote: > > > > On Wed, Feb 2, 2022 at 5:04 AM Michael Nazzareno Trimarchi > > <mich...@amarulasolutions.com> wrote: > > > > > > Hi Adam > > > > > > On Wed, Feb 2, 2022 at 2:02 PM Adam Ford <aford...@gmail.com> wrote: > > > > > > > > On Wed, Feb 2, 2022 at 6:24 AM Michael Nazzareno Trimarchi > > > > <mich...@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 > mich...@amarulasolutions.com > __________________________________ > > Amarula Solutions BV > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > T. +31 (0)85 111 9172 > i...@amarulasolutions.com > www.amarulasolutions.com > > > ------------------------------ > > Message: 10 > Date: Sun, 13 Feb 2022 01:04:33 +0100 > From: Pali Roh?r <p...@kernel.org> > To: Stefan Roese <s...@denx.de> > Cc: Marek Beh?n <marek.be...@nic.cz>, Tony Dinh <mibo...@gmail.com>, > u-boot@lists.denx.de > Subject: [PATCH] tools: kwbimage: Fix dumping DATA registers for v0 > images > Message-ID: <20220213000433.16131-1-p...@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 <p...@kernel.org> > Reported-by: Tony Dinh <mibo...@gmail.com> > Tested-by: Tony Dinh <mibo...@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 <p...@kernel.org> > To: Simon Glass <s...@chromium.org>, Jan Kiszka > <jan.kis...@siemens.com>, Heinrich Schuchardt <xypron.g...@gmx.de> > Cc: u-boot@lists.denx.de > Subject: [PATCH] tools: mkimage/dumpimage: Allow to use -l with -T > Message-ID: <20220213000946.16236-1-p...@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 <p...@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.pan...@windriver.com> > To: u-boot@lists.denx.de > Cc: Ovidiu Panait <ovidiu.pan...@windriver.com>, Michal Simek > <mon...@monstr.eu> > Subject: [PATCH 1/7] microblaze: exception: move privileged > instruction exception out of v5 ifdef > Message-ID: <20220213080925.1548411-1-ovidiu.pan...@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.pan...@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.pan...@windriver.com> > To: u-boot@lists.denx.de > Cc: Ovidiu Panait <ovidiu.pan...@windriver.com>, Michal Simek > <mon...@monstr.eu> > Subject: [PATCH 5/7] microblaze: exception: move unaligned access > printfs inside switch case > Message-ID: <20220213080925.1548411-5-ovidiu.pan...@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.pan...@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.pan...@windriver.com> > To: u-boot@lists.denx.de > Cc: Ovidiu Panait <ovidiu.pan...@windriver.com>, Michal Simek > <mon...@monstr.eu> > Subject: [PATCH 3/7] microblaze: exception: fix delay slot exception > handling > Message-ID: <20220213080925.1548411-3-ovidiu.pan...@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.pan...@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.pan...@windriver.com> > To: u-boot@lists.denx.de > Cc: Ovidiu Panait <ovidiu.pan...@windriver.com>, Michal Simek > <mon...@monstr.eu> > Subject: [PATCH 7/7] microblaze: exception: drop user exception > support > Message-ID: <20220213080925.1548411-7-ovidiu.pan...@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.pan...@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.pan...@windriver.com> > To: u-boot@lists.denx.de > Cc: Ovidiu Panait <ovidiu.pan...@windriver.com>, Michal Simek > <mon...@monstr.eu> > Subject: [PATCH 6/7] microblaze: exception: fix unaligned data access > register mask > Message-ID: <20220213080925.1548411-6-ovidiu.pan...@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.pan...@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.pan...@windriver.com> > To: u-boot@lists.denx.de > Cc: Ovidiu Panait <ovidiu.pan...@windriver.com>, Michal Simek > <mon...@monstr.eu> > Subject: [PATCH 2/7] microblaze: exception: migrate MICROBLAZE_V5 to > Kconfig > Message-ID: <20220213080925.1548411-2-ovidiu.pan...@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.pan...@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.pan...@windriver.com> > To: u-boot@lists.denx.de > Cc: Ovidiu Panait <ovidiu.pan...@windriver.com>, Michal Simek > <mon...@monstr.eu> > Subject: [PATCH 4/7] microblaze: exception: fix return address for > delay slot exceptions > Message-ID: <20220213080925.1548411-4-ovidiu.pan...@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.pan...@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.g...@gmx.de> > To: Masami Hiramatsu <masami.hirama...@linaro.org>, > u-boot@lists.denx.de > Cc: Patrick Delaunay <patrick.delau...@foss.st.com>, Patrice Chotard > <patrice.chot...@foss.st.com>, Alexander Graf <ag...@csgraf.de>, > AKASHI Takahiro <takahiro.aka...@linaro.org>, Simon Glass > <s...@chromium.org>, Bin Meng <bmeng...@gmail.com>, Ilias > Apalodimas > <ilias.apalodi...@linaro.org>, Jose Marinho <jose.mari...@arm.com > >, > Grant Likely <grant.lik...@arm.com>, Tom Rini <tr...@konsulko.com > >, > Etienne Carriere <etienne.carri...@linaro.org>, Sughosh Ganu > <sughosh.g...@linaro.org>, Paul Liu <paul....@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-4f9477a08...@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.aka...@linaro.org> > > Signed-off-by: Masami Hiramatsu <masami.hirama...@linaro.org> > > Reviewed-by: Heinrich Schuchardt <xypron.g...@gmx.de> > > > ------------------------------ > > Message: 20 > Date: Sun, 13 Feb 2022 10:01:35 +0100 > From: Heinrich Schuchardt <xypron.g...@gmx.de> > To: Masami Hiramatsu <masami.hirama...@linaro.org>, > u-boot@lists.denx.de > Cc: Patrick Delaunay <patrick.delau...@foss.st.com>, Patrice Chotard > <patrice.chot...@foss.st.com>, Heinrich Schuchardt > <xypron.g...@gmx.de>, Alexander Graf <ag...@csgraf.de>, AKASHI > Takahiro <takahiro.aka...@linaro.org>, Simon Glass < > s...@chromium.org>, > Bin Meng <bmeng...@gmail.com>, Ilias Apalodimas > <ilias.apalodi...@linaro.org>, Jose Marinho <jose.mari...@arm.com > >, > Grant Likely <grant.lik...@arm.com>, Tom Rini <tr...@konsulko.com > >, > Etienne Carriere <etienne.carri...@linaro.org>, Sughosh Ganu > <sughosh.g...@linaro.org>, Paul Liu <paul....@linaro.org> > Subject: Re: [PATCH v4 2/2] efi_loader: Reset system after > CapsuleUpdate on disk > Message-ID: <b69b5a40-14a2-6791-b63f-925b9b164...@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.hirama...@linaro.org> > > Reviewed-by: Heinrich Schuchardt <xypron.g...@gmx.de> > > > ------------------------------ > > Message: 21 > Date: Sun, 13 Feb 2022 10:58:17 +0100 > From: Heinrich Schuchardt <xypron.g...@gmx.de> > To: Masahisa Kojima <masahisa.koj...@linaro.org> > Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org>, Simon Glass > <s...@chromium.org>, Takahiro Akashi <takahiro.aka...@linaro.org>, > Francois Ozog <francois.o...@linaro.org>, Mark Kettenis > <mark.kette...@xs4all.nl>, u-boot@lists.denx.de > Subject: Re: [PATCH 3/3] efi_loader: add menu-driven UEFI Boot > Variable maintenance > Message-ID: <9e88e71a-ea98-98ad-e0db-1686c2045...@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.koj...@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 *)
