qemu-arm: Add persistent environment support

Message ID 1543231217-18799-1-git-send-email-sumit.garg@linaro.org
State Accepted
Commit 03fb0958c9090eb3add089cb9f5a8a5ecb30ddf6
Headers show
Series
  • qemu-arm: Add persistent environment support
Related show

Commit Message

Sumit Garg Nov. 26, 2018, 11:20 a.m.
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(-)

Comments

AKASHI Takahiro Nov. 27, 2018, 6:47 a.m. | #1
Sumit,

# I have a similar patch in my local branch :)

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?

> 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.

Do you have any reason to put env region in the second bank?
If not, I suggest that it be left free for other use.

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
>
Sumit Garg Nov. 27, 2018, 7:41 a.m. | #2
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.

> > 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.
>
> 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.

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.

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
> >
AKASHI Takahiro Nov. 27, 2018, 8:17 a.m. | #3
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.

> 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.

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
> > >
Sumit Garg Nov. 27, 2018, 9:51 a.m. | #4
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
> > > >
Sumit Garg Dec. 11, 2018, 12:34 p.m. | #5
On Mon, 26 Nov 2018 at 16:51, Sumit Garg <sumit.garg@linaro.org> 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(-)
>

Gentle reminder. Please let me know if you have any further comments.

-Sumit

> 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
> +CONFIG_CFI_FLASH=y
> +CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> +CONFIG_SYS_FLASH_CFI=y
> 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
>
>  #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
>
AKASHI Takahiro Dec. 12, 2018, 1:42 a.m. | #6
On Tue, Dec 11, 2018 at 06:04:05PM +0530, Sumit Garg wrote:
> On Mon, 26 Nov 2018 at 16:51, Sumit Garg <sumit.garg@linaro.org> 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(-)
> >
> 
> Gentle reminder. Please let me know if you have any further comments.

Another use case is atf + u-boot (although I don't know people are
interested in it). Put bl1.bin in flash0(0x0-0x4000000) and put
fip.bin in flash1(0x4000000-0x8000000). Please note that, with secure=on,
flash0 is in secure and flash1 is in non-secure.
While I admit that your patch is workable, my point is that there are
different use cases and it may not be a good idea to put one configuration
in qemu-arm.h.

Thanks,
-Takahiro Akashi


> -Sumit
> 
> > 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
> > +CONFIG_CFI_FLASH=y
> > +CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> > +CONFIG_SYS_FLASH_CFI=y
> > 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
> >
> >  #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
> >
Sumit Garg Dec. 12, 2018, 6:44 a.m. | #7
On Wed, 12 Dec 2018 at 07:09, Takahiro Akashi
<takahiro.akashi@linaro.org> wrote:
>
> On Tue, Dec 11, 2018 at 06:04:05PM +0530, Sumit Garg wrote:
> > On Mon, 26 Nov 2018 at 16:51, Sumit Garg <sumit.garg@linaro.org> 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(-)
> > >
> >
> > Gentle reminder. Please let me know if you have any further comments.
>
> Another use case is atf + u-boot (although I don't know people are
> interested in it). Put bl1.bin in flash0(0x0-0x4000000) and put
> fip.bin in flash1(0x4000000-0x8000000). Please note that, with secure=on,
> flash0 is in secure and flash1 is in non-secure.

I don't think current u-boot with "CONFIG_SYS_TEXT_BASE=0x00000000"
(flash0 address) could work in atf + u-boot configuration with bl33
address as NS_IMAGE_OFFSET=0x60000000 [1] (RAM address). Alternatively
we could use PRELOADED_BL33_BASE to specify flash address but that
certainly won't be flash0 start address.

Also from TF-A doc for qemu [2] it seems to support UEFI/edk2 boot.

IMHO, there should be separate u-boot ram defconfig to work with atf +
u-boot configuration. Also we may choose a different flash address for
environment for this target.

[1] https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/qemu/include/platform_def.h#L168
[2] https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/plat/qemu.rst

