Re: [U-Boot] [RFC PATCH 1/2] sunxi: retrieve FEL-provided values to environment variables

2015-09-14 Thread Bernhard Nortmann

Hi Ian, hello Hans!

That's an interesting find, Ian - thank you.

Unfortunately it seems that flagging our environment vars accordingly
isn't enough (on its own) to prevent them from being written by
"saveenv". I've been testing

#define CONFIG_ENV_FLAGS_LIST_STATIC "fel_booted:bo,fel_scriptaddr:xo"

which achieves the desired flags, but doesn't alter saveenv behaviour.
Setting them to read-only ("r" access flag) doesn't help, as it won't
allow the setenv_*() calls to alter the vars in the first place.

As you have observed, this is a more general U-Boot problem - other
'volatile' information might end up in the environment (and saveenv)
as well, e.g. "ipaddr" and "serverip" retrieved by DHCP.

Regards, B. Nortmann


Am 12.09.2015 um 14:24 schrieb Hans de Goede:

Hi,

On 12-09-15 13:58, Ian Campbell wrote:

On Thu, 2015-09-10 at 20:34 +0200, Hans de Goede wrote:

[...]

What if the user interrupts auto-boot with a fel provided boot.scr
and then does "saveenv" ?


This is an interesting question which is more generic than just these
variable, i.e. it applies to some extent to "ipaddr" when someone does
"dhcp ; saveenv" too.

Grepping around to see if there was any special handling for ipaddr I
came across "Vendor Parameter Protection" in the top-level README as
well as "CONFIG_ENV_FLAGS_LIST_DEFAULT" (and _STATIC) and various
default settings in include/env_flags.h.

I think CONFIG_ENV_FLAGS_LIST_* are what we want, and we want fel* to
be flagged "r" for read only


Ah, yes that sounds exactly what we want, thanks for figuring that
out.


and perhaps given an appropriate type
(either "d" or "x" for decimal or hex respectively, I suppose).


Regards,

Hans


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RFC PATCH 1/2] sunxi: retrieve FEL-provided values to environment variables

2015-09-12 Thread Ian Campbell
On Thu, 2015-09-10 at 20:34 +0200, Hans de Goede wrote:
> [...]
> 
> What if the user interrupts auto-boot with a fel provided boot.scr
> and then does "saveenv" ?

This is an interesting question which is more generic than just these
variable, i.e. it applies to some extent to "ipaddr" when someone does
"dhcp ; saveenv" too.

Grepping around to see if there was any special handling for ipaddr I
came across "Vendor Parameter Protection" in the top-level README as
well as "CONFIG_ENV_FLAGS_LIST_DEFAULT" (and _STATIC) and various
default settings in include/env_flags.h.

I think CONFIG_ENV_FLAGS_LIST_* are what we want, and we want fel* to
be flagged "r" for read only and perhaps given an appropriate type
(either "d" or "x" for decimal or hex respectively, I suppose).

Ian.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RFC PATCH 1/2] sunxi: retrieve FEL-provided values to environment variables

2015-09-12 Thread Hans de Goede

Hi,

On 12-09-15 13:58, Ian Campbell wrote:

On Thu, 2015-09-10 at 20:34 +0200, Hans de Goede wrote:

[...]

What if the user interrupts auto-boot with a fel provided boot.scr
and then does "saveenv" ?


This is an interesting question which is more generic than just these
variable, i.e. it applies to some extent to "ipaddr" when someone does
"dhcp ; saveenv" too.

Grepping around to see if there was any special handling for ipaddr I
came across "Vendor Parameter Protection" in the top-level README as
well as "CONFIG_ENV_FLAGS_LIST_DEFAULT" (and _STATIC) and various
default settings in include/env_flags.h.

I think CONFIG_ENV_FLAGS_LIST_* are what we want, and we want fel* to
be flagged "r" for read only


Ah, yes that sounds exactly what we want, thanks for figuring that
out.


and perhaps given an appropriate type
(either "d" or "x" for decimal or hex respectively, I suppose).


Regards,

Hans
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RFC PATCH 1/2] sunxi: retrieve FEL-provided values to environment variables

2015-09-11 Thread Bernhard Nortmann

Hi Hans!

