| 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 | expand |
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 >
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 > >
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 > > >
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 > > > >
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 >
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 > >
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 > > >
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 > > > >
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 > > > > >
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
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
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.
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
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
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 */
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(-)