-Sumit

> While I admit that your patch is workable, my point is that there are
> different use cases and it may not be a good idea to put one configuration
> in qemu-arm.h.
>
> Thanks,
> -Takahiro Akashi
>
>
> > -Sumit
> >
> > > 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
> > > +CONFIG_CFI_FLASH=y
> > > +CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> > > +CONFIG_SYS_FLASH_CFI=y
> > > 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
> > >
> > >  #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
> > >
AKASHI Takahiro Dec. 12, 2018, 7:40 a.m. | #8
On Wed, Dec 12, 2018 at 12:14:27PM +0530, Sumit Garg wrote:
> On Wed, 12 Dec 2018 at 07:09, Takahiro Akashi
> <takahiro.akashi@linaro.org> wrote:
> >
> > On Tue, Dec 11, 2018 at 06:04:05PM +0530, Sumit Garg wrote:
> > > On Mon, 26 Nov 2018 at 16:51, Sumit Garg <sumit.garg@linaro.org> 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(-)
> > > >
> > >
> > > Gentle reminder. Please let me know if you have any further comments.
> >
> > Another use case is atf + u-boot (although I don't know people are
> > interested in it). Put bl1.bin in flash0(0x0-0x4000000) and put
> > fip.bin in flash1(0x4000000-0x8000000). Please note that, with secure=on,
> > flash0 is in secure and flash1 is in non-secure.
> 
> I don't think current u-boot with "CONFIG_SYS_TEXT_BASE=0x00000000"
> (flash0 address) could work in atf + u-boot configuration with bl33
> address as NS_IMAGE_OFFSET=0x60000000 [1] (RAM address). Alternatively
> we could use PRELOADED_BL33_BASE to specify flash address but that
> certainly won't be flash0 start address.

I was able to run atf + u-boot with some tweaks on some CONFIG_*
and even successfully ran linux kernel with bootefi.
But I don't want to go into details now.

> Also from TF-A doc for qemu [2] it seems to support UEFI/edk2 boot.
> 
> IMHO, there should be separate u-boot ram defconfig to work with atf +
> u-boot configuration. Also we may choose a different flash address for
> environment for this target.

My point is, again,


> [1] https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/qemu/include/platform_def.h#L168
> [2] https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/plat/qemu.rst
> 
> -Sumit
> 
> > While I admit that your patch is workable, my point is that there are
> > different use cases and it may not be a good idea to put one configuration
> > in qemu-arm.h.

here.
It's time that we need get opinions from maintainers or anybody else.

Thanks,
-Takahiro Akashi

> > Thanks,
> > -Takahiro Akashi
> >
> >
> > > -Sumit
> > >
> > > > 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
> > > > +CONFIG_CFI_FLASH=y
> > > > +CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> > > > +CONFIG_SYS_FLASH_CFI=y
> > > > 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
> > > >
> > > >  #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
> > > >
Sumit Garg Dec. 12, 2018, 11:27 a.m. | #9
On Wed, 12 Dec 2018 at 13:07, Takahiro Akashi
<takahiro.akashi@linaro.org> wrote:
>
> On Wed, Dec 12, 2018 at 12:14:27PM +0530, Sumit Garg wrote:
> > On Wed, 12 Dec 2018 at 07:09, Takahiro Akashi
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > On Tue, Dec 11, 2018 at 06:04:05PM +0530, Sumit Garg wrote:
> > > > On Mon, 26 Nov 2018 at 16:51, Sumit Garg <sumit.garg@linaro.org> 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(-)
> > > > >
> > > >
> > > > Gentle reminder. Please let me know if you have any further comments.
> > >
> > > Another use case is atf + u-boot (although I don't know people are
> > > interested in it). Put bl1.bin in flash0(0x0-0x4000000) and put
> > > fip.bin in flash1(0x4000000-0x8000000). Please note that, with secure=on,
> > > flash0 is in secure and flash1 is in non-secure.
> >
> > I don't think current u-boot with "CONFIG_SYS_TEXT_BASE=0x00000000"
> > (flash0 address) could work in atf + u-boot configuration with bl33
> > address as NS_IMAGE_OFFSET=0x60000000 [1] (RAM address). Alternatively
> > we could use PRELOADED_BL33_BASE to specify flash address but that
> > certainly won't be flash0 start address.
>
> I was able to run atf + u-boot with some tweaks on some CONFIG_*
> and even successfully ran linux kernel with bootefi.
> But I don't want to go into details now.
>
> > Also from TF-A doc for qemu [2] it seems to support UEFI/edk2 boot.
> >
> > IMHO, there should be separate u-boot ram defconfig to work with atf +
> > u-boot configuration. Also we may choose a different flash address for
> > environment for this target.
>
> My point is, again,
>
>
> > [1] https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/qemu/include/platform_def.h#L168
> > [2] https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/plat/qemu.rst
> >
> > -Sumit
> >
> > > While I admit that your patch is workable, my point is that there are
> > > different use cases and it may not be a good idea to put one configuration
> > > in qemu-arm.h.
>
> here.

