[U-Boot] [PATCH] qemu-arm: Add persistent environment support

[Sumit Garg <sumit.garg@linaro.org>]

On Tue, 27 Nov 2018 at 13:44, Takahiro Akashi
<takahiro.akashi@linaro.org> wrote:
>
> On Tue, Nov 27, 2018 at 01:11:44PM +0530, Sumit Garg wrote:
> > Hi Akashi,
> >
> > On Tue, 27 Nov 2018 at 12:14, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > Sumit,
> > >
> > > # I have a similar patch in my local branch :)
> > >
> >
> > IIRC, we did sync on this work.
> >
> > > On Mon, Nov 26, 2018 at 04:50:17PM +0530, Sumit Garg wrote:
> > > > Currently on qemu-arm platforms environment is kept in RAM. Instead
> > > > use pflash device 1 to provide persistent environment support across
> > > > device reset.
> > > >
> > > > Also (optionally) provide support for persistent environment across
> > > > qemu machine OFF/ON using following instructions:
> > > >
> > > > - Create envstore.img using qemu-img:
> > > >     qemu-img create -f raw envstore.img 64M
> > > > - Add a pflash drive parameter to the command line:
> > > >     -drive if=pflash,format=raw,index=1,file=envstore.img
> > > >
> > > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > > > ---
> > > >  configs/qemu_arm64_defconfig | 7 +++++++
> > > >  configs/qemu_arm_defconfig   | 7 +++++++
> > > >  doc/README.qemu-arm          | 6 ++++++
> > > >  include/configs/qemu-arm.h   | 8 +++++++-
> > > >  4 files changed, 27 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
> > > > index f4502c9..0f13716 100644
> > > > --- a/configs/qemu_arm64_defconfig
> > > > +++ b/configs/qemu_arm64_defconfig
> > > > @@ -29,3 +29,10 @@ CONFIG_USB=y
> > > >  CONFIG_DM_USB=y
> > > >  CONFIG_USB_EHCI_HCD=y
> > > >  CONFIG_USB_EHCI_PCI=y
> > > > +CONFIG_ENV_IS_IN_FLASH=y
> > > > +CONFIG_MTD=y
> > > > +CONFIG_MTD_NOR_FLASH=y
> > > > +CONFIG_FLASH_CFI_DRIVER=y
> > >
> > > This one and
> > >
> > > > +CONFIG_CFI_FLASH=y
> > >
> > > > +CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> > > > +CONFIG_SYS_FLASH_CFI=y
> > >
> > > those two are not currently Kconfig parameters.
> > > So they should not be put in *_defconfig, but in include/configs/*.
> > > Or it would be better to define them as new Kconfigs?
> > >
> >
> > I do see these Kconfigs already defined in drivers/mtd/Kconfig.
>
> OK. I didn't notice that those were added in the last couple of weeks.
>
> > > > diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
> > > > index acebdc5..b75363e 100644
> > > > --- a/configs/qemu_arm_defconfig
> > > > +++ b/configs/qemu_arm_defconfig
> > > > @@ -29,3 +29,10 @@ CONFIG_USB=y
> > > >  CONFIG_DM_USB=y
> > > >  CONFIG_USB_EHCI_HCD=y
> > > >  CONFIG_USB_EHCI_PCI=y
> > > > +CONFIG_ENV_IS_IN_FLASH=y
> > > > +CONFIG_MTD=y
> > > > +CONFIG_MTD_NOR_FLASH=y
> > > > +CONFIG_FLASH_CFI_DRIVER=y
> > > > +CONFIG_CFI_FLASH=y
> > > > +CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> > > > +CONFIG_SYS_FLASH_CFI=y
> > > > diff --git a/doc/README.qemu-arm b/doc/README.qemu-arm
> > > > index 2601656..e67bc13 100644
> > > > --- a/doc/README.qemu-arm
> > > > +++ b/doc/README.qemu-arm
> > > > @@ -47,6 +47,12 @@ The minimal QEMU command line to get U-Boot up and running is:
> > > >  Note that for some odd reason qemu-system-aarch64 needs to be explicitly
> > > >  told to use a 64-bit CPU or it will boot in 32-bit mode.
> > > >
> > > > +Additional persistent U-boot environment support can be added as follows:
> > > > +- Create envstore.img using qemu-img:
> > > > +    qemu-img create -f raw envstore.img 64M
> > > > +- Add a pflash drive parameter to the command line:
> > > > +    -drive if=pflash,format=raw,index=1,file=envstore.img
> > > > +
> > > >  Additional peripherals that have been tested to work in both U-Boot and Linux
> > > >  can be enabled with the following command line parameters:
> > > >
> > > > diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> > > > index fedc466..83a930b 100644
> > > > --- a/include/configs/qemu-arm.h
> > > > +++ b/include/configs/qemu-arm.h
> > > > @@ -21,7 +21,8 @@
> > > >  #define CONFIG_SYS_HZ                       1000
> > > >
> > > >  /* Environment options */
> > > > -#define CONFIG_ENV_SIZE                              SZ_64K
> > > > +#define CONFIG_ENV_ADDR                      0x4000000
> > > > +#define CONFIG_ENV_SIZE                      SZ_256K
> > >
> > > # I'm not sure whether those definitions be enforced for all qemu-arm users.
>
> This is one of reasons.
>
> > > Do you have any reason to put env region in the second bank?
> >
> > Yes I thought it would be better to load "envstore.img" on bank #1
> > separately from u-boot.bin (loaded on bank #0) like we do for
> > "varstore" while booting UEFI/edk2. This also doesn't change existing
> > cmdline to use "-bios" option for u-boot.bin and rather provide an
> > optional cmdline argument to load environment.
>
> # I always re-create a rom image from u-boot and variable region
> # with a few lines of shell script.

This does create dependency to retain variable region while updating
u-boot image.

>
> > Also I have updated ENV_SIZE to be SZ_256K due to minimum erase size
> > for flash being 256K (sector size).
> >
> > > If not, I suggest that it be left free for other use.
> > >
> >
> > AFAIK, I haven't seen any other usage for flash. If you are aware of
> > any, please do let me know.
>
> Well, I'm thinking of using the second bank for "capsule on disk"
> to implement runtime efi variable accesses which I'm now working on
> locally.
> While it is not limited to a separate bank (nor even a flash), but
> we need a file system (as a boot device) somewhere.
>

Wouldn't EFI system partition be a good place for this? IIRC, in case
UEFI/edk2 its used for firmware capsule updates.

Regards,
Sumit

> Thanks,
> -Takahiro Akashi
>
> > Regards,
> > Sumit
> >
> > > Thanks,
> > > -Takahiro Akashi
> > >
> > > >  #define BOOT_TARGET_DEVICES(func) \
> > > >       func(SCSI, scsi, 0) \
> > > > @@ -42,4 +43,9 @@
> > > >
> > > >  #define CONFIG_SYS_CBSIZE 512
> > > >
> > > > +#define CONFIG_SYS_MONITOR_BASE              CONFIG_SYS_TEXT_BASE
> > > > +#define CONFIG_SYS_FLASH_BASE                0x0
> > > > +#define CONFIG_SYS_MAX_FLASH_BANKS   2
> > > > +#define CONFIG_SYS_MAX_FLASH_SECT    256 /* Sector: 256K, Bank: 64M */
> > > > +
> > > >  #endif /* __CONFIG_H */
> > > > --
> > > > 2.7.4
> > > >