| Message ID | 20190905082133.18996-1-takahiro.akashi@linaro.org |
|---|---|
| Headers | show |
| Series | efi_loader: non-volatile variables support | expand |
One more issue: * Currently, Travis CI fails in lots of builds. I will try to fix them in the succeeding versions. -Takahiro Akashi On Thu, Sep 05, 2019 at 05:21:14PM +0900, AKASHI Takahiro wrote: > # In version 5 of this patch set, the implementation is changed again. > # > # I believe that this is NOT intrusive, and that my approach here is NOT > # selfish at all. If Wolfgang doesn't accept this approach, however, > # I would like to go for "Plan B" for UEFI variables implementation, in > # which EFI will have its own drivers for storage instead of env/*. > > This patch set is an attempt to implement non-volatile attribute for > UEFI variables. Under the current implementation, > * SetVariable API doesn't recognize non-volatile attribute > * While some variables are defined non-volatile in UEFI specification, > they are NOT marked as non-volatile in the code. > * env_save() (or "env save" command) allows us to save all the variables > into persistent storage, but it may cause volatile UEFI variables, > along with irrelevant U-Boot variables, to be saved unconditionally. > > Those observation rationalizes that the implementation of UEFI variables > should be revamped utilizing dedicated storage for them. > > > Basic ideas: > * Sub-system users of U-Boot environment variables may have their own > "env contexts". More than one contexts allowed. > > See Patch#2 and Patch#18. > > * Each context is isolated from other contexts with different name spaces. > * Each context is associated with one backing storage driver and media > location. > * Different contexts may use different drivers and locations. > > * To access (get or set) a variable, associated context must be presented. > So, almost of all existing env interfaces are changed to accept one > extra argument, ctx. > (I believe that this is Wolfgang's *requirement*.) > > From viewpoint of APIs, env context is a pointer to opaque structure. > > * Non-volatile feature is not implemented in a general form and must be > implemented by users in their sub-systems. > > In version 4, U-Boot environment's attributes are extended to support > non-volatile (or auto-save capability), but Wolfgang rejected > my approach. > As modifying attributes for this purpose would cause bunch of > incompatibility issues (as far as I said in my cover letter and > the discussions in ML), I would prefer a much simple approach. > > See patch#19 about how it is easily implemented for UEFI variables. > > * Each backing storage driver must be converted to be aligned with > new env interfaces to handle multiple contexts in parallel and > provide context-specific Kconfig configurations for driver parameters. > > In this version, only FAT file system and flash devices are supported, > but it is quite straightforward to modify other drivers. > > See Patch#4 and Patch#5 about how drivers can shift to new interfaces. > > * Finally, all existing U-Boot variables hold the same semantics > as before. > > > Known issues/restriction/TODO: > * The current form of patchset is not 'bisect'able. > Not to break 'bisect,' all the patches in this patch set must be > put into a single commit when merging. > (This can be mitigated by modifying/splitting Patch#18/#19 though.) > > * Unfortunately, this code fails to read U-Boot environment from flash > at boot time due to incomprehensible memory corruption. > See murky workaround, which is marked as FIXME, in env/flash.c. > > Despite this bug, I was still be able to run/test my patch by hacking > the code with gdb. > (modifying data to correct value on the fly :) > I hope that it won't affect code review in most places for now. > > * Some minor issues for better coding. > They are also marked as FIXME in the source code. > > * Only FAT file system and flash are supported. > > * The whole area of storage will be saved at *every* update of > one UEFI variable. It should be optimized if possible. > > * An error during "save" operation may cause inconsistency between > cache (hash table) and the storage. > -> This is not UEFI specific though. > > * Add tests if necessary. > > * I cannot test all the platforms affected by this patchset. > > > Note: > If we need "secure storage" for UEFI variables, efi_get_variable/ > efi_get_next_variable_name/efi_set_variable() should be completely > replaced with stub functions to communicate with secure world. > This patchset has nothing to do with such an implementation. > > > Usage: > To enable this feature for example with FAT file system, the following > configs must be enabled: > CONFIG_ENV_IS_IN_FAT > CONFIG_ENV_FAT_INTERFACE > CONFIG_ENV_EFI_FAT_DEVICE_AND_PART > CONFIG_ENV_EFI_FAT_FILE > > You may define a non-volatile variable from command interface: > => setenv -e -nv FOO baa > => printenv -e FOO > FOO: NV|BS|RT, DataSize = 0x3 > 00000000: 62 61 61 baa > > > Patch#1 and #2 are to add multiples 'context' support to env interfaces > and to provide new env interfaces. > Patch#3 to #5 are to show how the existing drivers for U-Boot environment > should be modified to be aligned with new env interfaces. > (Only FAT file system and flash in this version of patch set.) > Patch#6 to #17 are to shift all the existing users of old env interfaces > to new ones. (But not tested for all the platforms.) > Patch#18 and #19 are to modify UEFI variable implementation to utilize > new env interfaces. > > Changes in v5 (September 4, 2019) > * rebased to v2019.10-rc3 > * revamp the implementation, removing changes on environment variable's > attributes (See above) > > Changes in v4 (July 17, 2019) > * remove already-merged patches > * revamp after Wolfgang's suggestion > > Changes in v3 (June 4, 2019) > * remove already-merged patches > * revamp the code again > * introduce CONFIG_EFI_VARIABLE_USE_ENV for this configuration. > Once another backing storage, i.e. StMM services for secure boot, > is supported, another option will be added. > > Changes in v2 (Apr 24, 2019) > * rebased on efi-2019-07 > * revamp the implementation > > v1 (Nov 28, 2018) > * initial > > AKASHI Takahiro (19): > env: extend interfaces allowing for env contexts > env: define env context for U-Boot environment > env: nowhere: rework with new env interfaces > env: flash: support multiple env contexts > env: fat: support multiple env contexts > hashtable: support multiple env contexts > api: converted with new env interfaces > arch: converted with new env interfaces > board: converted with new env interfaces > cmd: converted with new env interfaces > common: converted with new env interfaces > disk: converted with new env interfaces > drivers: converted with new env interfaces > fs: converted with new env interfaces > lib: converted with new env interfaces (except efi_loader) > net: converted with new env interfaces > post: converted with new env interfaces > env,efi_loader: define env context for UEFI variables > efi_loader: variable: rework with new env interfaces > > api/api.c | 8 +- > arch/arc/lib/bootm.c | 2 +- > arch/arm/cpu/arm926ejs/spear/spr_misc.c | 8 +- > arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 5 +- > arch/arm/cpu/armv8/fsl-layerscape/soc.c | 14 +- > arch/arm/lib/bootm.c | 6 +- > arch/arm/lib/semihosting.c | 2 +- > arch/arm/mach-imx/mx6/opos6ul.c | 4 +- > arch/arm/mach-imx/mx7/soc.c | 4 +- > arch/arm/mach-imx/video.c | 2 +- > arch/arm/mach-keystone/ddr3.c | 2 +- > arch/arm/mach-keystone/keystone.c | 2 +- > arch/arm/mach-kirkwood/cpu.c | 4 +- > arch/arm/mach-meson/board-common.c | 2 +- > arch/arm/mach-omap2/utils.c | 20 +- > arch/arm/mach-rmobile/cpu_info.c | 2 +- > arch/arm/mach-rockchip/boot_mode.c | 4 +- > arch/arm/mach-rockchip/rk3288/rk3288.c | 2 +- > arch/arm/mach-socfpga/misc_gen5.c | 5 +- > arch/arm/mach-socfpga/misc_s10.c | 2 +- > arch/arm/mach-stm32mp/cpu.c | 35 +- > arch/arm/mach-tegra/board2.c | 4 +- > arch/arm/mach-tegra/cboot.c | 18 +- > arch/arm/mach-uniphier/board_late_init.c | 19 +- > arch/arm/mach-uniphier/mmc-first-dev.c | 2 +- > arch/m68k/lib/bootm.c | 2 +- > arch/microblaze/lib/bootm.c | 2 +- > arch/mips/lib/bootm.c | 6 +- > arch/nds32/lib/bootm.c | 4 +- > arch/powerpc/cpu/mpc85xx/cpu_init.c | 10 +- > arch/powerpc/cpu/mpc85xx/fdt.c | 2 +- > arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c | 2 +- > arch/powerpc/lib/bootm.c | 2 +- > arch/sh/lib/bootm.c | 2 +- > arch/sh/lib/zimageboot.c | 2 +- > arch/x86/lib/zimage.c | 11 +- > arch/xtensa/lib/bootm.c | 2 +- > board/Arcturus/ucp1020/cmd_arc.c | 40 +- > board/Arcturus/ucp1020/ucp1020.c | 16 +- > board/BuR/brppt1/board.c | 4 +- > board/BuR/brxre1/board.c | 9 +- > board/BuR/common/br_resetc.c | 2 +- > board/BuR/common/common.c | 47 +- > board/BuS/eb_cpu5282/eb_cpu5282.c | 8 +- > board/CZ.NIC/turris_mox/turris_mox.c | 4 +- > board/CZ.NIC/turris_omnia/turris_omnia.c | 6 +- > board/CarMediaLab/flea3/flea3.c | 2 +- > board/LaCie/net2big_v2/net2big_v2.c | 2 +- > board/LaCie/netspace_v2/netspace_v2.c | 2 +- > board/Synology/ds414/cmd_syno.c | 6 +- > board/alliedtelesis/x530/x530.c | 2 +- > board/amazon/kc1/kc1.c | 4 +- > board/amlogic/p200/p200.c | 4 +- > board/amlogic/p201/p201.c | 4 +- > board/amlogic/p212/p212.c | 4 +- > board/amlogic/q200/q200.c | 4 +- > board/aristainetos/aristainetos-v2.c | 8 +- > board/armltd/integrator/integrator.c | 2 +- > board/atmel/common/board.c | 4 +- > board/atmel/common/mac_eeprom.c | 2 +- > board/atmel/sama5d3xek/sama5d3xek.c | 2 +- > board/bachmann/ot1200/ot1200.c | 4 +- > board/birdland/bav335x/board.c | 8 +- > board/bluegiga/apx4devkit/apx4devkit.c | 5 +- > board/bluewater/gurnard/gurnard.c | 6 +- > board/bosch/shc/board.c | 2 +- > board/boundary/nitrogen6x/nitrogen6x.c | 14 +- > board/broadcom/bcm23550_w1d/bcm23550_w1d.c | 2 +- > board/broadcom/bcm28155_ap/bcm28155_ap.c | 2 +- > board/broadcom/bcmstb/bcmstb.c | 2 +- > board/buffalo/lsxl/lsxl.c | 2 +- > board/cadence/xtfpga/xtfpga.c | 4 +- > board/ccv/xpress/xpress.c | 2 +- > board/compulab/cl-som-imx7/cl-som-imx7.c | 2 +- > board/compulab/cm_fx6/cm_fx6.c | 10 +- > board/compulab/common/omap3_display.c | 4 +- > board/congatec/cgtqmx6eval/cgtqmx6eval.c | 8 +- > board/cssi/MCR3000/MCR3000.c | 2 +- > board/davinci/da8xxevm/da850evm.c | 2 +- > board/davinci/da8xxevm/omapl138_lcdk.c | 6 +- > board/dhelectronics/dh_imx6/dh_imx6.c | 2 +- > board/eets/pdu001/board.c | 8 +- > board/el/el6x/el6x.c | 2 +- > board/emulation/qemu-riscv/qemu-riscv.c | 2 +- > board/engicam/common/board.c | 32 +- > board/esd/meesc/meesc.c | 6 +- > board/freescale/b4860qds/b4860qds.c | 5 +- > board/freescale/common/cmd_esbc_validate.c | 2 +- > board/freescale/common/fsl_chain_of_trust.c | 6 +- > board/freescale/common/sys_eeprom.c | 4 +- > board/freescale/common/vid.c | 4 +- > board/freescale/imx8mq_evk/imx8mq_evk.c | 4 +- > board/freescale/imx8qm_mek/imx8qm_mek.c | 4 +- > board/freescale/imx8qxp_mek/imx8qxp_mek.c | 4 +- > board/freescale/ls1088a/eth_ls1088aqds.c | 4 +- > board/freescale/ls1088a/ls1088a.c | 2 +- > board/freescale/ls2080aqds/eth.c | 6 +- > board/freescale/ls2080aqds/ls2080aqds.c | 2 +- > board/freescale/ls2080ardb/ls2080ardb.c | 6 +- > board/freescale/lx2160a/eth_lx2160aqds.c | 2 +- > board/freescale/mpc8323erdb/mpc8323erdb.c | 3 +- > board/freescale/mpc837xemds/pci.c | 2 +- > board/freescale/mpc837xerdb/mpc837xerdb.c | 2 +- > board/freescale/mx51evk/mx51evk_video.c | 2 +- > board/freescale/mx53loco/mx53loco.c | 4 +- > board/freescale/mx53loco/mx53loco_video.c | 2 +- > board/freescale/mx6sabreauto/mx6sabreauto.c | 8 +- > board/freescale/mx6sabresd/mx6sabresd.c | 8 +- > board/freescale/mx6sxsabresd/mx6sxsabresd.c | 2 +- > .../mx6ul_14x14_evk/mx6ul_14x14_evk.c | 6 +- > board/freescale/mx6ullevk/mx6ullevk.c | 4 +- > board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c | 2 +- > board/freescale/qemu-ppce500/qemu-ppce500.c | 4 +- > board/freescale/t4qds/t4240qds.c | 2 +- > board/gardena/smart-gateway-at91sam/board.c | 2 +- > board/gardena/smart-gateway-mt7688/board.c | 16 +- > board/gateworks/gw_ventana/common.c | 2 +- > board/gateworks/gw_ventana/gw_ventana.c | 61 +- > board/gateworks/gw_ventana/gw_ventana_spl.c | 4 +- > board/gdsys/a38x/keyprogram.c | 4 +- > board/gdsys/mpc8308/gazerbeam.c | 4 +- > board/gdsys/mpc8308/hrcon.c | 2 +- > board/gdsys/mpc8308/strider.c | 2 +- > board/gdsys/p1022/controlcenterd-id.c | 10 +- > board/gdsys/p1022/controlcenterd.c | 2 +- > board/ge/bx50v3/bx50v3.c | 13 +- > board/ge/common/ge_common.c | 4 +- > board/ge/mx53ppd/mx53ppd.c | 2 +- > board/grinn/chiliboard/board.c | 4 +- > board/grinn/liteboard/board.c | 6 +- > board/highbank/highbank.c | 9 +- > board/hisilicon/poplar/poplar.c | 2 +- > board/imgtec/ci20/ci20.c | 6 +- > board/intel/edison/edison.c | 14 +- > board/isee/igep003x/board.c | 6 +- > board/isee/igep00x0/igep00x0.c | 4 +- > board/k+p/kp_imx53/kp_id_rev.c | 20 +- > board/k+p/kp_imx53/kp_imx53.c | 4 +- > board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c | 4 +- > board/keymile/common/common.c | 26 +- > board/keymile/common/ivm.c | 8 +- > board/keymile/km83xx/km83xx.c | 2 +- > board/keymile/km_arm/km_arm.c | 6 +- > board/keymile/kmp204x/kmp204x.c | 4 +- > board/kosagi/novena/novena.c | 2 +- > board/laird/wb50n/wb50n.c | 2 +- > board/lg/sniper/sniper.c | 4 +- > board/liebherr/display5/spl.c | 4 +- > board/liebherr/mccmon6/mccmon6.c | 6 +- > board/logicpd/imx6/imx6logic.c | 8 +- > board/menlo/m53menlo/m53menlo.c | 2 +- > board/micronas/vct/vct.c | 2 +- > board/nokia/rx51/rx51.c | 10 +- > board/overo/overo.c | 45 +- > board/phytec/pcm052/pcm052.c | 4 +- > board/phytec/pfla02/pfla02.c | 2 +- > .../dragonboard410c/dragonboard410c.c | 6 +- > .../dragonboard820c/dragonboard820c.c | 2 +- > board/raspberrypi/rpi/rpi.c | 26 +- > board/renesas/alt/alt.c | 3 +- > board/renesas/gose/gose.c | 3 +- > board/renesas/koelsch/koelsch.c | 3 +- > board/renesas/lager/lager.c | 3 +- > board/renesas/porter/porter.c | 3 +- > board/renesas/sh7752evb/sh7752evb.c | 4 +- > board/renesas/sh7753evb/sh7753evb.c | 4 +- > board/renesas/sh7757lcr/sh7757lcr.c | 6 +- > board/renesas/silk/silk.c | 3 +- > board/renesas/stout/stout.c | 3 +- > board/rockchip/kylin_rk3036/kylin_rk3036.c | 2 +- > board/samsung/common/exynos5-dt.c | 2 +- > board/samsung/common/misc.c | 14 +- > board/samsung/odroid/odroid.c | 2 +- > board/samsung/trats/trats.c | 2 +- > board/samsung/universal_c210/universal.c | 2 +- > board/samtec/vining_fpga/socfpga.c | 16 +- > board/siemens/common/board.c | 4 +- > board/siemens/draco/board.c | 6 +- > board/siemens/pxm2/board.c | 4 +- > board/siemens/rut/board.c | 2 +- > board/siemens/taurus/taurus.c | 50 +- > board/socrates/socrates.c | 4 +- > board/softing/vining_2000/vining_2000.c | 8 +- > board/solidrun/mx6cuboxi/mx6cuboxi.c | 16 +- > .../stm32f429-discovery/stm32f429-discovery.c | 4 +- > .../stm32f429-evaluation.c | 4 +- > .../stm32f469-discovery/stm32f469-discovery.c | 4 +- > board/st/stm32mp1/stm32mp1.c | 11 +- > board/sunxi/board.c | 25 +- > board/synopsys/hsdk/env-lib.c | 11 +- > board/synopsys/hsdk/hsdk.c | 6 +- > board/syteco/zmx25/zmx25.c | 8 +- > board/tcl/sl50/board.c | 6 +- > .../puma_rk3399/puma-rk3399.c | 8 +- > board/ti/am335x/board.c | 18 +- > board/ti/am43xx/board.c | 6 +- > board/ti/am57xx/board.c | 14 +- > board/ti/beagle/beagle.c | 43 +- > board/ti/common/board_detect.c | 14 +- > board/ti/dra7xx/evm.c | 12 +- > board/ti/evm/evm.c | 2 +- > board/ti/ks2_evm/board.c | 10 +- > board/ti/ks2_evm/board_k2g.c | 8 +- > board/ti/panda/panda.c | 4 +- > board/toradex/apalis-imx8/apalis-imx8.c | 4 +- > board/toradex/apalis_imx6/apalis_imx6.c | 12 +- > .../toradex/colibri-imx6ull/colibri-imx6ull.c | 6 +- > board/toradex/colibri-imx8x/colibri-imx8x.c | 4 +- > board/toradex/colibri_imx6/colibri_imx6.c | 8 +- > board/toradex/colibri_vf/colibri_vf.c | 2 +- > board/toradex/common/tdx-cfg-block.c | 2 +- > board/toradex/common/tdx-common.c | 2 +- > board/tqc/tqma6/tqma6.c | 2 +- > board/udoo/neo/neo.c | 2 +- > board/udoo/udoo.c | 4 +- > board/varisys/common/sys_eeprom.c | 6 +- > board/vscom/baltos/board.c | 6 +- > board/wandboard/wandboard.c | 12 +- > board/warp7/warp7.c | 6 +- > .../work_92105/work_92105_display.c | 2 +- > board/xes/common/board.c | 6 +- > board/xilinx/zynq/board.c | 16 +- > board/xilinx/zynqmp/cmds.c | 2 +- > board/xilinx/zynqmp/zynqmp.c | 24 +- > cmd/ab_select.c | 2 +- > cmd/avb.c | 2 +- > cmd/bdinfo.c | 6 +- > cmd/binop.c | 4 +- > cmd/bootefi.c | 8 +- > cmd/bootm.c | 4 +- > cmd/bootmenu.c | 6 +- > cmd/cbfs.c | 2 +- > cmd/cramfs.c | 8 +- > cmd/dtimg.c | 2 +- > cmd/elf.c | 29 +- > cmd/fdt.c | 22 +- > cmd/fpga.c | 4 +- > cmd/gpt.c | 8 +- > cmd/ini.c | 6 +- > cmd/itest.c | 2 +- > cmd/jffs2.c | 4 +- > cmd/load.c | 10 +- > cmd/lzmadec.c | 2 +- > cmd/md5sum.c | 4 +- > cmd/mtdparts.c | 41 +- > cmd/mvebu/bubt.c | 2 +- > cmd/nand.c | 12 +- > cmd/net.c | 46 +- > cmd/nvedit.c | 394 +++++++--- > cmd/part.c | 6 +- > cmd/pxe.c | 33 +- > cmd/qfw.c | 6 +- > cmd/reiser.c | 8 +- > cmd/setexpr.c | 8 +- > cmd/spl.c | 5 +- > cmd/ti/ddr3.c | 2 +- > cmd/tpm-common.c | 2 +- > cmd/tpm-v1.c | 2 +- > cmd/trace.c | 18 +- > cmd/ubi.c | 2 +- > cmd/unzip.c | 2 +- > cmd/ximg.c | 4 +- > cmd/zfs.c | 6 +- > cmd/zip.c | 2 +- > common/autoboot.c | 22 +- > common/board_f.c | 3 +- > common/board_r.c | 10 +- > common/bootm.c | 12 +- > common/bootm_os.c | 12 +- > common/bootretry.c | 2 +- > common/cli.c | 2 +- > common/cli_hush.c | 14 +- > common/cli_simple.c | 2 +- > common/command.c | 2 +- > common/console.c | 14 +- > common/fdt_support.c | 6 +- > common/hash.c | 4 +- > common/hwconfig.c | 5 +- > common/image-android.c | 4 +- > common/image-fdt.c | 4 +- > common/image.c | 15 +- > common/main.c | 5 +- > common/spl/spl_dfu.c | 6 +- > common/spl/spl_ext.c | 4 +- > common/spl/spl_fat.c | 4 +- > common/spl/spl_net.c | 4 +- > common/splash.c | 4 +- > common/splash_source.c | 8 +- > common/update.c | 10 +- > common/usb_hub.c | 2 +- > common/usb_kbd.c | 6 +- > disk/part.c | 2 +- > disk/part_amiga.c | 4 +- > drivers/bootcount/bootcount_env.c | 12 +- > drivers/ddr/fsl/fsl_ddr_gen4.c | 2 +- > drivers/ddr/fsl/interactive.c | 5 +- > drivers/ddr/fsl/options.c | 4 +- > drivers/dfu/dfu.c | 6 +- > drivers/fastboot/fb_command.c | 4 +- > drivers/fastboot/fb_common.c | 2 +- > drivers/fastboot/fb_getvar.c | 8 +- > drivers/fastboot/fb_mmc.c | 2 +- > drivers/input/i8042.c | 2 +- > drivers/input/input.c | 2 +- > drivers/misc/fs_loader.c | 8 +- > drivers/mtd/cfi_flash.c | 2 +- > drivers/mtd/mtd_uboot.c | 11 +- > drivers/net/fec_mxc.c | 2 +- > drivers/net/fm/b4860.c | 3 +- > drivers/net/fm/fdt.c | 2 +- > drivers/net/fm/fm.c | 4 +- > drivers/net/fsl-mc/mc.c | 7 +- > drivers/net/netconsole.c | 14 +- > drivers/net/phy/micrel_ksz90x1.c | 4 +- > drivers/net/sandbox-raw.c | 4 +- > drivers/pci/pci.c | 4 +- > drivers/pci/pci_common.c | 2 +- > drivers/reset/reset-socfpga.c | 2 +- > drivers/rtc/m41t60.c | 2 +- > drivers/scsi/scsi.c | 2 +- > drivers/serial/usbtty.c | 4 +- > drivers/usb/gadget/designware_udc.c | 2 +- > drivers/usb/gadget/ether.c | 13 +- > drivers/usb/gadget/f_dfu.c | 2 +- > drivers/usb/gadget/f_fastboot.c | 2 +- > drivers/usb/gadget/f_rockusb.c | 2 +- > drivers/usb/gadget/f_sdp.c | 2 +- > drivers/usb/host/ehci-fsl.c | 2 +- > drivers/video/ati_radeon_fb.c | 2 +- > drivers/video/cfb_console.c | 2 +- > drivers/video/mb862xx.c | 2 +- > drivers/video/mx3fb.c | 2 +- > drivers/video/mxsfb.c | 2 +- > drivers/video/videomodes.c | 4 +- > env/Kconfig | 683 +----------------- > env/Kconfig.efi | 152 ++++ > env/Kconfig.uboot | 671 +++++++++++++++++ > env/Makefile | 33 +- > env/callback.c | 40 +- > env/common.c | 255 ++++--- > env/env.c | 158 ++-- > env/env_ctx_efi.c | 131 ++++ > env/env_ctx_uboot.c | 292 ++++++++ > env/fat.c | 102 ++- > env/flags.c | 35 +- > env/flash.c | 397 ++++++---- > env/nowhere.c | 7 +- > fs/fs.c | 14 +- > fs/ubifs/ubifs.c | 2 +- > include/_exports.h | 6 +- > include/common.h | 6 +- > include/env.h | 114 ++- > include/env_internal.h | 89 ++- > include/exports.h | 5 +- > include/search.h | 6 +- > lib/efi_loader/efi_console.c | 2 +- > lib/efi_loader/efi_variable.c | 91 ++- > lib/fdtdec.c | 2 +- > lib/hashtable.c | 14 +- > lib/smbios.c | 2 +- > lib/uuid.c | 2 +- > net/bootp.c | 17 +- > net/dns.c | 2 +- > net/eth-uclass.c | 6 +- > net/eth_common.c | 18 +- > net/eth_legacy.c | 2 +- > net/link_local.c | 2 +- > net/net.c | 11 +- > net/tftp.c | 10 +- > net/wol.c | 2 +- > post/post.c | 2 +- > 371 files changed, 3690 insertions(+), 2337 deletions(-) > create mode 100644 env/Kconfig.efi > create mode 100644 env/Kconfig.uboot > create mode 100644 env/env_ctx_efi.c > create mode 100644 env/env_ctx_uboot.c > > -- > 2.21.0 >
Wolfgang, I haven't seen any comments from you in the last one month. Could you please take time to review this patch? I'd like to confirm whether you can agree to my approach here or not in order to take the next step. Thanks, -Takahiro Akashi On Thu, Sep 05, 2019 at 05:21:14PM +0900, AKASHI Takahiro wrote: > # In version 5 of this patch set, the implementation is changed again. > # > # I believe that this is NOT intrusive, and that my approach here is NOT > # selfish at all. If Wolfgang doesn't accept this approach, however, > # I would like to go for "Plan B" for UEFI variables implementation, in > # which EFI will have its own drivers for storage instead of env/*. > > This patch set is an attempt to implement non-volatile attribute for > UEFI variables. Under the current implementation, > * SetVariable API doesn't recognize non-volatile attribute > * While some variables are defined non-volatile in UEFI specification, > they are NOT marked as non-volatile in the code. > * env_save() (or "env save" command) allows us to save all the variables > into persistent storage, but it may cause volatile UEFI variables, > along with irrelevant U-Boot variables, to be saved unconditionally. > > Those observation rationalizes that the implementation of UEFI variables > should be revamped utilizing dedicated storage for them. > > > Basic ideas: > * Sub-system users of U-Boot environment variables may have their own > "env contexts". More than one contexts allowed. > > See Patch#2 and Patch#18. > > * Each context is isolated from other contexts with different name spaces. > * Each context is associated with one backing storage driver and media > location. > * Different contexts may use different drivers and locations. > > * To access (get or set) a variable, associated context must be presented. > So, almost of all existing env interfaces are changed to accept one > extra argument, ctx. > (I believe that this is Wolfgang's *requirement*.) > > From viewpoint of APIs, env context is a pointer to opaque structure. > > * Non-volatile feature is not implemented in a general form and must be > implemented by users in their sub-systems. > > In version 4, U-Boot environment's attributes are extended to support > non-volatile (or auto-save capability), but Wolfgang rejected > my approach. > As modifying attributes for this purpose would cause bunch of > incompatibility issues (as far as I said in my cover letter and > the discussions in ML), I would prefer a much simple approach. > > See patch#19 about how it is easily implemented for UEFI variables. > > * Each backing storage driver must be converted to be aligned with > new env interfaces to handle multiple contexts in parallel and > provide context-specific Kconfig configurations for driver parameters. > > In this version, only FAT file system and flash devices are supported, > but it is quite straightforward to modify other drivers. > > See Patch#4 and Patch#5 about how drivers can shift to new interfaces. > > * Finally, all existing U-Boot variables hold the same semantics > as before. > > > Known issues/restriction/TODO: > * The current form of patchset is not 'bisect'able. > Not to break 'bisect,' all the patches in this patch set must be > put into a single commit when merging. > (This can be mitigated by modifying/splitting Patch#18/#19 though.) > > * Unfortunately, this code fails to read U-Boot environment from flash > at boot time due to incomprehensible memory corruption. > See murky workaround, which is marked as FIXME, in env/flash.c. > > Despite this bug, I was still be able to run/test my patch by hacking > the code with gdb. > (modifying data to correct value on the fly :) > I hope that it won't affect code review in most places for now. > > * Some minor issues for better coding. > They are also marked as FIXME in the source code. > > * Only FAT file system and flash are supported. > > * The whole area of storage will be saved at *every* update of > one UEFI variable. It should be optimized if possible. > > * An error during "save" operation may cause inconsistency between > cache (hash table) and the storage. > -> This is not UEFI specific though. > > * Add tests if necessary. > > * I cannot test all the platforms affected by this patchset. > > > Note: > If we need "secure storage" for UEFI variables, efi_get_variable/ > efi_get_next_variable_name/efi_set_variable() should be completely > replaced with stub functions to communicate with secure world. > This patchset has nothing to do with such an implementation. > > > Usage: > To enable this feature for example with FAT file system, the following > configs must be enabled: > CONFIG_ENV_IS_IN_FAT > CONFIG_ENV_FAT_INTERFACE > CONFIG_ENV_EFI_FAT_DEVICE_AND_PART > CONFIG_ENV_EFI_FAT_FILE > > You may define a non-volatile variable from command interface: > => setenv -e -nv FOO baa > => printenv -e FOO > FOO: NV|BS|RT, DataSize = 0x3 > 00000000: 62 61 61 baa > > > Patch#1 and #2 are to add multiples 'context' support to env interfaces > and to provide new env interfaces. > Patch#3 to #5 are to show how the existing drivers for U-Boot environment > should be modified to be aligned with new env interfaces. > (Only FAT file system and flash in this version of patch set.) > Patch#6 to #17 are to shift all the existing users of old env interfaces > to new ones. (But not tested for all the platforms.) > Patch#18 and #19 are to modify UEFI variable implementation to utilize > new env interfaces. > > Changes in v5 (September 4, 2019) > * rebased to v2019.10-rc3 > * revamp the implementation, removing changes on environment variable's > attributes (See above) > > Changes in v4 (July 17, 2019) > * remove already-merged patches > * revamp after Wolfgang's suggestion > > Changes in v3 (June 4, 2019) > * remove already-merged patches > * revamp the code again > * introduce CONFIG_EFI_VARIABLE_USE_ENV for this configuration. > Once another backing storage, i.e. StMM services for secure boot, > is supported, another option will be added. > > Changes in v2 (Apr 24, 2019) > * rebased on efi-2019-07 > * revamp the implementation > > v1 (Nov 28, 2018) > * initial > > AKASHI Takahiro (19): > env: extend interfaces allowing for env contexts > env: define env context for U-Boot environment > env: nowhere: rework with new env interfaces > env: flash: support multiple env contexts > env: fat: support multiple env contexts > hashtable: support multiple env contexts > api: converted with new env interfaces > arch: converted with new env interfaces > board: converted with new env interfaces > cmd: converted with new env interfaces > common: converted with new env interfaces > disk: converted with new env interfaces > drivers: converted with new env interfaces > fs: converted with new env interfaces > lib: converted with new env interfaces (except efi_loader) > net: converted with new env interfaces > post: converted with new env interfaces > env,efi_loader: define env context for UEFI variables > efi_loader: variable: rework with new env interfaces > > api/api.c | 8 +- > arch/arc/lib/bootm.c | 2 +- > arch/arm/cpu/arm926ejs/spear/spr_misc.c | 8 +- > arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 5 +- > arch/arm/cpu/armv8/fsl-layerscape/soc.c | 14 +- > arch/arm/lib/bootm.c | 6 +- > arch/arm/lib/semihosting.c | 2 +- > arch/arm/mach-imx/mx6/opos6ul.c | 4 +- > arch/arm/mach-imx/mx7/soc.c | 4 +- > arch/arm/mach-imx/video.c | 2 +- > arch/arm/mach-keystone/ddr3.c | 2 +- > arch/arm/mach-keystone/keystone.c | 2 +- > arch/arm/mach-kirkwood/cpu.c | 4 +- > arch/arm/mach-meson/board-common.c | 2 +- > arch/arm/mach-omap2/utils.c | 20 +- > arch/arm/mach-rmobile/cpu_info.c | 2 +- > arch/arm/mach-rockchip/boot_mode.c | 4 +- > arch/arm/mach-rockchip/rk3288/rk3288.c | 2 +- > arch/arm/mach-socfpga/misc_gen5.c | 5 +- > arch/arm/mach-socfpga/misc_s10.c | 2 +- > arch/arm/mach-stm32mp/cpu.c | 35 +- > arch/arm/mach-tegra/board2.c | 4 +- > arch/arm/mach-tegra/cboot.c | 18 +- > arch/arm/mach-uniphier/board_late_init.c | 19 +- > arch/arm/mach-uniphier/mmc-first-dev.c | 2 +- > arch/m68k/lib/bootm.c | 2 +- > arch/microblaze/lib/bootm.c | 2 +- > arch/mips/lib/bootm.c | 6 +- > arch/nds32/lib/bootm.c | 4 +- > arch/powerpc/cpu/mpc85xx/cpu_init.c | 10 +- > arch/powerpc/cpu/mpc85xx/fdt.c | 2 +- > arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c | 2 +- > arch/powerpc/lib/bootm.c | 2 +- > arch/sh/lib/bootm.c | 2 +- > arch/sh/lib/zimageboot.c | 2 +- > arch/x86/lib/zimage.c | 11 +- > arch/xtensa/lib/bootm.c | 2 +- > board/Arcturus/ucp1020/cmd_arc.c | 40 +- > board/Arcturus/ucp1020/ucp1020.c | 16 +- > board/BuR/brppt1/board.c | 4 +- > board/BuR/brxre1/board.c | 9 +- > board/BuR/common/br_resetc.c | 2 +- > board/BuR/common/common.c | 47 +- > board/BuS/eb_cpu5282/eb_cpu5282.c | 8 +- > board/CZ.NIC/turris_mox/turris_mox.c | 4 +- > board/CZ.NIC/turris_omnia/turris_omnia.c | 6 +- > board/CarMediaLab/flea3/flea3.c | 2 +- > board/LaCie/net2big_v2/net2big_v2.c | 2 +- > board/LaCie/netspace_v2/netspace_v2.c | 2 +- > board/Synology/ds414/cmd_syno.c | 6 +- > board/alliedtelesis/x530/x530.c | 2 +- > board/amazon/kc1/kc1.c | 4 +- > board/amlogic/p200/p200.c | 4 +- > board/amlogic/p201/p201.c | 4 +- > board/amlogic/p212/p212.c | 4 +- > board/amlogic/q200/q200.c | 4 +- > board/aristainetos/aristainetos-v2.c | 8 +- > board/armltd/integrator/integrator.c | 2 +- > board/atmel/common/board.c | 4 +- > board/atmel/common/mac_eeprom.c | 2 +- > board/atmel/sama5d3xek/sama5d3xek.c | 2 +- > board/bachmann/ot1200/ot1200.c | 4 +- > board/birdland/bav335x/board.c | 8 +- > board/bluegiga/apx4devkit/apx4devkit.c | 5 +- > board/bluewater/gurnard/gurnard.c | 6 +- > board/bosch/shc/board.c | 2 +- > board/boundary/nitrogen6x/nitrogen6x.c | 14 +- > board/broadcom/bcm23550_w1d/bcm23550_w1d.c | 2 +- > board/broadcom/bcm28155_ap/bcm28155_ap.c | 2 +- > board/broadcom/bcmstb/bcmstb.c | 2 +- > board/buffalo/lsxl/lsxl.c | 2 +- > board/cadence/xtfpga/xtfpga.c | 4 +- > board/ccv/xpress/xpress.c | 2 +- > board/compulab/cl-som-imx7/cl-som-imx7.c | 2 +- > board/compulab/cm_fx6/cm_fx6.c | 10 +- > board/compulab/common/omap3_display.c | 4 +- > board/congatec/cgtqmx6eval/cgtqmx6eval.c | 8 +- > board/cssi/MCR3000/MCR3000.c | 2 +- > board/davinci/da8xxevm/da850evm.c | 2 +- > board/davinci/da8xxevm/omapl138_lcdk.c | 6 +- > board/dhelectronics/dh_imx6/dh_imx6.c | 2 +- > board/eets/pdu001/board.c | 8 +- > board/el/el6x/el6x.c | 2 +- > board/emulation/qemu-riscv/qemu-riscv.c | 2 +- > board/engicam/common/board.c | 32 +- > board/esd/meesc/meesc.c | 6 +- > board/freescale/b4860qds/b4860qds.c | 5 +- > board/freescale/common/cmd_esbc_validate.c | 2 +- > board/freescale/common/fsl_chain_of_trust.c | 6 +- > board/freescale/common/sys_eeprom.c | 4 +- > board/freescale/common/vid.c | 4 +- > board/freescale/imx8mq_evk/imx8mq_evk.c | 4 +- > board/freescale/imx8qm_mek/imx8qm_mek.c | 4 +- > board/freescale/imx8qxp_mek/imx8qxp_mek.c | 4 +- > board/freescale/ls1088a/eth_ls1088aqds.c | 4 +- > board/freescale/ls1088a/ls1088a.c | 2 +- > board/freescale/ls2080aqds/eth.c | 6 +- > board/freescale/ls2080aqds/ls2080aqds.c | 2 +- > board/freescale/ls2080ardb/ls2080ardb.c | 6 +- > board/freescale/lx2160a/eth_lx2160aqds.c | 2 +- > board/freescale/mpc8323erdb/mpc8323erdb.c | 3 +- > board/freescale/mpc837xemds/pci.c | 2 +- > board/freescale/mpc837xerdb/mpc837xerdb.c | 2 +- > board/freescale/mx51evk/mx51evk_video.c | 2 +- > board/freescale/mx53loco/mx53loco.c | 4 +- > board/freescale/mx53loco/mx53loco_video.c | 2 +- > board/freescale/mx6sabreauto/mx6sabreauto.c | 8 +- > board/freescale/mx6sabresd/mx6sabresd.c | 8 +- > board/freescale/mx6sxsabresd/mx6sxsabresd.c | 2 +- > .../mx6ul_14x14_evk/mx6ul_14x14_evk.c | 6 +- > board/freescale/mx6ullevk/mx6ullevk.c | 4 +- > board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c | 2 +- > board/freescale/qemu-ppce500/qemu-ppce500.c | 4 +- > board/freescale/t4qds/t4240qds.c | 2 +- > board/gardena/smart-gateway-at91sam/board.c | 2 +- > board/gardena/smart-gateway-mt7688/board.c | 16 +- > board/gateworks/gw_ventana/common.c | 2 +- > board/gateworks/gw_ventana/gw_ventana.c | 61 +- > board/gateworks/gw_ventana/gw_ventana_spl.c | 4 +- > board/gdsys/a38x/keyprogram.c | 4 +- > board/gdsys/mpc8308/gazerbeam.c | 4 +- > board/gdsys/mpc8308/hrcon.c | 2 +- > board/gdsys/mpc8308/strider.c | 2 +- > board/gdsys/p1022/controlcenterd-id.c | 10 +- > board/gdsys/p1022/controlcenterd.c | 2 +- > board/ge/bx50v3/bx50v3.c | 13 +- > board/ge/common/ge_common.c | 4 +- > board/ge/mx53ppd/mx53ppd.c | 2 +- > board/grinn/chiliboard/board.c | 4 +- > board/grinn/liteboard/board.c | 6 +- > board/highbank/highbank.c | 9 +- > board/hisilicon/poplar/poplar.c | 2 +- > board/imgtec/ci20/ci20.c | 6 +- > board/intel/edison/edison.c | 14 +- > board/isee/igep003x/board.c | 6 +- > board/isee/igep00x0/igep00x0.c | 4 +- > board/k+p/kp_imx53/kp_id_rev.c | 20 +- > board/k+p/kp_imx53/kp_imx53.c | 4 +- > board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c | 4 +- > board/keymile/common/common.c | 26 +- > board/keymile/common/ivm.c | 8 +- > board/keymile/km83xx/km83xx.c | 2 +- > board/keymile/km_arm/km_arm.c | 6 +- > board/keymile/kmp204x/kmp204x.c | 4 +- > board/kosagi/novena/novena.c | 2 +- > board/laird/wb50n/wb50n.c | 2 +- > board/lg/sniper/sniper.c | 4 +- > board/liebherr/display5/spl.c | 4 +- > board/liebherr/mccmon6/mccmon6.c | 6 +- > board/logicpd/imx6/imx6logic.c | 8 +- > board/menlo/m53menlo/m53menlo.c | 2 +- > board/micronas/vct/vct.c | 2 +- > board/nokia/rx51/rx51.c | 10 +- > board/overo/overo.c | 45 +- > board/phytec/pcm052/pcm052.c | 4 +- > board/phytec/pfla02/pfla02.c | 2 +- > .../dragonboard410c/dragonboard410c.c | 6 +- > .../dragonboard820c/dragonboard820c.c | 2 +- > board/raspberrypi/rpi/rpi.c | 26 +- > board/renesas/alt/alt.c | 3 +- > board/renesas/gose/gose.c | 3 +- > board/renesas/koelsch/koelsch.c | 3 +- > board/renesas/lager/lager.c | 3 +- > board/renesas/porter/porter.c | 3 +- > board/renesas/sh7752evb/sh7752evb.c | 4 +- > board/renesas/sh7753evb/sh7753evb.c | 4 +- > board/renesas/sh7757lcr/sh7757lcr.c | 6 +- > board/renesas/silk/silk.c | 3 +- > board/renesas/stout/stout.c | 3 +- > board/rockchip/kylin_rk3036/kylin_rk3036.c | 2 +- > board/samsung/common/exynos5-dt.c | 2 +- > board/samsung/common/misc.c | 14 +- > board/samsung/odroid/odroid.c | 2 +- > board/samsung/trats/trats.c | 2 +- > board/samsung/universal_c210/universal.c | 2 +- > board/samtec/vining_fpga/socfpga.c | 16 +- > board/siemens/common/board.c | 4 +- > board/siemens/draco/board.c | 6 +- > board/siemens/pxm2/board.c | 4 +- > board/siemens/rut/board.c | 2 +- > board/siemens/taurus/taurus.c | 50 +- > board/socrates/socrates.c | 4 +- > board/softing/vining_2000/vining_2000.c | 8 +- > board/solidrun/mx6cuboxi/mx6cuboxi.c | 16 +- > .../stm32f429-discovery/stm32f429-discovery.c | 4 +- > .../stm32f429-evaluation.c | 4 +- > .../stm32f469-discovery/stm32f469-discovery.c | 4 +- > board/st/stm32mp1/stm32mp1.c | 11 +- > board/sunxi/board.c | 25 +- > board/synopsys/hsdk/env-lib.c | 11 +- > board/synopsys/hsdk/hsdk.c | 6 +- > board/syteco/zmx25/zmx25.c | 8 +- > board/tcl/sl50/board.c | 6 +- > .../puma_rk3399/puma-rk3399.c | 8 +- > board/ti/am335x/board.c | 18 +- > board/ti/am43xx/board.c | 6 +- > board/ti/am57xx/board.c | 14 +- > board/ti/beagle/beagle.c | 43 +- > board/ti/common/board_detect.c | 14 +- > board/ti/dra7xx/evm.c | 12 +- > board/ti/evm/evm.c | 2 +- > board/ti/ks2_evm/board.c | 10 +- > board/ti/ks2_evm/board_k2g.c | 8 +- > board/ti/panda/panda.c | 4 +- > board/toradex/apalis-imx8/apalis-imx8.c | 4 +- > board/toradex/apalis_imx6/apalis_imx6.c | 12 +- > .../toradex/colibri-imx6ull/colibri-imx6ull.c | 6 +- > board/toradex/colibri-imx8x/colibri-imx8x.c | 4 +- > board/toradex/colibri_imx6/colibri_imx6.c | 8 +- > board/toradex/colibri_vf/colibri_vf.c | 2 +- > board/toradex/common/tdx-cfg-block.c | 2 +- > board/toradex/common/tdx-common.c | 2 +- > board/tqc/tqma6/tqma6.c | 2 +- > board/udoo/neo/neo.c | 2 +- > board/udoo/udoo.c | 4 +- > board/varisys/common/sys_eeprom.c | 6 +- > board/vscom/baltos/board.c | 6 +- > board/wandboard/wandboard.c | 12 +- > board/warp7/warp7.c | 6 +- > .../work_92105/work_92105_display.c | 2 +- > board/xes/common/board.c | 6 +- > board/xilinx/zynq/board.c | 16 +- > board/xilinx/zynqmp/cmds.c | 2 +- > board/xilinx/zynqmp/zynqmp.c | 24 +- > cmd/ab_select.c | 2 +- > cmd/avb.c | 2 +- > cmd/bdinfo.c | 6 +- > cmd/binop.c | 4 +- > cmd/bootefi.c | 8 +- > cmd/bootm.c | 4 +- > cmd/bootmenu.c | 6 +- > cmd/cbfs.c | 2 +- > cmd/cramfs.c | 8 +- > cmd/dtimg.c | 2 +- > cmd/elf.c | 29 +- > cmd/fdt.c | 22 +- > cmd/fpga.c | 4 +- > cmd/gpt.c | 8 +- > cmd/ini.c | 6 +- > cmd/itest.c | 2 +- > cmd/jffs2.c | 4 +- > cmd/load.c | 10 +- > cmd/lzmadec.c | 2 +- > cmd/md5sum.c | 4 +- > cmd/mtdparts.c | 41 +- > cmd/mvebu/bubt.c | 2 +- > cmd/nand.c | 12 +- > cmd/net.c | 46 +- > cmd/nvedit.c | 394 +++++++--- > cmd/part.c | 6 +- > cmd/pxe.c | 33 +- > cmd/qfw.c | 6 +- > cmd/reiser.c | 8 +- > cmd/setexpr.c | 8 +- > cmd/spl.c | 5 +- > cmd/ti/ddr3.c | 2 +- > cmd/tpm-common.c | 2 +- > cmd/tpm-v1.c | 2 +- > cmd/trace.c | 18 +- > cmd/ubi.c | 2 +- > cmd/unzip.c | 2 +- > cmd/ximg.c | 4 +- > cmd/zfs.c | 6 +- > cmd/zip.c | 2 +- > common/autoboot.c | 22 +- > common/board_f.c | 3 +- > common/board_r.c | 10 +- > common/bootm.c | 12 +- > common/bootm_os.c | 12 +- > common/bootretry.c | 2 +- > common/cli.c | 2 +- > common/cli_hush.c | 14 +- > common/cli_simple.c | 2 +- > common/command.c | 2 +- > common/console.c | 14 +- > common/fdt_support.c | 6 +- > common/hash.c | 4 +- > common/hwconfig.c | 5 +- > common/image-android.c | 4 +- > common/image-fdt.c | 4 +- > common/image.c | 15 +- > common/main.c | 5 +- > common/spl/spl_dfu.c | 6 +- > common/spl/spl_ext.c | 4 +- > common/spl/spl_fat.c | 4 +- > common/spl/spl_net.c | 4 +- > common/splash.c | 4 +- > common/splash_source.c | 8 +- > common/update.c | 10 +- > common/usb_hub.c | 2 +- > common/usb_kbd.c | 6 +- > disk/part.c | 2 +- > disk/part_amiga.c | 4 +- > drivers/bootcount/bootcount_env.c | 12 +- > drivers/ddr/fsl/fsl_ddr_gen4.c | 2 +- > drivers/ddr/fsl/interactive.c | 5 +- > drivers/ddr/fsl/options.c | 4 +- > drivers/dfu/dfu.c | 6 +- > drivers/fastboot/fb_command.c | 4 +- > drivers/fastboot/fb_common.c | 2 +- > drivers/fastboot/fb_getvar.c | 8 +- > drivers/fastboot/fb_mmc.c | 2 +- > drivers/input/i8042.c | 2 +- > drivers/input/input.c | 2 +- > drivers/misc/fs_loader.c | 8 +- > drivers/mtd/cfi_flash.c | 2 +- > drivers/mtd/mtd_uboot.c | 11 +- > drivers/net/fec_mxc.c | 2 +- > drivers/net/fm/b4860.c | 3 +- > drivers/net/fm/fdt.c | 2 +- > drivers/net/fm/fm.c | 4 +- > drivers/net/fsl-mc/mc.c | 7 +- > drivers/net/netconsole.c | 14 +- > drivers/net/phy/micrel_ksz90x1.c | 4 +- > drivers/net/sandbox-raw.c | 4 +- > drivers/pci/pci.c | 4 +- > drivers/pci/pci_common.c | 2 +- > drivers/reset/reset-socfpga.c | 2 +- > drivers/rtc/m41t60.c | 2 +- > drivers/scsi/scsi.c | 2 +- > drivers/serial/usbtty.c | 4 +- > drivers/usb/gadget/designware_udc.c | 2 +- > drivers/usb/gadget/ether.c | 13 +- > drivers/usb/gadget/f_dfu.c | 2 +- > drivers/usb/gadget/f_fastboot.c | 2 +- > drivers/usb/gadget/f_rockusb.c | 2 +- > drivers/usb/gadget/f_sdp.c | 2 +- > drivers/usb/host/ehci-fsl.c | 2 +- > drivers/video/ati_radeon_fb.c | 2 +- > drivers/video/cfb_console.c | 2 +- > drivers/video/mb862xx.c | 2 +- > drivers/video/mx3fb.c | 2 +- > drivers/video/mxsfb.c | 2 +- > drivers/video/videomodes.c | 4 +- > env/Kconfig | 683 +----------------- > env/Kconfig.efi | 152 ++++ > env/Kconfig.uboot | 671 +++++++++++++++++ > env/Makefile | 33 +- > env/callback.c | 40 +- > env/common.c | 255 ++++--- > env/env.c | 158 ++-- > env/env_ctx_efi.c | 131 ++++ > env/env_ctx_uboot.c | 292 ++++++++ > env/fat.c | 102 ++- > env/flags.c | 35 +- > env/flash.c | 397 ++++++---- > env/nowhere.c | 7 +- > fs/fs.c | 14 +- > fs/ubifs/ubifs.c | 2 +- > include/_exports.h | 6 +- > include/common.h | 6 +- > include/env.h | 114 ++- > include/env_internal.h | 89 ++- > include/exports.h | 5 +- > include/search.h | 6 +- > lib/efi_loader/efi_console.c | 2 +- > lib/efi_loader/efi_variable.c | 91 ++- > lib/fdtdec.c | 2 +- > lib/hashtable.c | 14 +- > lib/smbios.c | 2 +- > lib/uuid.c | 2 +- > net/bootp.c | 17 +- > net/dns.c | 2 +- > net/eth-uclass.c | 6 +- > net/eth_common.c | 18 +- > net/eth_legacy.c | 2 +- > net/link_local.c | 2 +- > net/net.c | 11 +- > net/tftp.c | 10 +- > net/wol.c | 2 +- > post/post.c | 2 +- > 371 files changed, 3690 insertions(+), 2337 deletions(-) > create mode 100644 env/Kconfig.efi > create mode 100644 env/Kconfig.uboot > create mode 100644 env/env_ctx_efi.c > create mode 100644 env/env_ctx_uboot.c > > -- > 2.21.0 >
Wolfgang, This is my second ping. Could you please take time to review this patch? -Takahiro Akashi On Tue, Oct 01, 2019 at 03:28:31PM +0900, AKASHI Takahiro wrote: > Wolfgang, > > I haven't seen any comments from you in the last one month. > Could you please take time to review this patch? I'd like to > confirm whether you can agree to my approach here or not > in order to take the next step. > > Thanks, > -Takahiro Akashi > > On Thu, Sep 05, 2019 at 05:21:14PM +0900, AKASHI Takahiro wrote: > > # In version 5 of this patch set, the implementation is changed again. > > # > > # I believe that this is NOT intrusive, and that my approach here is NOT > > # selfish at all. If Wolfgang doesn't accept this approach, however, > > # I would like to go for "Plan B" for UEFI variables implementation, in > > # which EFI will have its own drivers for storage instead of env/*. > > > > This patch set is an attempt to implement non-volatile attribute for > > UEFI variables. Under the current implementation, > > * SetVariable API doesn't recognize non-volatile attribute > > * While some variables are defined non-volatile in UEFI specification, > > they are NOT marked as non-volatile in the code. > > * env_save() (or "env save" command) allows us to save all the variables > > into persistent storage, but it may cause volatile UEFI variables, > > along with irrelevant U-Boot variables, to be saved unconditionally. > > > > Those observation rationalizes that the implementation of UEFI variables > > should be revamped utilizing dedicated storage for them. > > > > > > Basic ideas: > > * Sub-system users of U-Boot environment variables may have their own > > "env contexts". More than one contexts allowed. > > > > See Patch#2 and Patch#18. > > > > * Each context is isolated from other contexts with different name spaces. > > * Each context is associated with one backing storage driver and media > > location. > > * Different contexts may use different drivers and locations. > > > > * To access (get or set) a variable, associated context must be presented. > > So, almost of all existing env interfaces are changed to accept one > > extra argument, ctx. > > (I believe that this is Wolfgang's *requirement*.) > > > > From viewpoint of APIs, env context is a pointer to opaque structure. > > > > * Non-volatile feature is not implemented in a general form and must be > > implemented by users in their sub-systems. > > > > In version 4, U-Boot environment's attributes are extended to support > > non-volatile (or auto-save capability), but Wolfgang rejected > > my approach. > > As modifying attributes for this purpose would cause bunch of > > incompatibility issues (as far as I said in my cover letter and > > the discussions in ML), I would prefer a much simple approach. > > > > See patch#19 about how it is easily implemented for UEFI variables. > > > > * Each backing storage driver must be converted to be aligned with > > new env interfaces to handle multiple contexts in parallel and > > provide context-specific Kconfig configurations for driver parameters. > > > > In this version, only FAT file system and flash devices are supported, > > but it is quite straightforward to modify other drivers. > > > > See Patch#4 and Patch#5 about how drivers can shift to new interfaces. > > > > * Finally, all existing U-Boot variables hold the same semantics > > as before. > > > > > > Known issues/restriction/TODO: > > * The current form of patchset is not 'bisect'able. > > Not to break 'bisect,' all the patches in this patch set must be > > put into a single commit when merging. > > (This can be mitigated by modifying/splitting Patch#18/#19 though.) > > > > * Unfortunately, this code fails to read U-Boot environment from flash > > at boot time due to incomprehensible memory corruption. > > See murky workaround, which is marked as FIXME, in env/flash.c. > > > > Despite this bug, I was still be able to run/test my patch by hacking > > the code with gdb. > > (modifying data to correct value on the fly :) > > I hope that it won't affect code review in most places for now. > > > > * Some minor issues for better coding. > > They are also marked as FIXME in the source code. > > > > * Only FAT file system and flash are supported. > > > > * The whole area of storage will be saved at *every* update of > > one UEFI variable. It should be optimized if possible. > > > > * An error during "save" operation may cause inconsistency between > > cache (hash table) and the storage. > > -> This is not UEFI specific though. > > > > * Add tests if necessary. > > > > * I cannot test all the platforms affected by this patchset. > > > > > > Note: > > If we need "secure storage" for UEFI variables, efi_get_variable/ > > efi_get_next_variable_name/efi_set_variable() should be completely > > replaced with stub functions to communicate with secure world. > > This patchset has nothing to do with such an implementation. > > > > > > Usage: > > To enable this feature for example with FAT file system, the following > > configs must be enabled: > > CONFIG_ENV_IS_IN_FAT > > CONFIG_ENV_FAT_INTERFACE > > CONFIG_ENV_EFI_FAT_DEVICE_AND_PART > > CONFIG_ENV_EFI_FAT_FILE > > > > You may define a non-volatile variable from command interface: > > => setenv -e -nv FOO baa > > => printenv -e FOO > > FOO: NV|BS|RT, DataSize = 0x3 > > 00000000: 62 61 61 baa > > > > > > Patch#1 and #2 are to add multiples 'context' support to env interfaces > > and to provide new env interfaces. > > Patch#3 to #5 are to show how the existing drivers for U-Boot environment > > should be modified to be aligned with new env interfaces. > > (Only FAT file system and flash in this version of patch set.) > > Patch#6 to #17 are to shift all the existing users of old env interfaces > > to new ones. (But not tested for all the platforms.) > > Patch#18 and #19 are to modify UEFI variable implementation to utilize > > new env interfaces. > > > > Changes in v5 (September 4, 2019) > > * rebased to v2019.10-rc3 > > * revamp the implementation, removing changes on environment variable's > > attributes (See above) > > > > Changes in v4 (July 17, 2019) > > * remove already-merged patches > > * revamp after Wolfgang's suggestion > > > > Changes in v3 (June 4, 2019) > > * remove already-merged patches > > * revamp the code again > > * introduce CONFIG_EFI_VARIABLE_USE_ENV for this configuration. > > Once another backing storage, i.e. StMM services for secure boot, > > is supported, another option will be added. > > > > Changes in v2 (Apr 24, 2019) > > * rebased on efi-2019-07 > > * revamp the implementation > > > > v1 (Nov 28, 2018) > > * initial > > > > AKASHI Takahiro (19): > > env: extend interfaces allowing for env contexts > > env: define env context for U-Boot environment > > env: nowhere: rework with new env interfaces > > env: flash: support multiple env contexts > > env: fat: support multiple env contexts > > hashtable: support multiple env contexts > > api: converted with new env interfaces > > arch: converted with new env interfaces > > board: converted with new env interfaces > > cmd: converted with new env interfaces > > common: converted with new env interfaces > > disk: converted with new env interfaces > > drivers: converted with new env interfaces > > fs: converted with new env interfaces > > lib: converted with new env interfaces (except efi_loader) > > net: converted with new env interfaces > > post: converted with new env interfaces > > env,efi_loader: define env context for UEFI variables > > efi_loader: variable: rework with new env interfaces > > > > api/api.c | 8 +- > > arch/arc/lib/bootm.c | 2 +- > > arch/arm/cpu/arm926ejs/spear/spr_misc.c | 8 +- > > arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 5 +- > > arch/arm/cpu/armv8/fsl-layerscape/soc.c | 14 +- > > arch/arm/lib/bootm.c | 6 +- > > arch/arm/lib/semihosting.c | 2 +- > > arch/arm/mach-imx/mx6/opos6ul.c | 4 +- > > arch/arm/mach-imx/mx7/soc.c | 4 +- > > arch/arm/mach-imx/video.c | 2 +- > > arch/arm/mach-keystone/ddr3.c | 2 +- > > arch/arm/mach-keystone/keystone.c | 2 +- > > arch/arm/mach-kirkwood/cpu.c | 4 +- > > arch/arm/mach-meson/board-common.c | 2 +- > > arch/arm/mach-omap2/utils.c | 20 +- > > arch/arm/mach-rmobile/cpu_info.c | 2 +- > > arch/arm/mach-rockchip/boot_mode.c | 4 +- > > arch/arm/mach-rockchip/rk3288/rk3288.c | 2 +- > > arch/arm/mach-socfpga/misc_gen5.c | 5 +- > > arch/arm/mach-socfpga/misc_s10.c | 2 +- > > arch/arm/mach-stm32mp/cpu.c | 35 +- > > arch/arm/mach-tegra/board2.c | 4 +- > > arch/arm/mach-tegra/cboot.c | 18 +- > > arch/arm/mach-uniphier/board_late_init.c | 19 +- > > arch/arm/mach-uniphier/mmc-first-dev.c | 2 +- > > arch/m68k/lib/bootm.c | 2 +- > > arch/microblaze/lib/bootm.c | 2 +- > > arch/mips/lib/bootm.c | 6 +- > > arch/nds32/lib/bootm.c | 4 +- > > arch/powerpc/cpu/mpc85xx/cpu_init.c | 10 +- > > arch/powerpc/cpu/mpc85xx/fdt.c | 2 +- > > arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c | 2 +- > > arch/powerpc/lib/bootm.c | 2 +- > > arch/sh/lib/bootm.c | 2 +- > > arch/sh/lib/zimageboot.c | 2 +- > > arch/x86/lib/zimage.c | 11 +- > > arch/xtensa/lib/bootm.c | 2 +- > > board/Arcturus/ucp1020/cmd_arc.c | 40 +- > > board/Arcturus/ucp1020/ucp1020.c | 16 +- > > board/BuR/brppt1/board.c | 4 +- > > board/BuR/brxre1/board.c | 9 +- > > board/BuR/common/br_resetc.c | 2 +- > > board/BuR/common/common.c | 47 +- > > board/BuS/eb_cpu5282/eb_cpu5282.c | 8 +- > > board/CZ.NIC/turris_mox/turris_mox.c | 4 +- > > board/CZ.NIC/turris_omnia/turris_omnia.c | 6 +- > > board/CarMediaLab/flea3/flea3.c | 2 +- > > board/LaCie/net2big_v2/net2big_v2.c | 2 +- > > board/LaCie/netspace_v2/netspace_v2.c | 2 +- > > board/Synology/ds414/cmd_syno.c | 6 +- > > board/alliedtelesis/x530/x530.c | 2 +- > > board/amazon/kc1/kc1.c | 4 +- > > board/amlogic/p200/p200.c | 4 +- > > board/amlogic/p201/p201.c | 4 +- > > board/amlogic/p212/p212.c | 4 +- > > board/amlogic/q200/q200.c | 4 +- > > board/aristainetos/aristainetos-v2.c | 8 +- > > board/armltd/integrator/integrator.c | 2 +- > > board/atmel/common/board.c | 4 +- > > board/atmel/common/mac_eeprom.c | 2 +- > > board/atmel/sama5d3xek/sama5d3xek.c | 2 +- > > board/bachmann/ot1200/ot1200.c | 4 +- > > board/birdland/bav335x/board.c | 8 +- > > board/bluegiga/apx4devkit/apx4devkit.c | 5 +- > > board/bluewater/gurnard/gurnard.c | 6 +- > > board/bosch/shc/board.c | 2 +- > > board/boundary/nitrogen6x/nitrogen6x.c | 14 +- > > board/broadcom/bcm23550_w1d/bcm23550_w1d.c | 2 +- > > board/broadcom/bcm28155_ap/bcm28155_ap.c | 2 +- > > board/broadcom/bcmstb/bcmstb.c | 2 +- > > board/buffalo/lsxl/lsxl.c | 2 +- > > board/cadence/xtfpga/xtfpga.c | 4 +- > > board/ccv/xpress/xpress.c | 2 +- > > board/compulab/cl-som-imx7/cl-som-imx7.c | 2 +- > > board/compulab/cm_fx6/cm_fx6.c | 10 +- > > board/compulab/common/omap3_display.c | 4 +- > > board/congatec/cgtqmx6eval/cgtqmx6eval.c | 8 +- > > board/cssi/MCR3000/MCR3000.c | 2 +- > > board/davinci/da8xxevm/da850evm.c | 2 +- > > board/davinci/da8xxevm/omapl138_lcdk.c | 6 +- > > board/dhelectronics/dh_imx6/dh_imx6.c | 2 +- > > board/eets/pdu001/board.c | 8 +- > > board/el/el6x/el6x.c | 2 +- > > board/emulation/qemu-riscv/qemu-riscv.c | 2 +- > > board/engicam/common/board.c | 32 +- > > board/esd/meesc/meesc.c | 6 +- > > board/freescale/b4860qds/b4860qds.c | 5 +- > > board/freescale/common/cmd_esbc_validate.c | 2 +- > > board/freescale/common/fsl_chain_of_trust.c | 6 +- > > board/freescale/common/sys_eeprom.c | 4 +- > > board/freescale/common/vid.c | 4 +- > > board/freescale/imx8mq_evk/imx8mq_evk.c | 4 +- > > board/freescale/imx8qm_mek/imx8qm_mek.c | 4 +- > > board/freescale/imx8qxp_mek/imx8qxp_mek.c | 4 +- > > board/freescale/ls1088a/eth_ls1088aqds.c | 4 +- > > board/freescale/ls1088a/ls1088a.c | 2 +- > > board/freescale/ls2080aqds/eth.c | 6 +- > > board/freescale/ls2080aqds/ls2080aqds.c | 2 +- > > board/freescale/ls2080ardb/ls2080ardb.c | 6 +- > > board/freescale/lx2160a/eth_lx2160aqds.c | 2 +- > > board/freescale/mpc8323erdb/mpc8323erdb.c | 3 +- > > board/freescale/mpc837xemds/pci.c | 2 +- > > board/freescale/mpc837xerdb/mpc837xerdb.c | 2 +- > > board/freescale/mx51evk/mx51evk_video.c | 2 +- > > board/freescale/mx53loco/mx53loco.c | 4 +- > > board/freescale/mx53loco/mx53loco_video.c | 2 +- > > board/freescale/mx6sabreauto/mx6sabreauto.c | 8 +- > > board/freescale/mx6sabresd/mx6sabresd.c | 8 +- > > board/freescale/mx6sxsabresd/mx6sxsabresd.c | 2 +- > > .../mx6ul_14x14_evk/mx6ul_14x14_evk.c | 6 +- > > board/freescale/mx6ullevk/mx6ullevk.c | 4 +- > > board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c | 2 +- > > board/freescale/qemu-ppce500/qemu-ppce500.c | 4 +- > > board/freescale/t4qds/t4240qds.c | 2 +- > > board/gardena/smart-gateway-at91sam/board.c | 2 +- > > board/gardena/smart-gateway-mt7688/board.c | 16 +- > > board/gateworks/gw_ventana/common.c | 2 +- > > board/gateworks/gw_ventana/gw_ventana.c | 61 +- > > board/gateworks/gw_ventana/gw_ventana_spl.c | 4 +- > > board/gdsys/a38x/keyprogram.c | 4 +- > > board/gdsys/mpc8308/gazerbeam.c | 4 +- > > board/gdsys/mpc8308/hrcon.c | 2 +- > > board/gdsys/mpc8308/strider.c | 2 +- > > board/gdsys/p1022/controlcenterd-id.c | 10 +- > > board/gdsys/p1022/controlcenterd.c | 2 +- > > board/ge/bx50v3/bx50v3.c | 13 +- > > board/ge/common/ge_common.c | 4 +- > > board/ge/mx53ppd/mx53ppd.c | 2 +- > > board/grinn/chiliboard/board.c | 4 +- > > board/grinn/liteboard/board.c | 6 +- > > board/highbank/highbank.c | 9 +- > > board/hisilicon/poplar/poplar.c | 2 +- > > board/imgtec/ci20/ci20.c | 6 +- > > board/intel/edison/edison.c | 14 +- > > board/isee/igep003x/board.c | 6 +- > > board/isee/igep00x0/igep00x0.c | 4 +- > > board/k+p/kp_imx53/kp_id_rev.c | 20 +- > > board/k+p/kp_imx53/kp_imx53.c | 4 +- > > board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c | 4 +- > > board/keymile/common/common.c | 26 +- > > board/keymile/common/ivm.c | 8 +- > > board/keymile/km83xx/km83xx.c | 2 +- > > board/keymile/km_arm/km_arm.c | 6 +- > > board/keymile/kmp204x/kmp204x.c | 4 +- > > board/kosagi/novena/novena.c | 2 +- > > board/laird/wb50n/wb50n.c | 2 +- > > board/lg/sniper/sniper.c | 4 +- > > board/liebherr/display5/spl.c | 4 +- > > board/liebherr/mccmon6/mccmon6.c | 6 +- > > board/logicpd/imx6/imx6logic.c | 8 +- > > board/menlo/m53menlo/m53menlo.c | 2 +- > > board/micronas/vct/vct.c | 2 +- > > board/nokia/rx51/rx51.c | 10 +- > > board/overo/overo.c | 45 +- > > board/phytec/pcm052/pcm052.c | 4 +- > > board/phytec/pfla02/pfla02.c | 2 +- > > .../dragonboard410c/dragonboard410c.c | 6 +- > > .../dragonboard820c/dragonboard820c.c | 2 +- > > board/raspberrypi/rpi/rpi.c | 26 +- > > board/renesas/alt/alt.c | 3 +- > > board/renesas/gose/gose.c | 3 +- > > board/renesas/koelsch/koelsch.c | 3 +- > > board/renesas/lager/lager.c | 3 +- > > board/renesas/porter/porter.c | 3 +- > > board/renesas/sh7752evb/sh7752evb.c | 4 +- > > board/renesas/sh7753evb/sh7753evb.c | 4 +- > > board/renesas/sh7757lcr/sh7757lcr.c | 6 +- > > board/renesas/silk/silk.c | 3 +- > > board/renesas/stout/stout.c | 3 +- > > board/rockchip/kylin_rk3036/kylin_rk3036.c | 2 +- > > board/samsung/common/exynos5-dt.c | 2 +- > > board/samsung/common/misc.c | 14 +- > > board/samsung/odroid/odroid.c | 2 +- > > board/samsung/trats/trats.c | 2 +- > > board/samsung/universal_c210/universal.c | 2 +- > > board/samtec/vining_fpga/socfpga.c | 16 +- > > board/siemens/common/board.c | 4 +- > > board/siemens/draco/board.c | 6 +- > > board/siemens/pxm2/board.c | 4 +- > > board/siemens/rut/board.c | 2 +- > > board/siemens/taurus/taurus.c | 50 +- > > board/socrates/socrates.c | 4 +- > > board/softing/vining_2000/vining_2000.c | 8 +- > > board/solidrun/mx6cuboxi/mx6cuboxi.c | 16 +- > > .../stm32f429-discovery/stm32f429-discovery.c | 4 +- > > .../stm32f429-evaluation.c | 4 +- > > .../stm32f469-discovery/stm32f469-discovery.c | 4 +- > > board/st/stm32mp1/stm32mp1.c | 11 +- > > board/sunxi/board.c | 25 +- > > board/synopsys/hsdk/env-lib.c | 11 +- > > board/synopsys/hsdk/hsdk.c | 6 +- > > board/syteco/zmx25/zmx25.c | 8 +- > > board/tcl/sl50/board.c | 6 +- > > .../puma_rk3399/puma-rk3399.c | 8 +- > > board/ti/am335x/board.c | 18 +- > > board/ti/am43xx/board.c | 6 +- > > board/ti/am57xx/board.c | 14 +- > > board/ti/beagle/beagle.c | 43 +- > > board/ti/common/board_detect.c | 14 +- > > board/ti/dra7xx/evm.c | 12 +- > > board/ti/evm/evm.c | 2 +- > > board/ti/ks2_evm/board.c | 10 +- > > board/ti/ks2_evm/board_k2g.c | 8 +- > > board/ti/panda/panda.c | 4 +- > > board/toradex/apalis-imx8/apalis-imx8.c | 4 +- > > board/toradex/apalis_imx6/apalis_imx6.c | 12 +- > > .../toradex/colibri-imx6ull/colibri-imx6ull.c | 6 +- > > board/toradex/colibri-imx8x/colibri-imx8x.c | 4 +- > > board/toradex/colibri_imx6/colibri_imx6.c | 8 +- > > board/toradex/colibri_vf/colibri_vf.c | 2 +- > > board/toradex/common/tdx-cfg-block.c | 2 +- > > board/toradex/common/tdx-common.c | 2 +- > > board/tqc/tqma6/tqma6.c | 2 +- > > board/udoo/neo/neo.c | 2 +- > > board/udoo/udoo.c | 4 +- > > board/varisys/common/sys_eeprom.c | 6 +- > > board/vscom/baltos/board.c | 6 +- > > board/wandboard/wandboard.c | 12 +- > > board/warp7/warp7.c | 6 +- > > .../work_92105/work_92105_display.c | 2 +- > > board/xes/common/board.c | 6 +- > > board/xilinx/zynq/board.c | 16 +- > > board/xilinx/zynqmp/cmds.c | 2 +- > > board/xilinx/zynqmp/zynqmp.c | 24 +- > > cmd/ab_select.c | 2 +- > > cmd/avb.c | 2 +- > > cmd/bdinfo.c | 6 +- > > cmd/binop.c | 4 +- > > cmd/bootefi.c | 8 +- > > cmd/bootm.c | 4 +- > > cmd/bootmenu.c | 6 +- > > cmd/cbfs.c | 2 +- > > cmd/cramfs.c | 8 +- > > cmd/dtimg.c | 2 +- > > cmd/elf.c | 29 +- > > cmd/fdt.c | 22 +- > > cmd/fpga.c | 4 +- > > cmd/gpt.c | 8 +- > > cmd/ini.c | 6 +- > > cmd/itest.c | 2 +- > > cmd/jffs2.c | 4 +- > > cmd/load.c | 10 +- > > cmd/lzmadec.c | 2 +- > > cmd/md5sum.c | 4 +- > > cmd/mtdparts.c | 41 +- > > cmd/mvebu/bubt.c | 2 +- > > cmd/nand.c | 12 +- > > cmd/net.c | 46 +- > > cmd/nvedit.c | 394 +++++++--- > > cmd/part.c | 6 +- > > cmd/pxe.c | 33 +- > > cmd/qfw.c | 6 +- > > cmd/reiser.c | 8 +- > > cmd/setexpr.c | 8 +- > > cmd/spl.c | 5 +- > > cmd/ti/ddr3.c | 2 +- > > cmd/tpm-common.c | 2 +- > > cmd/tpm-v1.c | 2 +- > > cmd/trace.c | 18 +- > > cmd/ubi.c | 2 +- > > cmd/unzip.c | 2 +- > > cmd/ximg.c | 4 +- > > cmd/zfs.c | 6 +- > > cmd/zip.c | 2 +- > > common/autoboot.c | 22 +- > > common/board_f.c | 3 +- > > common/board_r.c | 10 +- > > common/bootm.c | 12 +- > > common/bootm_os.c | 12 +- > > common/bootretry.c | 2 +- > > common/cli.c | 2 +- > > common/cli_hush.c | 14 +- > > common/cli_simple.c | 2 +- > > common/command.c | 2 +- > > common/console.c | 14 +- > > common/fdt_support.c | 6 +- > > common/hash.c | 4 +- > > common/hwconfig.c | 5 +- > > common/image-android.c | 4 +- > > common/image-fdt.c | 4 +- > > common/image.c | 15 +- > > common/main.c | 5 +- > > common/spl/spl_dfu.c | 6 +- > > common/spl/spl_ext.c | 4 +- > > common/spl/spl_fat.c | 4 +- > > common/spl/spl_net.c | 4 +- > > common/splash.c | 4 +- > > common/splash_source.c | 8 +- > > common/update.c | 10 +- > > common/usb_hub.c | 2 +- > > common/usb_kbd.c | 6 +- > > disk/part.c | 2 +- > > disk/part_amiga.c | 4 +- > > drivers/bootcount/bootcount_env.c | 12 +- > > drivers/ddr/fsl/fsl_ddr_gen4.c | 2 +- > > drivers/ddr/fsl/interactive.c | 5 +- > > drivers/ddr/fsl/options.c | 4 +- > > drivers/dfu/dfu.c | 6 +- > > drivers/fastboot/fb_command.c | 4 +- > > drivers/fastboot/fb_common.c | 2 +- > > drivers/fastboot/fb_getvar.c | 8 +- > > drivers/fastboot/fb_mmc.c | 2 +- > > drivers/input/i8042.c | 2 +- > > drivers/input/input.c | 2 +- > > drivers/misc/fs_loader.c | 8 +- > > drivers/mtd/cfi_flash.c | 2 +- > > drivers/mtd/mtd_uboot.c | 11 +- > > drivers/net/fec_mxc.c | 2 +- > > drivers/net/fm/b4860.c | 3 +- > > drivers/net/fm/fdt.c | 2 +- > > drivers/net/fm/fm.c | 4 +- > > drivers/net/fsl-mc/mc.c | 7 +- > > drivers/net/netconsole.c | 14 +- > > drivers/net/phy/micrel_ksz90x1.c | 4 +- > > drivers/net/sandbox-raw.c | 4 +- > > drivers/pci/pci.c | 4 +- > > drivers/pci/pci_common.c | 2 +- > > drivers/reset/reset-socfpga.c | 2 +- > > drivers/rtc/m41t60.c | 2 +- > > drivers/scsi/scsi.c | 2 +- > > drivers/serial/usbtty.c | 4 +- > > drivers/usb/gadget/designware_udc.c | 2 +- > > drivers/usb/gadget/ether.c | 13 +- > > drivers/usb/gadget/f_dfu.c | 2 +- > > drivers/usb/gadget/f_fastboot.c | 2 +- > > drivers/usb/gadget/f_rockusb.c | 2 +- > > drivers/usb/gadget/f_sdp.c | 2 +- > > drivers/usb/host/ehci-fsl.c | 2 +- > > drivers/video/ati_radeon_fb.c | 2 +- > > drivers/video/cfb_console.c | 2 +- > > drivers/video/mb862xx.c | 2 +- > > drivers/video/mx3fb.c | 2 +- > > drivers/video/mxsfb.c | 2 +- > > drivers/video/videomodes.c | 4 +- > > env/Kconfig | 683 +----------------- > > env/Kconfig.efi | 152 ++++ > > env/Kconfig.uboot | 671 +++++++++++++++++ > > env/Makefile | 33 +- > > env/callback.c | 40 +- > > env/common.c | 255 ++++--- > > env/env.c | 158 ++-- > > env/env_ctx_efi.c | 131 ++++ > > env/env_ctx_uboot.c | 292 ++++++++ > > env/fat.c | 102 ++- > > env/flags.c | 35 +- > > env/flash.c | 397 ++++++---- > > env/nowhere.c | 7 +- > > fs/fs.c | 14 +- > > fs/ubifs/ubifs.c | 2 +- > > include/_exports.h | 6 +- > > include/common.h | 6 +- > > include/env.h | 114 ++- > > include/env_internal.h | 89 ++- > > include/exports.h | 5 +- > > include/search.h | 6 +- > > lib/efi_loader/efi_console.c | 2 +- > > lib/efi_loader/efi_variable.c | 91 ++- > > lib/fdtdec.c | 2 +- > > lib/hashtable.c | 14 +- > > lib/smbios.c | 2 +- > > lib/uuid.c | 2 +- > > net/bootp.c | 17 +- > > net/dns.c | 2 +- > > net/eth-uclass.c | 6 +- > > net/eth_common.c | 18 +- > > net/eth_legacy.c | 2 +- > > net/link_local.c | 2 +- > > net/net.c | 11 +- > > net/tftp.c | 10 +- > > net/wol.c | 2 +- > > post/post.c | 2 +- > > 371 files changed, 3690 insertions(+), 2337 deletions(-) > > create mode 100644 env/Kconfig.efi > > create mode 100644 env/Kconfig.uboot > > create mode 100644 env/env_ctx_efi.c > > create mode 100644 env/env_ctx_uboot.c > > > > -- > > 2.21.0 > >
Dear Takahiro, In message <20191023065332.GE10448@linaro.org> you wrote: > > This is my second ping. > Could you please take time to review this patch? Sorry, I'm afraid I will not find the time to review any such monster patch series any time soon. I hope Joe (added to Cc:) has more resources available. Only a few comments below... > > > # In version 5 of this patch set, the implementation is changed again. > > > # > > > # I believe that this is NOT intrusive, and that my approach here is NOT > > > # selfish at all. If Wolfgang doesn't accept this approach, however, 371 files changed, 3690 insertions(+), 2337 deletions(-) I don't know what your scales are, but for me such a patch is extremely invasive. It affects a zillion of files in common code plus a ton of board specific files. I did not find any information about the size impact or if the modified code continues to build for all boards - I remember we have a number of board with tight resources here and there. You should at least provide some information how much bigger the new code gets. From a quick glance I think the patches are not cleanly separated - you cannot change interfaces for the implementation in one step and for the callers in another, as this breaks bisectability. My biggest concern is that such a highly invasive change cannot be simply rubberstamped in a code review - I think this also needs runtime testing on at least a significant number of the affected boards. We should try to get help from at least some board maintainers - maybe you should ask for help for such testing n the board maintainers mailing list? Please do not misunderstand me - I am not trying to block any of this - I understand and appreciate the huge amount of efforts you have put into this. But I feel this needs not only careful review, but also actual testing on as many of the effected boards as possible, and I simply don't have time for that. > > > * To access (get or set) a variable, associated context must be presented. > > > So, almost of all existing env interfaces are changed to accept one > > > extra argument, ctx. > > > (I believe that this is Wolfgang's *requirement*.) I wonder if we really need to change all interfaces. I fear the size impact might bite us. I only had a glimpse at the actual code, but it seemed to me as if we were just pssing the same information around everywhere. Could we not use GD nstead, for example? > > > * Non-volatile feature is not implemented in a general form and must be > > > implemented by users in their sub-systems. I don't understand what this means, or why such a decision was made. Which sub-systems do you have in mind here? What prevented you from implementing a solution to works for all of us? > > > In version 4, U-Boot environment's attributes are extended to support > > > non-volatile (or auto-save capability), but Wolfgang rejected > > > my approach. > > > As modifying attributes for this purpose would cause bunch of > > > incompatibility issues (as far as I said in my cover letter and > > > the discussions in ML), I would prefer a much simple approach. I think we still have a different opinion here, but I'm lacking time for a thorough readding of the new code, so I hold back. I hope that Joe can have a closer look... > > > * Each backing storage driver must be converted to be aligned with > > > new env interfaces to handle multiple contexts in parallel and > > > provide context-specific Kconfig configurations for driver parameters. > > > > > > In this version, only FAT file system and flash devices are supported, > > > but it is quite straightforward to modify other drivers. If I see this correctly, there is a fundamental change in the implementation before: Up to now, the environment seize on external storage has been a compile time constant (CONFIG_ENV_SIZE). Now this value gets computed, and I'm not even sure if this is a contant at run time. This scares me. Does this not break compatibility? How do you upgrade a system from an older version of U-Boot to one with your patches? > > > Known issues/restriction/TODO: > > > * The current form of patchset is not 'bisect'able. > > > Not to break 'bisect,' all the patches in this patch set must be > > > put into a single commit when merging. > > > (This can be mitigated by modifying/splitting Patch#18/#19 though.) OK, so you are aware of this problem. I must admit that I really hate this. If you could avoid all the API changes, this would solve this problem, wouldn't it? > > > * Unfortunately, this code fails to read U-Boot environment from flash > > > at boot time due to incomprehensible memory corruption. > > > See murky workaround, which is marked as FIXME, in env/flash.c. Argh. This is a killing point, isn't it? You don't seriously expect to have patches which cause "incomprehensible memory corruption" to be included into mainline? > > > * The whole area of storage will be saved at *every* update of > > > one UEFI variable. It should be optimized if possible. This is only true for UEFI variables, right? > > > * An error during "save" operation may cause inconsistency between > > > cache (hash table) and the storage. > > > -> This is not UEFI specific though. Is this a new problem, or do you just mention this here for completeness? We always had this issue, didn't we? > > > * I cannot test all the platforms affected by this patchset. Sure, so please seek help from the board maintainers. And please provide size statistics. > > > To enable this feature for example with FAT file system, the following > > > configs must be enabled: > > > CONFIG_ENV_IS_IN_FAT > > > CONFIG_ENV_FAT_INTERFACE > > > CONFIG_ENV_EFI_FAT_DEVICE_AND_PART > > > CONFIG_ENV_EFI_FAT_FILE How much testing can be done on boards that don't use FAT to store the environment? Sorry that I can't be of any better help here... Best regards, Wolfgang Denk
Hi Wolfgang, On Fri, Oct 25, 2019 at 09:06:44AM +0200, Wolfgang Denk wrote: > Dear Takahiro, > > In message <20191023065332.GE10448@linaro.org> you wrote: > > > > This is my second ping. > > Could you please take time to review this patch? > > Sorry, I'm afraid I will not find the time to review any such > monster patch series any time soon. I hope Joe (added to Cc:) > has more resources available. > > Only a few comments below... I won't and cannot make replies on every comment that you gave me below because we are very different at some basic points and other comments are just details. > > > > # In version 5 of this patch set, the implementation is changed again. > > > > # > > > > # I believe that this is NOT intrusive, and that my approach here is NOT > > > > # selfish at all. If Wolfgang doesn't accept this approach, however, > > 371 files changed, 3690 insertions(+), 2337 deletions(-) > > I don't know what your scales are, but for me such a patch is > extremely invasive. It affects a zillion of files in common code > plus a ton of board specific files. > > I did not find any information about the size impact or if the > modified code continues to build for all boards - I remember we have > a number of board with tight resources here and there. > > You should at least provide some information how much bigger the new > code gets. > > From a quick glance I think the patches are not cleanly separated - > you cannot change interfaces for the implementation in one step and > for the callers in another, as this breaks bisectability. As you noticed below, I'm aware of that. So first I wanted to know if you agree to my *basic* approach or not, not details, in order to go further, but still don't see yes or no below. > > My biggest concern is that such a highly invasive change cannot be > simply rubberstamped in a code review - I think this also needs > runtime testing on at least a significant number of the affected > boards. We should try to get help from at least some board > maintainers - maybe you should ask for help for such testing n the > board maintainers mailing list? > > > Please do not misunderstand me - I am not trying to block any of > this - I understand and appreciate the huge amount of efforts you > have put into this. But I feel this needs not only careful review, > but also actual testing on as many of the effected boards as > possible, and I simply don't have time for that. Okay. > > > > > * To access (get or set) a variable, associated context must be presented. > > > > So, almost of all existing env interfaces are changed to accept one > > > > extra argument, ctx. > > > > (I believe that this is Wolfgang's *requirement*.) > > I wonder if we really need to change all interfaces. I fear the > size impact might bite us. I only had a glimpse at the actual code, > but it seemed to me as if we were just pssing the same information > around everywhere. Could we not use GD nstead, for example? > > > > > * Non-volatile feature is not implemented in a general form and must be > > > > implemented by users in their sub-systems. > > I don't understand what this means, or why such a decision was made. > Which sub-systems do you have in mind here? UEFI sub-system/library. > What prevented you from implementing a solution to works for all of > us? > > > > > In version 4, U-Boot environment's attributes are extended to support > > > > non-volatile (or auto-save capability), but Wolfgang rejected > > > > my approach. > > > > As modifying attributes for this purpose would cause bunch of > > > > incompatibility issues (as far as I said in my cover letter and > > > > the discussions in ML), I would prefer a much simple approach. > > I think we still have a different opinion here, but I'm lacking time > for a thorough readding of the new code, so I hold back. I hope > that Joe can have a closer look... You don't have to worry about my previous versions. Just focus on the current v5. > > > > > * Each backing storage driver must be converted to be aligned with > > > > new env interfaces to handle multiple contexts in parallel and > > > > provide context-specific Kconfig configurations for driver parameters. > > > > > > > > In this version, only FAT file system and flash devices are supported, > > > > but it is quite straightforward to modify other drivers. > > If I see this correctly, there is a fundamental change in the > implementation before: Up to now, the environment seize on external > storage has been a compile time constant (CONFIG_ENV_SIZE). > > Now this value gets computed, and I'm not even sure if this is a > contant at run time. Yes, it's constant for "U-Boot environment" (== U-Boot variables) context. But for other sus-systems, in my case UEFI, the total size of storage for (UEFI) variables may be different, but still constant. > This scares me. Does this not break compatibility? How do you > upgrade a system from an older version of U-Boot to one with your > patches? > > > > > Known issues/restriction/TODO: > > > > * The current form of patchset is not 'bisect'able. > > > > Not to break 'bisect,' all the patches in this patch set must be > > > > put into a single commit when merging. > > > > (This can be mitigated by modifying/splitting Patch#18/#19 though.) > > OK, so you are aware of this problem. > > I must admit that I really hate this. If you could avoid all the API > changes, this would solve this problem, wouldn't it? "Avoid all the API changes" is an approach that I took in all my previous versions, but you *denied* it. That is: I proposed an approach in which the existing interfaces, env_get/set(), were maintained for existing users/sub-systems. Only new users who want to enjoy merits from new "context" feature may use new *extended* interfaces, env_[get|set]_ext(), in my case UEFI. As you *denied* it, this version (v5) is an inevitable result. Don't take me wrong, but I think that you made inconsistent comments. > > > > * Unfortunately, this code fails to read U-Boot environment from flash > > > > at boot time due to incomprehensible memory corruption. > > > > See murky workaround, which is marked as FIXME, in env/flash.c. > > Argh. This is a killing point, isn't it? > > You don't seriously expect to have patches which cause > "incomprehensible memory corruption" to be included into mainline? It will be just a matter of time for debugging. > > > > * The whole area of storage will be saved at *every* update of > > > > one UEFI variable. It should be optimized if possible. > > This is only true for UEFI variables, right? Yes. > > > > * An error during "save" operation may cause inconsistency between > > > > cache (hash table) and the storage. > > > > -> This is not UEFI specific though. > > Is this a new problem, or do you just mention this here for > completeness? We always had this issue, didn't we? As I said, "this is not UEFI specific." > > > > * I cannot test all the platforms affected by this patchset. > > Sure, so please seek help from the board maintainers. > > And please provide size statistics. > > > > > To enable this feature for example with FAT file system, the following > > > > configs must be enabled: > > > > CONFIG_ENV_IS_IN_FAT > > > > CONFIG_ENV_FAT_INTERFACE > > > > CONFIG_ENV_EFI_FAT_DEVICE_AND_PART > > > > CONFIG_ENV_EFI_FAT_FILE > > How much testing can be done on boards that don't use FAT to store > the environment? As I said, > > > > In this version, only FAT file system and flash devices are supported, > > > > but it is quite straightforward to modify other drivers. If you don't think my patch is not qualified for a "PATCH" for some reason, I will sent one as "RFC" from the next version. I don't care. Thanks, -Takahiro Akashi > > Sorry that I can't be of any better help here... > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > 'What shall we do?' said Twoflower. 'Panic?' said Rincewind hope- > fully. He always held that panic was the best means of survival; back > in the olden days, his theory went, people faced with hungry sabre- > toothed tigers could be divided very simply in those who panicked and > those who stood there saying 'What a magnificent brute!' or 'Here, > pussy.' - Terry Pratchett, _The Light Fantastic_
Dear Takahiro, In message <20191025075645.GJ10448@linaro.org> you wrote: > > I won't and cannot make replies on every comment that you gave me > below because we are very different at some basic points and > other comments are just details. Can you please at least comment on the size impact? How much does the code size grow on everage, especially for SPL? > So first I wanted to know if you agree to my *basic* approach or not, > not details, in order to go further, but still don't see > yes or no below. To be honest: when I saw your monster patch series which basically touches every piece of code in U-Boot, I felt a strong temptation to just send a NAK and be done with it. But I know this would not be fair. But to be able to say yes I would need days to review the code and probably run some tests myself, and I don;t have that time. So I can neither say yes or no, sorry. > > My biggest concern is that such a highly invasive change cannot be > > simply rubberstamped in a code review - I think this also needs > > runtime testing on at least a significant number of the affected > > boards. We should try to get help from at least some board > > maintainers - maybe you should ask for help for such testing n the > > board maintainers mailing list? This is a point which is important to me. We need at least a few "Tested-by" credits... > > > > > * Non-volatile feature is not implemented in a general form and must be > > > > > implemented by users in their sub-systems. > > > > I don't understand what this means, or why such a decision was made. > > Which sub-systems do you have in mind here? > > UEFI sub-system/library. What needs to be done to have this - say - for U-Boot context? > > What prevented you from implementing a solution to works for all of > > us? ? > > > > > Known issues/restriction/TODO: > > > > > * The current form of patchset is not 'bisect'able. > > > > > Not to break 'bisect,' all the patches in this patch set must be > > > > > put into a single commit when merging. > > > > > (This can be mitigated by modifying/splitting Patch#18/#19 though.) > > > > OK, so you are aware of this problem. > > > > I must admit that I really hate this. If you could avoid all the API > > changes, this would solve this problem, wouldn't it? > > "Avoid all the API changes" is an approach that I took in all my > previous versions, but you *denied* it. > > That is: I proposed an approach in which the existing interfaces, > env_get/set(), were maintained for existing users/sub-systems. > Only new users who want to enjoy merits from new "context" feature may > use new *extended* interfaces, env_[get|set]_ext(), in my case UEFI. > As you *denied* it, this version (v5) is an inevitable result. > > Don't take me wrong, but I think that you made inconsistent comments. I think you misunderstand. If we just need the same pointer in all functions dealing with the environment, there are at least two ways to implement this: we can add it as an argument to each and every function call; this will blow up code size and also impact execution speed. Or we can add it to some (private or public) data structure that is visible everywhere. In the simplest case we could add such a pointer to the global data (GD) structure as I suggested in my previous mail. this would allow you to use basically the same code as now, but without needing to change all the argument lists. In the result, you could drop your modifications of all common and board specififc files. The code changes would be concentrated on the environment code, and it should be anle to submit bisectable patches again. Yes, global variables have disadvantages, too, but does it not make sense here? I think we will have only one active environment context at any time in U-Boot, so this seems to be at least a lesser evil than the zillion of changes of all call arguments. > > > > > * Unfortunately, this code fails to read U-Boot environment from flash > > > > > at boot time due to incomprehensible memory corruption. > > > > > See murky workaround, which is marked as FIXME, in env/flash.c. > > > > Argh. This is a killing point, isn't it? > > > > You don't seriously expect to have patches which cause > > "incomprehensible memory corruption" to be included into mainline? > > It will be just a matter of time for debugging. It might be difficult to find willing testers under such circumstances. I would not want to run code with serious known bugs. > > > > > * An error during "save" operation may cause inconsistency between > > > > > cache (hash table) and the storage. > > > > > -> This is not UEFI specific though. > > > > Is this a new problem, or do you just mention this here for > > completeness? We always had this issue, didn't we? > > As I said, "this is not UEFI specific." This does not answer my question. Are you just refering to the general problem that a write to the persistent storage area might fail, which has ever been present and which is inherently unfixxable, or does your new code add any additional problems of this kind? > > How much testing can be done on boards that don't use FAT to store > > the environment? > > As I said, > > > > > In this version, only FAT file system and flash devices are supported, > > > > > but it is quite straightforward to modify other drivers. > > If you don't think my patch is not qualified for a "PATCH" for some reason, > I will sent one as "RFC" from the next version. I don't care. I think this is a new feature that needs to be tested. You focus on UEFI + FAT and I don't know if UEFI + non-FAT makes sense, but at least (1) non-UEFI + FAT and (2) non-UEFI + non-FAT are configurations which must be tested - absolute minimum is at least one example implementation. My suggestion would be to use something that is not file system based, but using plain storage, say a board which has the environment in SPI NOR flash. Best regards, Wolfgang Denk
Wolfgang, On Fri, Oct 25, 2019 at 03:25:23PM +0200, Wolfgang Denk wrote: > Dear Takahiro, > > In message <20191025075645.GJ10448@linaro.org> you wrote: > > > > I won't and cannot make replies on every comment that you gave me > > below because we are very different at some basic points and > > other comments are just details. > > Can you please at least comment on the size impact? How much does > the code size grow on everage, especially for SPL? Well, from the next version as I will make a drastic change as far as I follow your suggestion below. > > > So first I wanted to know if you agree to my *basic* approach or not, > > not details, in order to go further, but still don't see > > yes or no below. > > To be honest: when I saw your monster patch series which basically > touches every piece of code in U-Boot, I felt a strong temptation to > just send a NAK and be done with it. But I know this would not be > fair. But to be able to say yes I would need days to review the > code and probably run some tests myself, and I don;t have that time. Okay, I see but I hope that you should have said so much earlier. My patch (v5) has been stranded almost two months without any comments. > So I can neither say yes or no, sorry. > > > > My biggest concern is that such a highly invasive change cannot be > > > simply rubberstamped in a code review - I think this also needs > > > runtime testing on at least a significant number of the affected > > > boards. We should try to get help from at least some board > > > maintainers - maybe you should ask for help for such testing n the > > > board maintainers mailing list? > > This is a point which is important to me. We need at least a few > "Tested-by" credits... > > > > > > > * Non-volatile feature is not implemented in a general form and must be > > > > > > implemented by users in their sub-systems. > > > > > > I don't understand what this means, or why such a decision was made. > > > Which sub-systems do you have in mind here? > > > > UEFI sub-system/library. > > What needs to be done to have this - say - for U-Boot context? > > > > What prevented you from implementing a solution to works for all of > > > us? > > ? At least for me or UEFI, nothing more, other than a *context*, is needed to meet the requirement. If some one really want to have such(?) a feature, he or she can on top of my patch. I believe that my attitude here is fair enough. > > > > > > > Known issues/restriction/TODO: > > > > > > * The current form of patchset is not 'bisect'able. > > > > > > Not to break 'bisect,' all the patches in this patch set must be > > > > > > put into a single commit when merging. > > > > > > (This can be mitigated by modifying/splitting Patch#18/#19 though.) > > > > > > OK, so you are aware of this problem. > > > > > > I must admit that I really hate this. If you could avoid all the API > > > changes, this would solve this problem, wouldn't it? > > > > "Avoid all the API changes" is an approach that I took in all my > > previous versions, but you *denied* it. > > > > That is: I proposed an approach in which the existing interfaces, > > env_get/set(), were maintained for existing users/sub-systems. > > Only new users who want to enjoy merits from new "context" feature may > > use new *extended* interfaces, env_[get|set]_ext(), in my case UEFI. > > As you *denied* it, this version (v5) is an inevitable result. > > > > Don't take me wrong, but I think that you made inconsistent comments. > > I think you misunderstand. If we just need the same pointer in all > functions dealing with the environment, there are at least two ways > to implement this: we can add it as an argument to each and every > function call; this will blow up code size and also impact execution > speed. Or we can add it to some (private or public) data structure > that is visible everywhere. In the simplest case we could add such > a pointer to the global data (GD) structure as I suggested in my > previous mail. this would allow you to use basically the same code > as now, but without needing to change all the argument lists. In > the result, you could drop your modifications of all common and > board specififc files. The code changes would be concentrated on > the environment code, and it should be anle to submit bisectable > patches again. > > Yes, global variables have disadvantages, too, but does it not make > sense here? I think we will have only one active environment > context at any time in U-Boot, so this seems to be at least a lesser > evil than the zillion of changes of all call arguments. I have thought of an idea as you mentioned above before but immediately gave it up as it seems to be an ugly implementation and I simply dislike it. But if you and Tom (or whoever can say ACK to my patch) think that it is the *only* solution that you can accept, I will have to change my mind. So do you always admit the following coding? ===8<=== (in some UEFI function) ... old_ctx = env_set_context(ctx_uefi); env_set(var, value); env_set_context(old_ctx); ... ===>8=== > > > > > > > * Unfortunately, this code fails to read U-Boot environment from flash > > > > > > at boot time due to incomprehensible memory corruption. > > > > > > See murky workaround, which is marked as FIXME, in env/flash.c. > > > > > > Argh. This is a killing point, isn't it? > > > > > > You don't seriously expect to have patches which cause > > > "incomprehensible memory corruption" to be included into mainline? > > > > It will be just a matter of time for debugging. > > It might be difficult to find willing testers under such > circumstances. I would not want to run code with serious known > bugs. I believe that the issue is independent from the basic approach that we are now discussing. > > > > > > * An error during "save" operation may cause inconsistency between > > > > > > cache (hash table) and the storage. > > > > > > -> This is not UEFI specific though. > > > > > > Is this a new problem, or do you just mention this here for > > > completeness? We always had this issue, didn't we? > > > > As I said, "this is not UEFI specific." > > This does not answer my question. Are you just refering to the > general problem that a write to the persistent storage area might > fail, which has ever been present and which is inherently > unfixxable, or does your new code add any additional problems of > this kind? A general problem in your term. > > > How much testing can be done on boards that don't use FAT to store > > > the environment? > > > > As I said, > > > > > > In this version, only FAT file system and flash devices are supported, > > > > > > but it is quite straightforward to modify other drivers. > > > > If you don't think my patch is not qualified for a "PATCH" for some reason, > > I will sent one as "RFC" from the next version. I don't care. > > I think this is a new feature that needs to be tested. You focus on > UEFI + FAT and I don't know if UEFI + non-FAT makes sense, but at > least (1) non-UEFI + FAT and (2) non-UEFI + non-FAT are > configurations which must be tested - absolute minimum is at least > one example implementation. My suggestion would be to use something > that is not file system based, but using plain storage, say a board > which has the environment in SPI NOR flash. What do you mean by "non-UEFI"? Disabling UEFI, or testing U-Boot environment variables on no-FAT device? If the latter, I have tested it with flash (on qemu). Thanks, -Takahiro Akashi > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > Our OS who art in CPU, UNIX be thy name. > Thy programs run, thy syscalls done, > In kernel as it is in user!
Dear Takahiro, In message <20191028011435.GP10448@linaro.org> you wrote: > > > Can you please at least comment on the size impact? How much does > > the code size grow on everage, especially for SPL? > > Well, from the next version as I will make a drastic change as far as > I follow your suggestion below. OK. > Okay, I see but I hope that you should have said so much earlier. > My patch (v5) has been stranded almost two months without any comments. I have said so uin lcear and unmistakable words when discussion was to add me as maintainer for environment code. I cannot commit to such a position as I don't have enough time available. > > > > > > > * Non-volatile feature is not implemented in a general form and must be > > > > > > > implemented by users in their sub-systems. > > > > > > > > I don't understand what this means, or why such a decision was made. > > > > Which sub-systems do you have in mind here? > > > > > > UEFI sub-system/library. > > > > What needs to be done to have this - say - for U-Boot context? > > > > > > What prevented you from implementing a solution to works for all of > > > > us? > > > > ? > > At least for me or UEFI, nothing more, other than a *context*, is needed > to meet the requirement. > If some one really want to have such(?) a feature, he or she can on top > of my patch. > I believe that my attitude here is fair enough. I would appreciate if you could actually answer my questions. there are two of them: You write: "Non-volatile feature is not implemented in a general form and must be implemented by users in their sub-systems." Q1: Why did you not implement this feature in a generic way, so it is automatically available to all sub-systems that support contexts? Q2: What exactly needs to be done, which code needs to be writtenm, to implement this feature - say - for U-Boot context? > So do you always admit the following coding? > ===8<=== > (in some UEFI function) > ... > old_ctx = env_set_context(ctx_uefi); > env_set(var, value); > env_set_context(old_ctx); > ... > ===>8=== This is even worse, as instead of adding a single argument to a function call you are adding two full fucntion calls. But why would that be needed? U-Boot is strictly single tasking. You always know what the current context is, and nobody will change it behind your back. And UEFI functions are not being called in random places, they are limited to a small set of UEFI commands, right? So why do you not just enter UEFI context when you start executing UEFI code, and restore the rpevious state when returning to non-UEFI U-Boot? Or am I missing something? > > > > You don't seriously expect to have patches which cause > > > > "incomprehensible memory corruption" to be included into mainline? > > > > > > It will be just a matter of time for debugging. > > > > It might be difficult to find willing testers under such > > circumstances. I would not want to run code with serious known > > bugs. > > I believe that the issue is independent from the basic approach > that we are now discussing. The reason of this problem must be understood and the bug fixed before it can go into mainline. > > I think this is a new feature that needs to be tested. You focus on > > UEFI + FAT and I don't know if UEFI + non-FAT makes sense, but at > > least (1) non-UEFI + FAT and (2) non-UEFI + non-FAT are > > configurations which must be tested - absolute minimum is at least > > one example implementation. My suggestion would be to use something > > that is not file system based, but using plain storage, say a board > > which has the environment in SPI NOR flash. > > What do you mean by "non-UEFI"? Disabling UEFI, or testing U-Boot environment > variables on no-FAT device? > If the latter, I have tested it with flash (on qemu). We need to test if the new context code works (and does not cause problems) on systems where UEFI is not enabled. Best regards, Wolfgang Denk
Wolfgang, I would like to focus on the critical and most arguable point in this email. On Tue, Oct 29, 2019 at 02:28:19PM +0100, Wolfgang Denk wrote: > Dear Takahiro, > > In message <20191028011435.GP10448@linaro.org> you wrote: > > > > > Can you please at least comment on the size impact? How much does > > > the code size grow on everage, especially for SPL? > > > > Well, from the next version as I will make a drastic change as far as > > I follow your suggestion below. > > OK. > > > Okay, I see but I hope that you should have said so much earlier. > > My patch (v5) has been stranded almost two months without any comments. > > I have said so uin lcear and unmistakable words when discussion was > to add me as maintainer for environment code. I cannot commit to > such a position as I don't have enough time available. > > > > > > > > > * Non-volatile feature is not implemented in a general form and must be > > > > > > > > implemented by users in their sub-systems. > > > > > > > > > > I don't understand what this means, or why such a decision was made. > > > > > Which sub-systems do you have in mind here? > > > > > > > > UEFI sub-system/library. > > > > > > What needs to be done to have this - say - for U-Boot context? > > > > > > > > What prevented you from implementing a solution to works for all of > > > > > us? > > > > > > ? > > > > At least for me or UEFI, nothing more, other than a *context*, is needed > > to meet the requirement. > > If some one really want to have such(?) a feature, he or she can on top > > of my patch. > > I believe that my attitude here is fair enough. > > I would appreciate if you could actually answer my questions. there > are two of them: > > You write: "Non-volatile feature is not implemented in a general > form and must be implemented by users in their sub-systems." > > Q1: Why did you not implement this feature in a generic way, so it > is automatically available to all sub-systems that support > contexts? > > Q2: What exactly needs to be done, which code needs to be writtenm, > to implement this feature - say - for U-Boot context? > > > > So do you always admit the following coding? > > ===8<=== > > (in some UEFI function) > > ... > > old_ctx = env_set_context(ctx_uefi); > > env_set(var, value); > > env_set_context(old_ctx); > > ... > > ===>8=== > > This is even worse, as instead of adding a single argument to a > function call you are adding two full fucntion calls. > > But why would that be needed? > > U-Boot is strictly single tasking. You always know what the current > context is, and nobody will change it behind your back. > > And UEFI functions are not being called in random places, they are > limited to a small set of UEFI commands, right? So why do you not > just enter UEFI context when you start executing UEFI code, and > restore the rpevious state when returning to non-UEFI U-Boot? Can you elaborate What you mean by "entering UEFI context"? What I'm thinking of here is that we would add one more member field, which is a pointer to my "env_context," in GD and change it in env_set_context(). This can be defined even as an inline function. (My other patches, at least patch#1 to #6, can be used almost as they are.) I don't see why you deny such a simple solution with lots of flexibility. -Takahiro Akashi > Or am I missing something? > > > > > > You don't seriously expect to have patches which cause > > > > > "incomprehensible memory corruption" to be included into mainline? > > > > > > > > It will be just a matter of time for debugging. > > > > > > It might be difficult to find willing testers under such > > > circumstances. I would not want to run code with serious known > > > bugs. > > > > I believe that the issue is independent from the basic approach > > that we are now discussing. > > The reason of this problem must be understood and the bug fixed > before it can go into mainline. > > > > I think this is a new feature that needs to be tested. You focus on > > > UEFI + FAT and I don't know if UEFI + non-FAT makes sense, but at > > > least (1) non-UEFI + FAT and (2) non-UEFI + non-FAT are > > > configurations which must be tested - absolute minimum is at least > > > one example implementation. My suggestion would be to use something > > > that is not file system based, but using plain storage, say a board > > > which has the environment in SPI NOR flash. > > > > What do you mean by "non-UEFI"? Disabling UEFI, or testing U-Boot environment > > variables on no-FAT device? > > If the latter, I have tested it with flash (on qemu). > > We need to test if the new context code works (and does not cause > problems) on systems where UEFI is not enabled. > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > Don't you know anything? I should have thought anyone knows that who > knows anything about anything... - Terry Pratchett, _Soul Music_
Dear Takahiro, In message <20191101060434.GV10448@linaro.org> you wrote: > > > This is even worse, as instead of adding a single argument to a > > function call you are adding two full fucntion calls. > > > > But why would that be needed? > > > > U-Boot is strictly single tasking. You always know what the current > > context is, and nobody will change it behind your back. > > > > And UEFI functions are not being called in random places, they are > > limited to a small set of UEFI commands, right? So why do you not > > just enter UEFI context when you start executing UEFI code, and > > restore the rpevious state when returning to non-UEFI U-Boot? > > Can you elaborate What you mean by "entering UEFI context"? You wrote: | So do you always admit the following coding? | ===8<=== | (in some UEFI function) | ... | old_ctx = env_set_context(ctx_uefi); | env_set(var, value); | env_set_context(old_ctx); | ... | ===>8=== In this code, the function call ``env_set_context(ctx_uefi)'' would enter the UEFI context, right? This is what I mean. > What I'm thinking of here is that we would add one more member field, which > is a pointer to my "env_context," in GD and change it in env_set_context(). > This can be defined even as an inline function. Yes, this is OK - I think I understood this already. What I mean is that such a call of env_set_context() is not needed for each and every call of other env_*() functions. If I understand correctly, only the UEFI specific commands will actually care about UEFI contexts - most likely no code outside cmd/nvedit_efi.c ? So you have pretty clear entry and exit points into and from the UEFI world, and it should be sufficient to set and restore this context only then. > I don't see why you deny such a simple solution with lots of flexibility. I am concerned about code size and execution speed. So far, you did not provide any such numbers, even though I repeatedly asked for the size impact, especially for non-UEFI systems. Please keep in mind that there are many cases where we need access to the environment already in SPL, and on all systems where security plays any role we must have the same set of features enabled for the environment code in SPL and TPL (if these need access to the envronment) as in U-Boot proper, and memory is a precious resource there. Best regards, Wolfgang Denk
On Mon, Nov 04, 2019 at 05:00:03PM +0100, Wolfgang Denk wrote: > Dear Takahiro, > > In message <20191101060434.GV10448@linaro.org> you wrote: > > > > > This is even worse, as instead of adding a single argument to a > > > function call you are adding two full fucntion calls. > > > > > > But why would that be needed? > > > > > > U-Boot is strictly single tasking. You always know what the current > > > context is, and nobody will change it behind your back. > > > > > > And UEFI functions are not being called in random places, they are > > > limited to a small set of UEFI commands, right? So why do you not > > > just enter UEFI context when you start executing UEFI code, and > > > restore the rpevious state when returning to non-UEFI U-Boot? > > > > Can you elaborate What you mean by "entering UEFI context"? > > You wrote: > > | So do you always admit the following coding? > | ===8<=== > | (in some UEFI function) > | ... > | old_ctx = env_set_context(ctx_uefi); > | env_set(var, value); > | env_set_context(old_ctx); > | ... > | ===>8=== > > In this code, the function call ``env_set_context(ctx_uefi)'' would > enter the UEFI context, right? This is what I mean. > > > What I'm thinking of here is that we would add one more member field, which > > is a pointer to my "env_context," in GD and change it in env_set_context(). > > This can be defined even as an inline function. > > Yes, this is OK - I think I understood this already. > > What I mean is that such a call of env_set_context() is not needed > for each and every call of other env_*() functions. > > If I understand correctly, only the UEFI specific commands will > actually care about UEFI contexts - most likely no code outside > cmd/nvedit_efi.c ? > > So you have pretty clear entry and exit points into and from the > UEFI world, and it should be sufficient to set and restore this > context only then. > > > I don't see why you deny such a simple solution with lots of flexibility. > > I am concerned about code size and execution speed. So far, you did > not provide any such numbers, even though I repeatedly asked for the > size impact, especially for non-UEFI systems. > > Please keep in mind that there are many cases where we need access > to the environment already in SPL, and on all systems where security > plays any role we must have the same set of features enabled for the > environment code in SPL and TPL (if these need access to the > envronment) as in U-Boot proper, and memory is a precious resource > there. I too am very concerned that whatever we come up with here needs to have between zero and near-zero impact on the rest of the U-Boot world. We've once again hit the case where some boards don't build due to bugfixes increasing their binary size. And they aren't "no one uses these, we can drop them" boards either.
Wolfgang, On Mon, Nov 04, 2019 at 05:00:03PM +0100, Wolfgang Denk wrote: > Dear Takahiro, > > In message <20191101060434.GV10448@linaro.org> you wrote: > > > > > This is even worse, as instead of adding a single argument to a > > > function call you are adding two full fucntion calls. > > > > > > But why would that be needed? > > > > > > U-Boot is strictly single tasking. You always know what the current > > > context is, and nobody will change it behind your back. > > > > > > And UEFI functions are not being called in random places, they are > > > limited to a small set of UEFI commands, right? So why do you not > > > just enter UEFI context when you start executing UEFI code, and > > > restore the rpevious state when returning to non-UEFI U-Boot? > > > > Can you elaborate What you mean by "entering UEFI context"? > > You wrote: > > | So do you always admit the following coding? > | ===8<=== > | (in some UEFI function) > | ... > | old_ctx = env_set_context(ctx_uefi); > | env_set(var, value); > | env_set_context(old_ctx); > | ... > | ===>8=== > > In this code, the function call ``env_set_context(ctx_uefi)'' would > enter the UEFI context, right? This is what I mean. My point is not there. See below. > > What I'm thinking of here is that we would add one more member field, which > > is a pointer to my "env_context," in GD and change it in env_set_context(). > > This can be defined even as an inline function. > > Yes, this is OK - I think I understood this already. So we get one step closer. > What I mean is that such a call of env_set_context() is not needed > for each and every call of other env_*() functions. > > If I understand correctly, only the UEFI specific commands will > actually care about UEFI contexts - most likely no code outside > cmd/nvedit_efi.c ? No. There are a couple of reasons: 1) Other commands include cmd/bootefi.c (and cmd/efidebug.c). 2) Any EFI application, once kicked off, can directly call EFI APIs. 3) Bunch of EFI features (either services or protocols) fundamentally reply on U-Boot native functionality, including devices (disk, network, console, ...), file systems and so on. So calling such UEFI interfaces may eventually end up with invoking env_*() *indirectly*. I don't come up with good examples (probably, network is a candidate) now but such a situation will be quite likely and we should not exclude such a possibility in the future. > So you have pretty clear entry and exit points into and from the > UEFI world, and it should be sufficient to set and restore this > context only then. My point is what entry or exit points are. They cannot be at each command, but *each* API. So, let me repeat this example: ===8<=== (in some UEFI function) ... old_ctx = env_set_context(ctx_uefi); env_set(var, value); env_set_context(old_ctx); ... ===>8=== This can be at most relaxed to: ===8<=== some_efi_api(...) { old_ctx = env_set_context(ctx_uefi); ... (env_get(var1);) ... env_set(var2, value); ... env_set_context(old_ctx); return; } ===>8=== It is merely a matter of optimization. This is the reason that I want to introduce env_set_context(). Thanks, -Takahiro Akashi > > I don't see why you deny such a simple solution with lots of flexibility. > > I am concerned about code size and execution speed. So far, you did > not provide any such numbers, even though I repeatedly asked for the > size impact, especially for non-UEFI systems. > > Please keep in mind that there are many cases where we need access > to the environment already in SPL, and on all systems where security > plays any role we must have the same set of features enabled for the > environment code in SPL and TPL (if these need access to the > envronment) as in U-Boot proper, and memory is a precious resource > there. > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > You may call me by my name, Wirth, or by my value, Worth. > - Nicklaus Wirth
# In version 5 of this patch set, the implementation is changed again. # # I believe that this is NOT intrusive, and that my approach here is NOT # selfish at all. If Wolfgang doesn't accept this approach, however, # I would like to go for "Plan B" for UEFI variables implementation, in # which EFI will have its own drivers for storage instead of env/*. This patch set is an attempt to implement non-volatile attribute for UEFI variables. Under the current implementation, * SetVariable API doesn't recognize non-volatile attribute * While some variables are defined non-volatile in UEFI specification, they are NOT marked as non-volatile in the code. * env_save() (or "env save" command) allows us to save all the variables into persistent storage, but it may cause volatile UEFI variables, along with irrelevant U-Boot variables, to be saved unconditionally. Those observation rationalizes that the implementation of UEFI variables should be revamped utilizing dedicated storage for them. Basic ideas: * Sub-system users of U-Boot environment variables may have their own "env contexts". More than one contexts allowed. See Patch#2 and Patch#18. * Each context is isolated from other contexts with different name spaces. * Each context is associated with one backing storage driver and media location. * Different contexts may use different drivers and locations. * To access (get or set) a variable, associated context must be presented. So, almost of all existing env interfaces are changed to accept one extra argument, ctx. (I believe that this is Wolfgang's *requirement*.) From viewpoint of APIs, env context is a pointer to opaque structure. * Non-volatile feature is not implemented in a general form and must be implemented by users in their sub-systems. In version 4, U-Boot environment's attributes are extended to support non-volatile (or auto-save capability), but Wolfgang rejected my approach. As modifying attributes for this purpose would cause bunch of incompatibility issues (as far as I said in my cover letter and the discussions in ML), I would prefer a much simple approach. See patch#19 about how it is easily implemented for UEFI variables. * Each backing storage driver must be converted to be aligned with new env interfaces to handle multiple contexts in parallel and provide context-specific Kconfig configurations for driver parameters. In this version, only FAT file system and flash devices are supported, but it is quite straightforward to modify other drivers. See Patch#4 and Patch#5 about how drivers can shift to new interfaces. * Finally, all existing U-Boot variables hold the same semantics as before. Known issues/restriction/TODO: * The current form of patchset is not 'bisect'able. Not to break 'bisect,' all the patches in this patch set must be put into a single commit when merging. (This can be mitigated by modifying/splitting Patch#18/#19 though.) * Unfortunately, this code fails to read U-Boot environment from flash at boot time due to incomprehensible memory corruption. See murky workaround, which is marked as FIXME, in env/flash.c. Despite this bug, I was still be able to run/test my patch by hacking the code with gdb. (modifying data to correct value on the fly :) I hope that it won't affect code review in most places for now. * Some minor issues for better coding. They are also marked as FIXME in the source code. * Only FAT file system and flash are supported. * The whole area of storage will be saved at *every* update of one UEFI variable. It should be optimized if possible. * An error during "save" operation may cause inconsistency between cache (hash table) and the storage. -> This is not UEFI specific though. * Add tests if necessary. * I cannot test all the platforms affected by this patchset. Note: If we need "secure storage" for UEFI variables, efi_get_variable/ efi_get_next_variable_name/efi_set_variable() should be completely replaced with stub functions to communicate with secure world. This patchset has nothing to do with such an implementation. Usage: To enable this feature for example with FAT file system, the following configs must be enabled: CONFIG_ENV_IS_IN_FAT CONFIG_ENV_FAT_INTERFACE CONFIG_ENV_EFI_FAT_DEVICE_AND_PART CONFIG_ENV_EFI_FAT_FILE You may define a non-volatile variable from command interface: => setenv -e -nv FOO baa => printenv -e FOO FOO: NV|BS|RT, DataSize = 0x3 00000000: 62 61 61 baa Patch#1 and #2 are to add multiples 'context' support to env interfaces and to provide new env interfaces. Patch#3 to #5 are to show how the existing drivers for U-Boot environment should be modified to be aligned with new env interfaces. (Only FAT file system and flash in this version of patch set.) Patch#6 to #17 are to shift all the existing users of old env interfaces to new ones. (But not tested for all the platforms.) Patch#18 and #19 are to modify UEFI variable implementation to utilize new env interfaces. Changes in v5 (September 4, 2019) * rebased to v2019.10-rc3 * revamp the implementation, removing changes on environment variable's attributes (See above) Changes in v4 (July 17, 2019) * remove already-merged patches * revamp after Wolfgang's suggestion Changes in v3 (June 4, 2019) * remove already-merged patches * revamp the code again * introduce CONFIG_EFI_VARIABLE_USE_ENV for this configuration. Once another backing storage, i.e. StMM services for secure boot, is supported, another option will be added. Changes in v2 (Apr 24, 2019) * rebased on efi-2019-07 * revamp the implementation v1 (Nov 28, 2018) * initial AKASHI Takahiro (19): env: extend interfaces allowing for env contexts env: define env context for U-Boot environment env: nowhere: rework with new env interfaces env: flash: support multiple env contexts env: fat: support multiple env contexts hashtable: support multiple env contexts api: converted with new env interfaces arch: converted with new env interfaces board: converted with new env interfaces cmd: converted with new env interfaces common: converted with new env interfaces disk: converted with new env interfaces drivers: converted with new env interfaces fs: converted with new env interfaces lib: converted with new env interfaces (except efi_loader) net: converted with new env interfaces post: converted with new env interfaces env,efi_loader: define env context for UEFI variables efi_loader: variable: rework with new env interfaces api/api.c | 8 +- arch/arc/lib/bootm.c | 2 +- arch/arm/cpu/arm926ejs/spear/spr_misc.c | 8 +- arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 5 +- arch/arm/cpu/armv8/fsl-layerscape/soc.c | 14 +- arch/arm/lib/bootm.c | 6 +- arch/arm/lib/semihosting.c | 2 +- arch/arm/mach-imx/mx6/opos6ul.c | 4 +- arch/arm/mach-imx/mx7/soc.c | 4 +- arch/arm/mach-imx/video.c | 2 +- arch/arm/mach-keystone/ddr3.c | 2 +- arch/arm/mach-keystone/keystone.c | 2 +- arch/arm/mach-kirkwood/cpu.c | 4 +- arch/arm/mach-meson/board-common.c | 2 +- arch/arm/mach-omap2/utils.c | 20 +- arch/arm/mach-rmobile/cpu_info.c | 2 +- arch/arm/mach-rockchip/boot_mode.c | 4 +- arch/arm/mach-rockchip/rk3288/rk3288.c | 2 +- arch/arm/mach-socfpga/misc_gen5.c | 5 +- arch/arm/mach-socfpga/misc_s10.c | 2 +- arch/arm/mach-stm32mp/cpu.c | 35 +- arch/arm/mach-tegra/board2.c | 4 +- arch/arm/mach-tegra/cboot.c | 18 +- arch/arm/mach-uniphier/board_late_init.c | 19 +- arch/arm/mach-uniphier/mmc-first-dev.c | 2 +- arch/m68k/lib/bootm.c | 2 +- arch/microblaze/lib/bootm.c | 2 +- arch/mips/lib/bootm.c | 6 +- arch/nds32/lib/bootm.c | 4 +- arch/powerpc/cpu/mpc85xx/cpu_init.c | 10 +- arch/powerpc/cpu/mpc85xx/fdt.c | 2 +- arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c | 2 +- arch/powerpc/lib/bootm.c | 2 +- arch/sh/lib/bootm.c | 2 +- arch/sh/lib/zimageboot.c | 2 +- arch/x86/lib/zimage.c | 11 +- arch/xtensa/lib/bootm.c | 2 +- board/Arcturus/ucp1020/cmd_arc.c | 40 +- board/Arcturus/ucp1020/ucp1020.c | 16 +- board/BuR/brppt1/board.c | 4 +- board/BuR/brxre1/board.c | 9 +- board/BuR/common/br_resetc.c | 2 +- board/BuR/common/common.c | 47 +- board/BuS/eb_cpu5282/eb_cpu5282.c | 8 +- board/CZ.NIC/turris_mox/turris_mox.c | 4 +- board/CZ.NIC/turris_omnia/turris_omnia.c | 6 +- board/CarMediaLab/flea3/flea3.c | 2 +- board/LaCie/net2big_v2/net2big_v2.c | 2 +- board/LaCie/netspace_v2/netspace_v2.c | 2 +- board/Synology/ds414/cmd_syno.c | 6 +- board/alliedtelesis/x530/x530.c | 2 +- board/amazon/kc1/kc1.c | 4 +- board/amlogic/p200/p200.c | 4 +- board/amlogic/p201/p201.c | 4 +- board/amlogic/p212/p212.c | 4 +- board/amlogic/q200/q200.c | 4 +- board/aristainetos/aristainetos-v2.c | 8 +- board/armltd/integrator/integrator.c | 2 +- board/atmel/common/board.c | 4 +- board/atmel/common/mac_eeprom.c | 2 +- board/atmel/sama5d3xek/sama5d3xek.c | 2 +- board/bachmann/ot1200/ot1200.c | 4 +- board/birdland/bav335x/board.c | 8 +- board/bluegiga/apx4devkit/apx4devkit.c | 5 +- board/bluewater/gurnard/gurnard.c | 6 +- board/bosch/shc/board.c | 2 +- board/boundary/nitrogen6x/nitrogen6x.c | 14 +- board/broadcom/bcm23550_w1d/bcm23550_w1d.c | 2 +- board/broadcom/bcm28155_ap/bcm28155_ap.c | 2 +- board/broadcom/bcmstb/bcmstb.c | 2 +- board/buffalo/lsxl/lsxl.c | 2 +- board/cadence/xtfpga/xtfpga.c | 4 +- board/ccv/xpress/xpress.c | 2 +- board/compulab/cl-som-imx7/cl-som-imx7.c | 2 +- board/compulab/cm_fx6/cm_fx6.c | 10 +- board/compulab/common/omap3_display.c | 4 +- board/congatec/cgtqmx6eval/cgtqmx6eval.c | 8 +- board/cssi/MCR3000/MCR3000.c | 2 +- board/davinci/da8xxevm/da850evm.c | 2 +- board/davinci/da8xxevm/omapl138_lcdk.c | 6 +- board/dhelectronics/dh_imx6/dh_imx6.c | 2 +- board/eets/pdu001/board.c | 8 +- board/el/el6x/el6x.c | 2 +- board/emulation/qemu-riscv/qemu-riscv.c | 2 +- board/engicam/common/board.c | 32 +- board/esd/meesc/meesc.c | 6 +- board/freescale/b4860qds/b4860qds.c | 5 +- board/freescale/common/cmd_esbc_validate.c | 2 +- board/freescale/common/fsl_chain_of_trust.c | 6 +- board/freescale/common/sys_eeprom.c | 4 +- board/freescale/common/vid.c | 4 +- board/freescale/imx8mq_evk/imx8mq_evk.c | 4 +- board/freescale/imx8qm_mek/imx8qm_mek.c | 4 +- board/freescale/imx8qxp_mek/imx8qxp_mek.c | 4 +- board/freescale/ls1088a/eth_ls1088aqds.c | 4 +- board/freescale/ls1088a/ls1088a.c | 2 +- board/freescale/ls2080aqds/eth.c | 6 +- board/freescale/ls2080aqds/ls2080aqds.c | 2 +- board/freescale/ls2080ardb/ls2080ardb.c | 6 +- board/freescale/lx2160a/eth_lx2160aqds.c | 2 +- board/freescale/mpc8323erdb/mpc8323erdb.c | 3 +- board/freescale/mpc837xemds/pci.c | 2 +- board/freescale/mpc837xerdb/mpc837xerdb.c | 2 +- board/freescale/mx51evk/mx51evk_video.c | 2 +- board/freescale/mx53loco/mx53loco.c | 4 +- board/freescale/mx53loco/mx53loco_video.c | 2 +- board/freescale/mx6sabreauto/mx6sabreauto.c | 8 +- board/freescale/mx6sabresd/mx6sabresd.c | 8 +- board/freescale/mx6sxsabresd/mx6sxsabresd.c | 2 +- .../mx6ul_14x14_evk/mx6ul_14x14_evk.c | 6 +- board/freescale/mx6ullevk/mx6ullevk.c | 4 +- board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c | 2 +- board/freescale/qemu-ppce500/qemu-ppce500.c | 4 +- board/freescale/t4qds/t4240qds.c | 2 +- board/gardena/smart-gateway-at91sam/board.c | 2 +- board/gardena/smart-gateway-mt7688/board.c | 16 +- board/gateworks/gw_ventana/common.c | 2 +- board/gateworks/gw_ventana/gw_ventana.c | 61 +- board/gateworks/gw_ventana/gw_ventana_spl.c | 4 +- board/gdsys/a38x/keyprogram.c | 4 +- board/gdsys/mpc8308/gazerbeam.c | 4 +- board/gdsys/mpc8308/hrcon.c | 2 +- board/gdsys/mpc8308/strider.c | 2 +- board/gdsys/p1022/controlcenterd-id.c | 10 +- board/gdsys/p1022/controlcenterd.c | 2 +- board/ge/bx50v3/bx50v3.c | 13 +- board/ge/common/ge_common.c | 4 +- board/ge/mx53ppd/mx53ppd.c | 2 +- board/grinn/chiliboard/board.c | 4 +- board/grinn/liteboard/board.c | 6 +- board/highbank/highbank.c | 9 +- board/hisilicon/poplar/poplar.c | 2 +- board/imgtec/ci20/ci20.c | 6 +- board/intel/edison/edison.c | 14 +- board/isee/igep003x/board.c | 6 +- board/isee/igep00x0/igep00x0.c | 4 +- board/k+p/kp_imx53/kp_id_rev.c | 20 +- board/k+p/kp_imx53/kp_imx53.c | 4 +- board/k+p/kp_imx6q_tpc/kp_imx6q_tpc.c | 4 +- board/keymile/common/common.c | 26 +- board/keymile/common/ivm.c | 8 +- board/keymile/km83xx/km83xx.c | 2 +- board/keymile/km_arm/km_arm.c | 6 +- board/keymile/kmp204x/kmp204x.c | 4 +- board/kosagi/novena/novena.c | 2 +- board/laird/wb50n/wb50n.c | 2 +- board/lg/sniper/sniper.c | 4 +- board/liebherr/display5/spl.c | 4 +- board/liebherr/mccmon6/mccmon6.c | 6 +- board/logicpd/imx6/imx6logic.c | 8 +- board/menlo/m53menlo/m53menlo.c | 2 +- board/micronas/vct/vct.c | 2 +- board/nokia/rx51/rx51.c | 10 +- board/overo/overo.c | 45 +- board/phytec/pcm052/pcm052.c | 4 +- board/phytec/pfla02/pfla02.c | 2 +- .../dragonboard410c/dragonboard410c.c | 6 +- .../dragonboard820c/dragonboard820c.c | 2 +- board/raspberrypi/rpi/rpi.c | 26 +- board/renesas/alt/alt.c | 3 +- board/renesas/gose/gose.c | 3 +- board/renesas/koelsch/koelsch.c | 3 +- board/renesas/lager/lager.c | 3 +- board/renesas/porter/porter.c | 3 +- board/renesas/sh7752evb/sh7752evb.c | 4 +- board/renesas/sh7753evb/sh7753evb.c | 4 +- board/renesas/sh7757lcr/sh7757lcr.c | 6 +- board/renesas/silk/silk.c | 3 +- board/renesas/stout/stout.c | 3 +- board/rockchip/kylin_rk3036/kylin_rk3036.c | 2 +- board/samsung/common/exynos5-dt.c | 2 +- board/samsung/common/misc.c | 14 +- board/samsung/odroid/odroid.c | 2 +- board/samsung/trats/trats.c | 2 +- board/samsung/universal_c210/universal.c | 2 +- board/samtec/vining_fpga/socfpga.c | 16 +- board/siemens/common/board.c | 4 +- board/siemens/draco/board.c | 6 +- board/siemens/pxm2/board.c | 4 +- board/siemens/rut/board.c | 2 +- board/siemens/taurus/taurus.c | 50 +- board/socrates/socrates.c | 4 +- board/softing/vining_2000/vining_2000.c | 8 +- board/solidrun/mx6cuboxi/mx6cuboxi.c | 16 +- .../stm32f429-discovery/stm32f429-discovery.c | 4 +- .../stm32f429-evaluation.c | 4 +- .../stm32f469-discovery/stm32f469-discovery.c | 4 +- board/st/stm32mp1/stm32mp1.c | 11 +- board/sunxi/board.c | 25 +- board/synopsys/hsdk/env-lib.c | 11 +- board/synopsys/hsdk/hsdk.c | 6 +- board/syteco/zmx25/zmx25.c | 8 +- board/tcl/sl50/board.c | 6 +- .../puma_rk3399/puma-rk3399.c | 8 +- board/ti/am335x/board.c | 18 +- board/ti/am43xx/board.c | 6 +- board/ti/am57xx/board.c | 14 +- board/ti/beagle/beagle.c | 43 +- board/ti/common/board_detect.c | 14 +- board/ti/dra7xx/evm.c | 12 +- board/ti/evm/evm.c | 2 +- board/ti/ks2_evm/board.c | 10 +- board/ti/ks2_evm/board_k2g.c | 8 +- board/ti/panda/panda.c | 4 +- board/toradex/apalis-imx8/apalis-imx8.c | 4 +- board/toradex/apalis_imx6/apalis_imx6.c | 12 +- .../toradex/colibri-imx6ull/colibri-imx6ull.c | 6 +- board/toradex/colibri-imx8x/colibri-imx8x.c | 4 +- board/toradex/colibri_imx6/colibri_imx6.c | 8 +- board/toradex/colibri_vf/colibri_vf.c | 2 +- board/toradex/common/tdx-cfg-block.c | 2 +- board/toradex/common/tdx-common.c | 2 +- board/tqc/tqma6/tqma6.c | 2 +- board/udoo/neo/neo.c | 2 +- board/udoo/udoo.c | 4 +- board/varisys/common/sys_eeprom.c | 6 +- board/vscom/baltos/board.c | 6 +- board/wandboard/wandboard.c | 12 +- board/warp7/warp7.c | 6 +- .../work_92105/work_92105_display.c | 2 +- board/xes/common/board.c | 6 +- board/xilinx/zynq/board.c | 16 +- board/xilinx/zynqmp/cmds.c | 2 +- board/xilinx/zynqmp/zynqmp.c | 24 +- cmd/ab_select.c | 2 +- cmd/avb.c | 2 +- cmd/bdinfo.c | 6 +- cmd/binop.c | 4 +- cmd/bootefi.c | 8 +- cmd/bootm.c | 4 +- cmd/bootmenu.c | 6 +- cmd/cbfs.c | 2 +- cmd/cramfs.c | 8 +- cmd/dtimg.c | 2 +- cmd/elf.c | 29 +- cmd/fdt.c | 22 +- cmd/fpga.c | 4 +- cmd/gpt.c | 8 +- cmd/ini.c | 6 +- cmd/itest.c | 2 +- cmd/jffs2.c | 4 +- cmd/load.c | 10 +- cmd/lzmadec.c | 2 +- cmd/md5sum.c | 4 +- cmd/mtdparts.c | 41 +- cmd/mvebu/bubt.c | 2 +- cmd/nand.c | 12 +- cmd/net.c | 46 +- cmd/nvedit.c | 394 +++++++--- cmd/part.c | 6 +- cmd/pxe.c | 33 +- cmd/qfw.c | 6 +- cmd/reiser.c | 8 +- cmd/setexpr.c | 8 +- cmd/spl.c | 5 +- cmd/ti/ddr3.c | 2 +- cmd/tpm-common.c | 2 +- cmd/tpm-v1.c | 2 +- cmd/trace.c | 18 +- cmd/ubi.c | 2 +- cmd/unzip.c | 2 +- cmd/ximg.c | 4 +- cmd/zfs.c | 6 +- cmd/zip.c | 2 +- common/autoboot.c | 22 +- common/board_f.c | 3 +- common/board_r.c | 10 +- common/bootm.c | 12 +- common/bootm_os.c | 12 +- common/bootretry.c | 2 +- common/cli.c | 2 +- common/cli_hush.c | 14 +- common/cli_simple.c | 2 +- common/command.c | 2 +- common/console.c | 14 +- common/fdt_support.c | 6 +- common/hash.c | 4 +- common/hwconfig.c | 5 +- common/image-android.c | 4 +- common/image-fdt.c | 4 +- common/image.c | 15 +- common/main.c | 5 +- common/spl/spl_dfu.c | 6 +- common/spl/spl_ext.c | 4 +- common/spl/spl_fat.c | 4 +- common/spl/spl_net.c | 4 +- common/splash.c | 4 +- common/splash_source.c | 8 +- common/update.c | 10 +- common/usb_hub.c | 2 +- common/usb_kbd.c | 6 +- disk/part.c | 2 +- disk/part_amiga.c | 4 +- drivers/bootcount/bootcount_env.c | 12 +- drivers/ddr/fsl/fsl_ddr_gen4.c | 2 +- drivers/ddr/fsl/interactive.c | 5 +- drivers/ddr/fsl/options.c | 4 +- drivers/dfu/dfu.c | 6 +- drivers/fastboot/fb_command.c | 4 +- drivers/fastboot/fb_common.c | 2 +- drivers/fastboot/fb_getvar.c | 8 +- drivers/fastboot/fb_mmc.c | 2 +- drivers/input/i8042.c | 2 +- drivers/input/input.c | 2 +- drivers/misc/fs_loader.c | 8 +- drivers/mtd/cfi_flash.c | 2 +- drivers/mtd/mtd_uboot.c | 11 +- drivers/net/fec_mxc.c | 2 +- drivers/net/fm/b4860.c | 3 +- drivers/net/fm/fdt.c | 2 +- drivers/net/fm/fm.c | 4 +- drivers/net/fsl-mc/mc.c | 7 +- drivers/net/netconsole.c | 14 +- drivers/net/phy/micrel_ksz90x1.c | 4 +- drivers/net/sandbox-raw.c | 4 +- drivers/pci/pci.c | 4 +- drivers/pci/pci_common.c | 2 +- drivers/reset/reset-socfpga.c | 2 +- drivers/rtc/m41t60.c | 2 +- drivers/scsi/scsi.c | 2 +- drivers/serial/usbtty.c | 4 +- drivers/usb/gadget/designware_udc.c | 2 +- drivers/usb/gadget/ether.c | 13 +- drivers/usb/gadget/f_dfu.c | 2 +- drivers/usb/gadget/f_fastboot.c | 2 +- drivers/usb/gadget/f_rockusb.c | 2 +- drivers/usb/gadget/f_sdp.c | 2 +- drivers/usb/host/ehci-fsl.c | 2 +- drivers/video/ati_radeon_fb.c | 2 +- drivers/video/cfb_console.c | 2 +- drivers/video/mb862xx.c | 2 +- drivers/video/mx3fb.c | 2 +- drivers/video/mxsfb.c | 2 +- drivers/video/videomodes.c | 4 +- env/Kconfig | 683 +----------------- env/Kconfig.efi | 152 ++++ env/Kconfig.uboot | 671 +++++++++++++++++ env/Makefile | 33 +- env/callback.c | 40 +- env/common.c | 255 ++++--- env/env.c | 158 ++-- env/env_ctx_efi.c | 131 ++++ env/env_ctx_uboot.c | 292 ++++++++ env/fat.c | 102 ++- env/flags.c | 35 +- env/flash.c | 397 ++++++---- env/nowhere.c | 7 +- fs/fs.c | 14 +- fs/ubifs/ubifs.c | 2 +- include/_exports.h | 6 +- include/common.h | 6 +- include/env.h | 114 ++- include/env_internal.h | 89 ++- include/exports.h | 5 +- include/search.h | 6 +- lib/efi_loader/efi_console.c | 2 +- lib/efi_loader/efi_variable.c | 91 ++- lib/fdtdec.c | 2 +- lib/hashtable.c | 14 +- lib/smbios.c | 2 +- lib/uuid.c | 2 +- net/bootp.c | 17 +- net/dns.c | 2 +- net/eth-uclass.c | 6 +- net/eth_common.c | 18 +- net/eth_legacy.c | 2 +- net/link_local.c | 2 +- net/net.c | 11 +- net/tftp.c | 10 +- net/wol.c | 2 +- post/post.c | 2 +- 371 files changed, 3690 insertions(+), 2337 deletions(-) create mode 100644 env/Kconfig.efi create mode 100644 env/Kconfig.uboot create mode 100644 env/env_ctx_efi.c create mode 100644 env/env_ctx_uboot.c