So do you suggest here to move following config options to Kconfig?

1. CONFIG_ENV_ADDR
2. CONFIG_ENV_SIZE
3. CONFIG_ENV_OFFSET

If yes, I think this could be explored.

-Sumit

> It's time that we need get opinions from maintainers or anybody else.
>
> Thanks,
> -Takahiro Akashi
>
> > > Thanks,
> > > -Takahiro Akashi
> > >
> > >
> > > > -Sumit
> > > >
> > > > > 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
> > > > > +CONFIG_CFI_FLASH=y
> > > > > +CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
> > > > > +CONFIG_SYS_FLASH_CFI=y
> > > > > 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
> > > > >
> > > > >  #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
> > > > >
Tuomas Tynkkynen Dec. 13, 2018, 12:43 a.m. | #10
Hi Sumit, Takahiro,

On Wed, 12 Dec 2018 10:42:56 +0900
Takahiro Akashi <takahiro.akashi@linaro.org> wrote:

> On Tue, Dec 11, 2018 at 06:04:05PM +0530, Sumit Garg wrote:
> > On Mon, 26 Nov 2018 at 16:51, Sumit Garg <sumit.garg@linaro.org>
> > 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(-)
> > >  
> > 
> > Gentle reminder. Please let me know if you have any further
> > comments.  

I'm not too familiar with the UEFI/ATF related aspects here, but I
think that the read-only parts (aka u-boot.bin) and read-write parts
(the U-Boot environment) should belong to different files (which is
what this patch series does). Basically, IMO as a normal user I should
be able to run QEMU on a distro-provided U-Boot binary with something
like:

    qemu-system-arm -bios /usr/share/u-boot/qemu_arm.bin

and not have it fail due to not having write permission to /usr/.

> Another use case is atf + u-boot (although I don't know people are
> interested in it). Put bl1.bin in flash0(0x0-0x4000000) and put
> fip.bin in flash1(0x4000000-0x8000000). Please note that, with
> secure=on, flash0 is in secure and flash1 is in non-secure.
> While I admit that your patch is workable, my point is that there are
> different use cases and it may not be a good idea to put one
> configuration in qemu-arm.h.

Can EDK2 in QEMU boot with ATF and if so, how does it lay out things?
Would it be possible to build U-Boot in such a way that u-boot.bin
could be substituted in existing build scripts or instructions in place
of the EDK2 binary so that things still work the same?

Or in other words, if EDK2 has already has a working
implementation of something (such as the flash layout), IMO we should
prefer to use that instead of reimplementing it in a different
way.

- Tuomas
Tom Rini Dec. 13, 2018, 12:47 a.m. | #11
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>


Applied to u-boot/master, thanks!