Am 10.09.2015 um 20:34 schrieb Hans de Goede:


What if the user interrupts auto-boot with a fel provided boot.scr
and then does "saveenv" ?

Then we end up with a fel_data_addr and fel_data_size permanently
in the env.

At a minimum this function must always do:

setenv("fel_data_addr", NULL);
setenv("fel_data_size", NULL);

(rather then only when we've a spl fel header but no addr / size)

So that we do not end up trying to interpret old values ever.


Good point.



Ideally though we would find another way not involving putting these
in the environment (not sure if that is easily doable).


I also have no idea how to achieve that, especially if we want to keep
the FEL case "in line" with other, more conventional ways of booting.


Maybe clear all env variables here?

Or even better clear them all unconditionally, and then set
them when the checks succeed ?



That's something that may be best aligned somehow with the NAND and
MMC boot handling, if possible? I.e. have some centralized piece of
code that determines the active boot method and retrieves/presets the
environments accordingly. I'll have to re-examine that more closely.

Regards, B. Nortmann
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RFC PATCH 1/2] sunxi: retrieve FEL-provided values to environment variables

2015-09-10 Thread Hans de Goede

Hi,

On 03-09-15 16:11, Bernhard Nortmann wrote:

This patch extends the misc_init_r() function on sunxi boards
to test for the presence of a suitable "sunxi" SPL header. If
found, and the loader ("fel" utility) provided non-zero values
for either data address or size, then corresponding environment
variables get set.

misc_init_r() also sets (or clears) the "fel_booted" variable depending
on the active boot device, using the same logic as spl_boot_device().

The goal is to provide sufficient information (within the U-Boot
environment) to make intelligent decisions on how to continue the boot
process, allowing specific customizations for the "FEL boot" case.

Signed-off-by: Bernhard Nortmann 
---

  board/sunxi/board.c | 59 +
  1 file changed, 59 insertions(+)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 9c855f6..ceab1e0 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -516,6 +516,54 @@ void get_board_serial(struct tag_serialnr *serialnr)
  }
  #endif

+#if !defined(CONFIG_SPL_BUILD)
+static int check_signature(unsigned long io_addr, const char *signature,
+  int length)
+{
+   do {
+   if (readb(io_addr) != *signature)
+   return 0;
+   io_addr++;
+   signature++;
+   } while (--length > 0);
+   return 1;
+}
+
+#define SPL_SIGNATURE  "SPL" /* marks "sunxi" header */
+#define SPL_EXPECTED_VERSION   1
+
+/*
+ * Check the SPL header for the "sunxi" variant. If found: parse values
+ * that might have been passed by the loader ("fel" utility), and update
+ * the environment accordingly.
+ */
+static void parse_spl_header(void)
+{
+   uint8_t spl_header_version;
+   uint32_t fel_data_address;
+   uint32_t fel_data_size;
+
+   if (check_signature(0x14, SPL_SIGNATURE, 3)) {
+   spl_header_version = readb(0x17);
+   if (spl_header_version == SPL_EXPECTED_VERSION) {
+   fel_data_address = readl(0x18);
+   fel_data_size = readl(0x1C);
+   if (fel_data_address)
+   setenv_hex("fel_data_addr", fel_data_address);
+   else
+   setenv("fel_data_addr", NULL);
+   if (fel_data_size)
+   setenv_hex("fel_data_size", fel_data_size);
+   else
+   setenv("fel_data_size", NULL);
+   } else {
+   printf("sunxi SPL header version mismatch: expected %u, got 
%u\n",
+  SPL_EXPECTED_VERSION, spl_header_version);
+   }
+   }
+}


What if the user interrupts auto-boot with a fel provided boot.scr
and then does "saveenv" ?

Then we end up with a fel_data_addr and fel_data_size permanently
in the env.

At a minimum this function must always do:

setenv("fel_data_addr", NULL);
setenv("fel_data_size", NULL);