-- 
Tom
Daniel Thompson Dec. 13, 2018, 9:30 a.m. | #12
On Thu, Dec 13, 2018 at 02:43:58AM +0200, Tuomas Tynkkynen wrote:
> > Another use case is atf + u-boot (although I don't know people are
> > interested in it). Put bl1.bin in flash0(0x0-0x4000000) and put
> > fip.bin in flash1(0x4000000-0x8000000). Please note that, with
> > secure=on, flash0 is in secure and flash1 is in non-secure.
> > While I admit that your patch is workable, my point is that there are
> > different use cases and it may not be a good idea to put one
> > configuration in qemu-arm.h.
> 
> Can EDK2 in QEMU boot with ATF and if so, how does it lay out things?
>
> Would it be possible to build U-Boot in such a way that u-boot.bin
> could be substituted in existing build scripts or instructions in place
> of the EDK2 binary so that things still work the same?
> 
> Or in other words, if EDK2 has already has a working
> implementation of something (such as the flash layout), IMO we should
> prefer to use that instead of reimplementing it in a different
> way.

The EDK2 binaries I am using don't include ATF. IIRC qemu with default
arguments boots directly into EL1 so most of the off-the-shelf binaries
will not include a trusted firmware.

EDK2 keeps its varstore on the second pflash.


Daniel.
AKASHI Takahiro Dec. 14, 2018, 11 a.m. | #13
On Thu, Dec 13, 2018 at 02:43:58AM +0200, Tuomas Tynkkynen wrote:
> Hi Sumit, Takahiro,
> 
> On Wed, 12 Dec 2018 10:42:56 +0900
> Takahiro Akashi <takahiro.akashi@linaro.org> wrote:
> 
> > On Tue, Dec 11, 2018 at 06:04:05PM +0530, Sumit Garg wrote:
> > > On Mon, 26 Nov 2018 at 16:51, Sumit Garg <sumit.garg@linaro.org>
> > > 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(-)
> > > >  
> > > 
> > > Gentle reminder. Please let me know if you have any further
> > > comments.  
> 
> I'm not too familiar with the UEFI/ATF related aspects here, but I
> think that the read-only parts (aka u-boot.bin) and read-write parts
> (the U-Boot environment) should belong to different files (which is
> what this patch series does). Basically, IMO as a normal user I should
> be able to run QEMU on a distro-provided U-Boot binary with something
> like:

So probably I'm not a normal user.
Tom has already applied this patch, and I would change qemu-arm.h
in my local repo if necessary. That's fine.

>     qemu-system-arm -bios /usr/share/u-boot/qemu_arm.bin

# As u-boot is quite board-specific, I don't think distro's default
u-boot, if any, won't work on qemu.

> and not have it fail due to not having write permission to /usr/.
> 
> > Another use case is atf + u-boot (although I don't know people are
> > interested in it). Put bl1.bin in flash0(0x0-0x4000000) and put
> > fip.bin in flash1(0x4000000-0x8000000). Please note that, with
> > secure=on, flash0 is in secure and flash1 is in non-secure.
> > While I admit that your patch is workable, my point is that there are
> > different use cases and it may not be a good idea to put one
> > configuration in qemu-arm.h.

> Can EDK2 in QEMU boot with ATF and if so, how does it lay out things?

Do you mean UEFI? EDK2 is an implementation, UEFI is the specification/
interface.
Just FYI, as I said, I experimentally succeeded to run atf + u-boot
as BL33, only modifying CONFIG_SYS_TEXT_BASE and CONFIG_SYS_FLASH_BASE
(and CONFIG_ENV_* if needed).

> Would it be possible to build U-Boot in such a way that u-boot.bin
> could be substituted in existing build scripts or instructions in place
> of the EDK2 binary so that things still work the same?
> 
> Or in other words, if EDK2 has already has a working
> implementation of something (such as the flash layout), IMO we should
> prefer to use that instead of reimplementing it in a different
> way.

It is up to the implementation how efi variables are stored
in storage while efi variables on u-boot are mapped to corresponding
u-boot environment variables. So there's no compatibility in flash layout
between EDK2 and u-boot/UEFI.

Thanks,
-Takahiro Akashi


> - Tuomas
Daniel Thompson Dec. 14, 2018, 11:11 a.m. | #14
On Fri, Dec 14, 2018 at 08:00:11PM +0900, Takahiro Akashi wrote:
> On Thu, Dec 13, 2018 at 02:43:58AM +0200, Tuomas Tynkkynen wrote:
> > Hi Sumit, Takahiro,
> > 
> > On Wed, 12 Dec 2018 10:42:56 +0900
> > Takahiro Akashi <takahiro.akashi@linaro.org> wrote:
> > 
> > > On Tue, Dec 11, 2018 at 06:04:05PM +0530, Sumit Garg wrote:
> > > > On Mon, 26 Nov 2018 at 16:51, Sumit Garg <sumit.garg@linaro.org>
> > > > 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(-)
> > > > >  
> > > > 
> > > > Gentle reminder. Please let me know if you have any further
> > > > comments.  
> > 
> > I'm not too familiar with the UEFI/ATF related aspects here, but I
> > think that the read-only parts (aka u-boot.bin) and read-write parts
> > (the U-Boot environment) should belong to different files (which is
> > what this patch series does). Basically, IMO as a normal user I should
> > be able to run QEMU on a distro-provided U-Boot binary with something
> > like:
> 
> So probably I'm not a normal user.
> Tom has already applied this patch, and I would change qemu-arm.h
> in my local repo if necessary. That's fine.

To be honest I think we should seriously consider changing TF-A so that
is doesn't look for the FIP in the non-secure flash. When TF-A and the
secure world payload are part of the FIP then loading them from
non-secure flash is a very odd thing to do (in addition to the
practical problems related to read-only binaries in /usr when firmware
and varstore live in the same pflash).


Daniel.

> 
> >     qemu-system-arm -bios /usr/share/u-boot/qemu_arm.bin
> 
> # As u-boot is quite board-specific, I don't think distro's default
> u-boot, if any, won't work on qemu.
> 
> > and not have it fail due to not having write permission to /usr/.
> > 
> > > Another use case is atf + u-boot (although I don't know people are
> > > interested in it). Put bl1.bin in flash0(0x0-0x4000000) and put
> > > fip.bin in flash1(0x4000000-0x8000000). Please note that, with
> > > secure=on, flash0 is in secure and flash1 is in non-secure.
> > > While I admit that your patch is workable, my point is that there are
> > > different use cases and it may not be a good idea to put one
> > > configuration in qemu-arm.h.
> 
> > Can EDK2 in QEMU boot with ATF and if so, how does it lay out things?
> 
> Do you mean UEFI? EDK2 is an implementation, UEFI is the specification/
> interface.
> Just FYI, as I said, I experimentally succeeded to run atf + u-boot
> as BL33, only modifying CONFIG_SYS_TEXT_BASE and CONFIG_SYS_FLASH_BASE
> (and CONFIG_ENV_* if needed).
> 
> > Would it be possible to build U-Boot in such a way that u-boot.bin
> > could be substituted in existing build scripts or instructions in place
> > of the EDK2 binary so that things still work the same?
> > 
> > Or in other words, if EDK2 has already has a working
> > implementation of something (such as the flash layout), IMO we should
> > prefer to use that instead of reimplementing it in a different
> > way.
> 
> It is up to the implementation how efi variables are stored
> in storage while efi variables on u-boot are mapped to corresponding
> u-boot environment variables. So there's no compatibility in flash layout
> between EDK2 and u-boot/UEFI.
> 
> Thanks,
> -Takahiro Akashi
> 
> 
> > - Tuomas

Patch

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
+CONFIG_CFI_FLASH=y
+CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
+CONFIG_SYS_FLASH_CFI=y
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
 
 #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 */