(rather then only when we've a spl fel header but no addr / size)

So that we do not end up trying to interpret old values ever.

Ideally though we would find another way not involving putting these
in the environment (not sure if that is easily doable).



+#endif /* !defined(CONFIG_SPL_BUILD) */
+
  #ifdef CONFIG_MISC_INIT_R
  int misc_init_r(void)
  {
@@ -524,6 +572,17 @@ int misc_init_r(void)
uint8_t mac_addr[6];
int ret;

+#if !defined(CONFIG_SPL_BUILD)
+   /* determine if we are running in FEL mode */
+   if (readl(4) != 0x4E4F4765 || readl(8) != 0x3054422E) { /* eGON.BT0 */
+   setenv("fel_booted", "1");
+   parse_spl_header();
+   } else {
+   /* not in FEL mode, delete/clear env var (if present) */
+   setenv("fel_booted", NULL);


Maybe clear all env variables here?

Or even better clear them all unconditionally, and then set
them when the checks succeed ?



+   }
+#endif
+
ret = sunxi_get_sid(sid);
if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
if (!getenv("ethaddr")) {



Regards,

Hans
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [RFC PATCH 1/2] sunxi: retrieve FEL-provided values to environment variables

2015-09-03 Thread Bernhard Nortmann
This patch extends the misc_init_r() function on sunxi boards
to test for the presence of a suitable "sunxi" SPL header. If
found, and the loader ("fel" utility) provided non-zero values
for either data address or size, then corresponding environment
variables get set.

misc_init_r() also sets (or clears) the "fel_booted" variable depending
on the active boot device, using the same logic as spl_boot_device().

The goal is to provide sufficient information (within the U-Boot
environment) to make intelligent decisions on how to continue the boot
process, allowing specific customizations for the "FEL boot" case.

Signed-off-by: Bernhard Nortmann 
---

 board/sunxi/board.c | 59 +
 1 file changed, 59 insertions(+)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 9c855f6..ceab1e0 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -516,6 +516,54 @@ void get_board_serial(struct tag_serialnr *serialnr)
 }
 #endif
 
+#if !defined(CONFIG_SPL_BUILD)
+static int check_signature(unsigned long io_addr, const char *signature,
+  int length)
+{
+   do {
+   if (readb(io_addr) != *signature)
+   return 0;
+   io_addr++;
+   signature++;
+   } while (--length > 0);
+   return 1;
+}
+
+#define SPL_SIGNATURE  "SPL" /* marks "sunxi" header */
+#define SPL_EXPECTED_VERSION   1
+
+/*
+ * Check the SPL header for the "sunxi" variant. If found: parse values
+ * that might have been passed by the loader ("fel" utility), and update
+ * the environment accordingly.
+ */
+static void parse_spl_header(void)
+{
+   uint8_t spl_header_version;
+   uint32_t fel_data_address;
+   uint32_t fel_data_size;
+
+   if (check_signature(0x14, SPL_SIGNATURE, 3)) {
+   spl_header_version = readb(0x17);
+   if (spl_header_version == SPL_EXPECTED_VERSION) {
+   fel_data_address = readl(0x18);
+   fel_data_size = readl(0x1C);
+   if (fel_data_address)
+   setenv_hex("fel_data_addr", fel_data_address);
+   else
+   setenv("fel_data_addr", NULL);
+   if (fel_data_size)
+   setenv_hex("fel_data_size", fel_data_size);
+   else
+   setenv("fel_data_size", NULL);
+   } else {
+   printf("sunxi SPL header version mismatch: expected %u, 
got %u\n",
+  SPL_EXPECTED_VERSION, spl_header_version);
+   }
+   }
+}
+#endif /* !defined(CONFIG_SPL_BUILD) */
+
 #ifdef CONFIG_MISC_INIT_R
 int misc_init_r(void)
 {
@@ -524,6 +572,17 @@ int misc_init_r(void)
uint8_t mac_addr[6];
int ret;
 
+#if !defined(CONFIG_SPL_BUILD)
+   /* determine if we are running in FEL mode */
+   if (readl(4) != 0x4E4F4765 || readl(8) != 0x3054422E) { /* eGON.BT0 */
+   setenv("fel_booted", "1");
+   parse_spl_header();
+   } else {
+   /* not in FEL mode, delete/clear env var (if present) */
+   setenv("fel_booted", NULL);
+   }
+#endif
+
ret = sunxi_get_sid(sid);
if (ret == 0 && sid[0] != 0 && sid[3] != 0) {
if (!getenv("ethaddr")) {
-- 
2.4.6

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot