Re: [PATCH v2 3/6] efi_loader: don't load signature database from file
On 8/27/21 6:49 AM, AKASHI Takahiro wrote:
On Fri, Aug 27, 2021 at 06:42:39AM +0200, Heinrich Schuchardt wrote:
On 8/27/21 6:12 AM, AKASHI Takahiro wrote:
On Thu, Aug 26, 2021 at 03:48:02PM +0200, Heinrich Schuchardt wrote:
The UEFI specification requires that the signature database may only be
stored in tamper-resistant storage. So these variable may not be read
from an unsigned file.
I don't have a strong opinion here, but it seems to be too restrictive.
Nobody expects that file-based variable implementation is *safe*.
Leave it as it is so that people can easily experiment secure boot.
If the prior boot stage checks the integrity of the U-Boot binary, the
file based implementation becomes 'safe' with this patch.
How safe (or secure) is it? That is a question.
What is your thread model?
The preseed store is as safe as the capsule updates that Linaro is
working on where the certificate for verifying the capsule is baked into
U-Boot or the StMM based variables.
They all require that an attacker can neither load a manipulated U-Boot
nor that he can alter the memory containing U-Boot at runtime.
Best regards
Heinrich
-Takahiro Akashi
Users can still experiment with secure boot by setting the secure boot
variables via the efidebug command.
I cannot see a use case for having the secure boot data base on an
insecure medium.
Best regards
Heinrich
-Takahiro Akashi
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
include/efi_variable.h | 5 +++-
lib/efi_loader/efi_var_common.c | 2 --
lib/efi_loader/efi_var_file.c | 41 -
lib/efi_loader/efi_variable.c | 2 +-
4 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h
index 4623a64142..2d97655e1f 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct
efi_var_file **bufp, loff_t *
/**
* efi_var_restore() - restore EFI variables from buffer
*
+ * Only if @safe is set secure boot related variables will be restored.
+ *
* @buf: buffer
+ * @safe: restoring from tamper-resistant storage
* Return: status code
*/
-efi_status_t efi_var_restore(struct efi_var_file *buf);
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
/**
* efi_var_from_file() - read variables from file
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index cf7afecd60..b0c5b672c5 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[] = {
{u"KEK", _global_variable_guid, EFI_AUTH_VAR_KEK},
{u"db", _guid_image_security_database, EFI_AUTH_VAR_DB},
{u"dbx", _guid_image_security_database, EFI_AUTH_VAR_DBX},
- /* not used yet
{u"dbt", _guid_image_security_database, EFI_AUTH_VAR_DBT},
{u"dbr", _guid_image_security_database, EFI_AUTH_VAR_DBR},
- */
};
static bool efi_secure_boot;
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index de076b8cbc..c7c6805ed0 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -148,9 +148,10 @@ error:
#endif
}
-efi_status_t efi_var_restore(struct efi_var_file *buf)
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
{
struct efi_var_entry *var, *last_var;
+ u16 *data;
efi_status_t ret;
if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
@@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file *buf)
return EFI_INVALID_PARAMETER;
}
- var = buf->var;
last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
- while (var < last_var) {
- u16 *data = var->name + u16_strlen(var->name) + 1;
-
- if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
- ret = efi_var_mem_ins(var->name, >guid, var->attr,
- var->length, data, 0, NULL,
- var->time);
- if (ret != EFI_SUCCESS)
- log_err("Failed to set EFI variable %ls\n",
- var->name);
- }
- var = (struct efi_var_entry *)
- ALIGN((uintptr_t)data + var->length, 8);
+ for (var = buf->var; var < last_var;
+var = (struct efi_var_entry *)
+ ALIGN((uintptr_t)data + var->length, 8)) {
+
+ data = var->name + u16_strlen(var->name) + 1;
+
+ /*
+* Secure boot related and non-volatile variables shall only be
+* restored from U-Boot's preseed.
+*/
+ if (!safe &&
+
Re: [PATCH v2 6/6] efi_loader: always initialize the secure boot state
On 8/27/21 6:47 AM, AKASHI Takahiro wrote:
On Fri, Aug 27, 2021 at 06:34:30AM +0200, Heinrich Schuchardt wrote:
On 8/27/21 5:53 AM, AKASHI Takahiro wrote:
On Thu, Aug 26, 2021 at 03:48:05PM +0200, Heinrich Schuchardt wrote:
Even if we cannot read the variable store from disk we still need to
initialize the secure boot state.
Don't continue to boot if the variable preseed is invalid as this indicates
that the variable store has been tampered.
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
lib/efi_loader/efi_variable.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 80996d0f47..6d92229e2a 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -427,13 +427,17 @@ efi_status_t efi_init_variables(void)
if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
ret = efi_var_restore((struct efi_var_file *)
__efi_var_file_begin, true);
- if (ret != EFI_SUCCESS)
+ if (ret != EFI_SUCCESS) {
log_err("Invalid EFI variable seed\n");
+ return ret;
+ }
}
-
- ret = efi_var_from_file();
+ ret = efi_init_secure_state();
if (ret != EFI_SUCCESS)
return ret;
- return efi_init_secure_state();
+ /* Don't stop booting if variable store is not available */
+ efi_var_from_file();
I think we have to think about two different cases:
1) there is no "variable store" file available.
2) it does exists, but reading from it (efi_var_restore()) failed
For (2), we should return with an error as in the case of
CONFIG_EFI_VARIABLES_PRESEED.
Otherwise, the behavior is inconsistent.
The preseed store is used for secure boot related variables.
Where does this restriction come from?
Kconfig says:
Include a file with the initial values for non-volatile UEFI variables
into the U-Boot binary. If this configuration option is set, changes
to authentication related variables (PK, KEK, db, dbx) are not
allowed.
# oops: I didn't notice the last sentence.
# but anyhow, it seems too restrictive.
The UEFI spec requires that the secure boot database is stored in
tamper-resistant storage. So it cannot be on the ESP.
Best regards
Heinrich
If this
store is inconsistent, failing is required to ensure secure boot.
As I said, a file-based variable configuration cannot work
as a secure platform any way.
Why do we need this kind of restriction?
-Takahiro Akashi
With my patches applied the file store can not contain any secure boot
related variables but it may contain boot options.
Your suggestion could mean that if the file store is corrupted the board
is bricked.
If the boot options cannot be read either because the file does not
exist or because the file is corrupt, I still want the user to have a
chance to load shim, GRUB, or the kernel and boot via the bootefi command.
I cannot see any inconsistency here.
Best regards
Heinrich
Re: [PATCH v2 3/6] efi_loader: don't load signature database from file
On Fri, Aug 27, 2021 at 01:49:41PM +0900, AKASHI Takahiro wrote:
> On Fri, Aug 27, 2021 at 06:42:39AM +0200, Heinrich Schuchardt wrote:
> > On 8/27/21 6:12 AM, AKASHI Takahiro wrote:
> > > On Thu, Aug 26, 2021 at 03:48:02PM +0200, Heinrich Schuchardt wrote:
> > > > The UEFI specification requires that the signature database may only be
> > > > stored in tamper-resistant storage. So these variable may not be read
> > > > from an unsigned file.
> > >
> > > I don't have a strong opinion here, but it seems to be too restrictive.
> > > Nobody expects that file-based variable implementation is *safe*.
> > > Leave it as it is so that people can easily experiment secure boot.
> >
> > If the prior boot stage checks the integrity of the U-Boot binary, the
> > file based implementation becomes 'safe' with this patch.
>
> How safe (or secure) is it? That is a question.
> What is your thread model?
Obviously, thread -> threat
>
> -Takahiro Akashi
>
>
> > Users can still experiment with secure boot by setting the secure boot
> > variables via the efidebug command.
> >
> > I cannot see a use case for having the secure boot data base on an
> > insecure medium.
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > > -Takahiro Akashi
> > >
> > >
> > > > Signed-off-by: Heinrich Schuchardt
> > > > ---
> > > > v2:
> > > > no change
> > > > ---
> > > > include/efi_variable.h | 5 +++-
> > > > lib/efi_loader/efi_var_common.c | 2 --
> > > > lib/efi_loader/efi_var_file.c | 41 -
> > > > lib/efi_loader/efi_variable.c | 2 +-
> > > > 4 files changed, 30 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/include/efi_variable.h b/include/efi_variable.h
> > > > index 4623a64142..2d97655e1f 100644
> > > > --- a/include/efi_variable.h
> > > > +++ b/include/efi_variable.h
> > > > @@ -161,10 +161,13 @@ efi_status_t __maybe_unused
> > > > efi_var_collect(struct efi_var_file **bufp, loff_t *
> > > > /**
> > > >* efi_var_restore() - restore EFI variables from buffer
> > > >*
> > > > + * Only if @safe is set secure boot related variables will be restored.
> > > > + *
> > > >* @buf: buffer
> > > > + * @safe: restoring from tamper-resistant storage
> > > >* Return:status code
> > > >*/
> > > > -efi_status_t efi_var_restore(struct efi_var_file *buf);
> > > > +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
> > > >
> > > > /**
> > > >* efi_var_from_file() - read variables from file
> > > > diff --git a/lib/efi_loader/efi_var_common.c
> > > > b/lib/efi_loader/efi_var_common.c
> > > > index cf7afecd60..b0c5b672c5 100644
> > > > --- a/lib/efi_loader/efi_var_common.c
> > > > +++ b/lib/efi_loader/efi_var_common.c
> > > > @@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type
> > > > name_type[] = {
> > > > {u"KEK", _global_variable_guid, EFI_AUTH_VAR_KEK},
> > > > {u"db", _guid_image_security_database, EFI_AUTH_VAR_DB},
> > > > {u"dbx", _guid_image_security_database, EFI_AUTH_VAR_DBX},
> > > > - /* not used yet
> > > > {u"dbt", _guid_image_security_database, EFI_AUTH_VAR_DBT},
> > > > {u"dbr", _guid_image_security_database, EFI_AUTH_VAR_DBR},
> > > > - */
> > > > };
> > > >
> > > > static bool efi_secure_boot;
> > > > diff --git a/lib/efi_loader/efi_var_file.c
> > > > b/lib/efi_loader/efi_var_file.c
> > > > index de076b8cbc..c7c6805ed0 100644
> > > > --- a/lib/efi_loader/efi_var_file.c
> > > > +++ b/lib/efi_loader/efi_var_file.c
> > > > @@ -148,9 +148,10 @@ error:
> > > > #endif
> > > > }
> > > >
> > > > -efi_status_t efi_var_restore(struct efi_var_file *buf)
> > > > +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
> > > > {
> > > > struct efi_var_entry *var, *last_var;
> > > > + u16 *data;
> > > > efi_status_t ret;
> > > >
> > > > if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
> > > > @@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file
> > > > *buf)
> > > > return EFI_INVALID_PARAMETER;
> > > > }
> > > >
> > > > - var = buf->var;
> > > > last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
> > > > - while (var < last_var) {
> > > > - u16 *data = var->name + u16_strlen(var->name) + 1;
> > > > -
> > > > - if (var->attr & EFI_VARIABLE_NON_VOLATILE &&
> > > > var->length) {
> > > > - ret = efi_var_mem_ins(var->name, >guid,
> > > > var->attr,
> > > > - var->length, data, 0,
> > > > NULL,
> > > > - var->time);
> > > > - if (ret != EFI_SUCCESS)
> > > > - log_err("Failed to set EFI variable
> > > > %ls\n",
> > > > - var->name);
> > > > - }
> > > > -
Re: [PATCH v2 3/6] efi_loader: don't load signature database from file
On Fri, Aug 27, 2021 at 06:42:39AM +0200, Heinrich Schuchardt wrote:
> On 8/27/21 6:12 AM, AKASHI Takahiro wrote:
> > On Thu, Aug 26, 2021 at 03:48:02PM +0200, Heinrich Schuchardt wrote:
> > > The UEFI specification requires that the signature database may only be
> > > stored in tamper-resistant storage. So these variable may not be read
> > > from an unsigned file.
> >
> > I don't have a strong opinion here, but it seems to be too restrictive.
> > Nobody expects that file-based variable implementation is *safe*.
> > Leave it as it is so that people can easily experiment secure boot.
>
> If the prior boot stage checks the integrity of the U-Boot binary, the
> file based implementation becomes 'safe' with this patch.
How safe (or secure) is it? That is a question.
What is your thread model?
-Takahiro Akashi
> Users can still experiment with secure boot by setting the secure boot
> variables via the efidebug command.
>
> I cannot see a use case for having the secure boot data base on an
> insecure medium.
>
> Best regards
>
> Heinrich
>
> >
> > -Takahiro Akashi
> >
> >
> > > Signed-off-by: Heinrich Schuchardt
> > > ---
> > > v2:
> > > no change
> > > ---
> > > include/efi_variable.h | 5 +++-
> > > lib/efi_loader/efi_var_common.c | 2 --
> > > lib/efi_loader/efi_var_file.c | 41 -
> > > lib/efi_loader/efi_variable.c | 2 +-
> > > 4 files changed, 30 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/include/efi_variable.h b/include/efi_variable.h
> > > index 4623a64142..2d97655e1f 100644
> > > --- a/include/efi_variable.h
> > > +++ b/include/efi_variable.h
> > > @@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct
> > > efi_var_file **bufp, loff_t *
> > > /**
> > >* efi_var_restore() - restore EFI variables from buffer
> > >*
> > > + * Only if @safe is set secure boot related variables will be restored.
> > > + *
> > >* @buf:buffer
> > > + * @safe:restoring from tamper-resistant storage
> > >* Return: status code
> > >*/
> > > -efi_status_t efi_var_restore(struct efi_var_file *buf);
> > > +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
> > >
> > > /**
> > >* efi_var_from_file() - read variables from file
> > > diff --git a/lib/efi_loader/efi_var_common.c
> > > b/lib/efi_loader/efi_var_common.c
> > > index cf7afecd60..b0c5b672c5 100644
> > > --- a/lib/efi_loader/efi_var_common.c
> > > +++ b/lib/efi_loader/efi_var_common.c
> > > @@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[]
> > > = {
> > > {u"KEK", _global_variable_guid, EFI_AUTH_VAR_KEK},
> > > {u"db", _guid_image_security_database, EFI_AUTH_VAR_DB},
> > > {u"dbx", _guid_image_security_database, EFI_AUTH_VAR_DBX},
> > > - /* not used yet
> > > {u"dbt", _guid_image_security_database, EFI_AUTH_VAR_DBT},
> > > {u"dbr", _guid_image_security_database, EFI_AUTH_VAR_DBR},
> > > - */
> > > };
> > >
> > > static bool efi_secure_boot;
> > > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> > > index de076b8cbc..c7c6805ed0 100644
> > > --- a/lib/efi_loader/efi_var_file.c
> > > +++ b/lib/efi_loader/efi_var_file.c
> > > @@ -148,9 +148,10 @@ error:
> > > #endif
> > > }
> > >
> > > -efi_status_t efi_var_restore(struct efi_var_file *buf)
> > > +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
> > > {
> > > struct efi_var_entry *var, *last_var;
> > > + u16 *data;
> > > efi_status_t ret;
> > >
> > > if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
> > > @@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file
> > > *buf)
> > > return EFI_INVALID_PARAMETER;
> > > }
> > >
> > > - var = buf->var;
> > > last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
> > > - while (var < last_var) {
> > > - u16 *data = var->name + u16_strlen(var->name) + 1;
> > > -
> > > - if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
> > > - ret = efi_var_mem_ins(var->name, >guid, var->attr,
> > > - var->length, data, 0, NULL,
> > > - var->time);
> > > - if (ret != EFI_SUCCESS)
> > > - log_err("Failed to set EFI variable %ls\n",
> > > - var->name);
> > > - }
> > > - var = (struct efi_var_entry *)
> > > - ALIGN((uintptr_t)data + var->length, 8);
> > > + for (var = buf->var; var < last_var;
> > > + var = (struct efi_var_entry *)
> > > +ALIGN((uintptr_t)data + var->length, 8)) {
> > > +
> > > + data = var->name + u16_strlen(var->name) + 1;
> > > +
> > > + /*
> > > + * Secure boot related and non-volatile variables shall only be
> > > + *
Re: [PATCH v2 6/6] efi_loader: always initialize the secure boot state
On Fri, Aug 27, 2021 at 06:34:30AM +0200, Heinrich Schuchardt wrote:
> On 8/27/21 5:53 AM, AKASHI Takahiro wrote:
> > On Thu, Aug 26, 2021 at 03:48:05PM +0200, Heinrich Schuchardt wrote:
> > > Even if we cannot read the variable store from disk we still need to
> > > initialize the secure boot state.
> > >
> > > Don't continue to boot if the variable preseed is invalid as this
> > > indicates
> > > that the variable store has been tampered.
> > >
> > > Signed-off-by: Heinrich Schuchardt
> > > ---
> > > v2:
> > > no change
> > > ---
> > > lib/efi_loader/efi_variable.c | 12
> > > 1 file changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > > index 80996d0f47..6d92229e2a 100644
> > > --- a/lib/efi_loader/efi_variable.c
> > > +++ b/lib/efi_loader/efi_variable.c
> > > @@ -427,13 +427,17 @@ efi_status_t efi_init_variables(void)
> > > if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
> > > ret = efi_var_restore((struct efi_var_file *)
> > > __efi_var_file_begin, true);
> > > - if (ret != EFI_SUCCESS)
> > > + if (ret != EFI_SUCCESS) {
> > > log_err("Invalid EFI variable seed\n");
> > > + return ret;
> > > + }
> > > }
> > > -
> > > - ret = efi_var_from_file();
> > > + ret = efi_init_secure_state();
> > > if (ret != EFI_SUCCESS)
> > > return ret;
> > >
> > > - return efi_init_secure_state();
> > > + /* Don't stop booting if variable store is not available */
> > > + efi_var_from_file();
> >
> > I think we have to think about two different cases:
> > 1) there is no "variable store" file available.
> > 2) it does exists, but reading from it (efi_var_restore()) failed
> >
> > For (2), we should return with an error as in the case of
> > CONFIG_EFI_VARIABLES_PRESEED.
> > Otherwise, the behavior is inconsistent.
>
> The preseed store is used for secure boot related variables.
Where does this restriction come from?
Kconfig says:
Include a file with the initial values for non-volatile UEFI variables
into the U-Boot binary. If this configuration option is set, changes
to authentication related variables (PK, KEK, db, dbx) are not
allowed.
# oops: I didn't notice the last sentence.
# but anyhow, it seems too restrictive.
> If this
> store is inconsistent, failing is required to ensure secure boot.
As I said, a file-based variable configuration cannot work
as a secure platform any way.
Why do we need this kind of restriction?
-Takahiro Akashi
> With my patches applied the file store can not contain any secure boot
> related variables but it may contain boot options.
>
> Your suggestion could mean that if the file store is corrupted the board
> is bricked.
>
> If the boot options cannot be read either because the file does not
> exist or because the file is corrupt, I still want the user to have a
> chance to load shim, GRUB, or the kernel and boot via the bootefi command.
>
> I cannot see any inconsistency here.
>
> Best regards
>
> Heinrich
Re: [PATCH v2 3/6] efi_loader: don't load signature database from file
On 8/27/21 6:12 AM, AKASHI Takahiro wrote:
On Thu, Aug 26, 2021 at 03:48:02PM +0200, Heinrich Schuchardt wrote:
The UEFI specification requires that the signature database may only be
stored in tamper-resistant storage. So these variable may not be read
from an unsigned file.
I don't have a strong opinion here, but it seems to be too restrictive.
Nobody expects that file-based variable implementation is *safe*.
Leave it as it is so that people can easily experiment secure boot.
If the prior boot stage checks the integrity of the U-Boot binary, the
file based implementation becomes 'safe' with this patch.
Users can still experiment with secure boot by setting the secure boot
variables via the efidebug command.
I cannot see a use case for having the secure boot data base on an
insecure medium.
Best regards
Heinrich
-Takahiro Akashi
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
include/efi_variable.h | 5 +++-
lib/efi_loader/efi_var_common.c | 2 --
lib/efi_loader/efi_var_file.c | 41 -
lib/efi_loader/efi_variable.c | 2 +-
4 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h
index 4623a64142..2d97655e1f 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct
efi_var_file **bufp, loff_t *
/**
* efi_var_restore() - restore EFI variables from buffer
*
+ * Only if @safe is set secure boot related variables will be restored.
+ *
* @buf: buffer
+ * @safe: restoring from tamper-resistant storage
* Return:status code
*/
-efi_status_t efi_var_restore(struct efi_var_file *buf);
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
/**
* efi_var_from_file() - read variables from file
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index cf7afecd60..b0c5b672c5 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[] = {
{u"KEK", _global_variable_guid, EFI_AUTH_VAR_KEK},
{u"db", _guid_image_security_database, EFI_AUTH_VAR_DB},
{u"dbx", _guid_image_security_database, EFI_AUTH_VAR_DBX},
- /* not used yet
{u"dbt", _guid_image_security_database, EFI_AUTH_VAR_DBT},
{u"dbr", _guid_image_security_database, EFI_AUTH_VAR_DBR},
- */
};
static bool efi_secure_boot;
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index de076b8cbc..c7c6805ed0 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -148,9 +148,10 @@ error:
#endif
}
-efi_status_t efi_var_restore(struct efi_var_file *buf)
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
{
struct efi_var_entry *var, *last_var;
+ u16 *data;
efi_status_t ret;
if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
@@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file *buf)
return EFI_INVALID_PARAMETER;
}
- var = buf->var;
last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
- while (var < last_var) {
- u16 *data = var->name + u16_strlen(var->name) + 1;
-
- if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
- ret = efi_var_mem_ins(var->name, >guid, var->attr,
- var->length, data, 0, NULL,
- var->time);
- if (ret != EFI_SUCCESS)
- log_err("Failed to set EFI variable %ls\n",
- var->name);
- }
- var = (struct efi_var_entry *)
- ALIGN((uintptr_t)data + var->length, 8);
+ for (var = buf->var; var < last_var;
+var = (struct efi_var_entry *)
+ ALIGN((uintptr_t)data + var->length, 8)) {
+
+ data = var->name + u16_strlen(var->name) + 1;
+
+ /*
+* Secure boot related and non-volatile variables shall only be
+* restored from U-Boot's preseed.
+*/
+ if (!safe &&
+ (efi_auth_var_get_type(var->name, >guid) !=
+EFI_AUTH_VAR_NONE ||
+!(var->attr & EFI_VARIABLE_NON_VOLATILE)))
+ continue;
+ if (!var->length)
+ continue;
+ ret = efi_var_mem_ins(var->name, >guid, var->attr,
+ var->length, data, 0, NULL,
+ var->time);
+ if (ret != EFI_SUCCESS)
+ log_err("Failed to set EFI variable %ls\n", var->name);
}
return
Re: [PATCH v2 6/6] efi_loader: always initialize the secure boot state
On 8/27/21 5:53 AM, AKASHI Takahiro wrote:
On Thu, Aug 26, 2021 at 03:48:05PM +0200, Heinrich Schuchardt wrote:
Even if we cannot read the variable store from disk we still need to
initialize the secure boot state.
Don't continue to boot if the variable preseed is invalid as this indicates
that the variable store has been tampered.
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
lib/efi_loader/efi_variable.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 80996d0f47..6d92229e2a 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -427,13 +427,17 @@ efi_status_t efi_init_variables(void)
if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
ret = efi_var_restore((struct efi_var_file *)
__efi_var_file_begin, true);
- if (ret != EFI_SUCCESS)
+ if (ret != EFI_SUCCESS) {
log_err("Invalid EFI variable seed\n");
+ return ret;
+ }
}
-
- ret = efi_var_from_file();
+ ret = efi_init_secure_state();
if (ret != EFI_SUCCESS)
return ret;
- return efi_init_secure_state();
+ /* Don't stop booting if variable store is not available */
+ efi_var_from_file();
I think we have to think about two different cases:
1) there is no "variable store" file available.
2) it does exists, but reading from it (efi_var_restore()) failed
For (2), we should return with an error as in the case of
CONFIG_EFI_VARIABLES_PRESEED.
Otherwise, the behavior is inconsistent.
The preseed store is used for secure boot related variables. If this
store is inconsistent, failing is required to ensure secure boot.
With my patches applied the file store can not contain any secure boot
related variables but it may contain boot options.
Your suggestion could mean that if the file store is corrupted the board
is bricked.
If the boot options cannot be read either because the file does not
exist or because the file is corrupt, I still want the user to have a
chance to load shim, GRUB, or the kernel and boot via the bootefi command.
I cannot see any inconsistency here.
Best regards
Heinrich
Re: [PATCH v2 3/6] efi_loader: don't load signature database from file
On Thu, Aug 26, 2021 at 03:48:02PM +0200, Heinrich Schuchardt wrote:
> The UEFI specification requires that the signature database may only be
> stored in tamper-resistant storage. So these variable may not be read
> from an unsigned file.
I don't have a strong opinion here, but it seems to be too restrictive.
Nobody expects that file-based variable implementation is *safe*.
Leave it as it is so that people can easily experiment secure boot.
-Takahiro Akashi
> Signed-off-by: Heinrich Schuchardt
> ---
> v2:
> no change
> ---
> include/efi_variable.h | 5 +++-
> lib/efi_loader/efi_var_common.c | 2 --
> lib/efi_loader/efi_var_file.c | 41 -
> lib/efi_loader/efi_variable.c | 2 +-
> 4 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/include/efi_variable.h b/include/efi_variable.h
> index 4623a64142..2d97655e1f 100644
> --- a/include/efi_variable.h
> +++ b/include/efi_variable.h
> @@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct
> efi_var_file **bufp, loff_t *
> /**
> * efi_var_restore() - restore EFI variables from buffer
> *
> + * Only if @safe is set secure boot related variables will be restored.
> + *
> * @buf: buffer
> + * @safe:restoring from tamper-resistant storage
> * Return: status code
> */
> -efi_status_t efi_var_restore(struct efi_var_file *buf);
> +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
>
> /**
> * efi_var_from_file() - read variables from file
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> index cf7afecd60..b0c5b672c5 100644
> --- a/lib/efi_loader/efi_var_common.c
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[] = {
> {u"KEK", _global_variable_guid, EFI_AUTH_VAR_KEK},
> {u"db", _guid_image_security_database, EFI_AUTH_VAR_DB},
> {u"dbx", _guid_image_security_database, EFI_AUTH_VAR_DBX},
> - /* not used yet
> {u"dbt", _guid_image_security_database, EFI_AUTH_VAR_DBT},
> {u"dbr", _guid_image_security_database, EFI_AUTH_VAR_DBR},
> - */
> };
>
> static bool efi_secure_boot;
> diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> index de076b8cbc..c7c6805ed0 100644
> --- a/lib/efi_loader/efi_var_file.c
> +++ b/lib/efi_loader/efi_var_file.c
> @@ -148,9 +148,10 @@ error:
> #endif
> }
>
> -efi_status_t efi_var_restore(struct efi_var_file *buf)
> +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
> {
> struct efi_var_entry *var, *last_var;
> + u16 *data;
> efi_status_t ret;
>
> if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
> @@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file *buf)
> return EFI_INVALID_PARAMETER;
> }
>
> - var = buf->var;
> last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
> - while (var < last_var) {
> - u16 *data = var->name + u16_strlen(var->name) + 1;
> -
> - if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
> - ret = efi_var_mem_ins(var->name, >guid, var->attr,
> - var->length, data, 0, NULL,
> - var->time);
> - if (ret != EFI_SUCCESS)
> - log_err("Failed to set EFI variable %ls\n",
> - var->name);
> - }
> - var = (struct efi_var_entry *)
> - ALIGN((uintptr_t)data + var->length, 8);
> + for (var = buf->var; var < last_var;
> + var = (struct efi_var_entry *)
> +ALIGN((uintptr_t)data + var->length, 8)) {
> +
> + data = var->name + u16_strlen(var->name) + 1;
> +
> + /*
> + * Secure boot related and non-volatile variables shall only be
> + * restored from U-Boot's preseed.
> + */
> + if (!safe &&
> + (efi_auth_var_get_type(var->name, >guid) !=
> + EFI_AUTH_VAR_NONE ||
> + !(var->attr & EFI_VARIABLE_NON_VOLATILE)))
> + continue;
> + if (!var->length)
> + continue;
> + ret = efi_var_mem_ins(var->name, >guid, var->attr,
> + var->length, data, 0, NULL,
> + var->time);
> + if (ret != EFI_SUCCESS)
> + log_err("Failed to set EFI variable %ls\n", var->name);
> }
> return EFI_SUCCESS;
> }
> @@ -213,7 +222,7 @@ efi_status_t efi_var_from_file(void)
> log_err("Failed to load EFI variables\n");
> goto error;
> }
> - if (buf->length != len || efi_var_restore(buf) != EFI_SUCCESS)
> + if (buf->length != len || efi_var_restore(buf,
Re: [PATCH v2 5/6] efi_loader: writing AuditMode, DeployedMode
On 8/27/21 5:05 AM, AKASHI Takahiro wrote:
Heinrich,
On Thu, Aug 26, 2021 at 03:48:04PM +0200, Heinrich Schuchardt wrote:
Writing variables AuditMode or Deployed Mode must update the secure boot
state.
Signed-off-by: Heinrich Schuchardt
---
v2:
correct variable name in lib/efi_loader/efi_variable_tee.c
---
include/efi_variable.h| 1 +
lib/efi_loader/efi_var_common.c | 2 ++
lib/efi_loader/efi_variable.c | 6 +++---
lib/efi_loader/efi_variable_tee.c | 4 +++-
4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h
index 2d97655e1f..0440d356bc 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -12,6 +12,7 @@
enum efi_auth_var_type {
EFI_AUTH_VAR_NONE = 0,
+ EFI_AUTH_MODE,
EFI_AUTH_VAR_PK,
EFI_AUTH_VAR_KEK,
EFI_AUTH_VAR_DB,
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 63ad6fea9e..6fabcfe72c 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -34,6 +34,8 @@ static const struct efi_auth_var_name_type name_type[] = {
{u"dbx", _guid_image_security_database, EFI_AUTH_VAR_DBX},
{u"dbt", _guid_image_security_database, EFI_AUTH_VAR_DBT},
{u"dbr", _guid_image_security_database, EFI_AUTH_VAR_DBR},
+ {u"AuditMode", _global_variable_guid, EFI_AUTH_MODE},
+ {u"DeployedMode", _global_variable_guid, EFI_AUTH_MODE},
};
static bool efi_secure_boot;
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index a7d305ffbc..80996d0f47 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -247,7 +247,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
return EFI_WRITE_PROTECTED;
if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
- if (var_type != EFI_AUTH_VAR_NONE)
+ if (var_type >= EFI_AUTH_VAR_PK)
This change is irrelevant to the purpose of this commit.
Thank you for reviewing the series.
EFI_AUTH_MODE is needed in the implementation of this patch and requires
this change. But I can split the patch in two.
return EFI_WRITE_PROTECTED;
}
@@ -268,7 +268,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
return EFI_NOT_FOUND;
}
- if (var_type != EFI_AUTH_VAR_NONE) {
+ if (var_type >= EFI_AUTH_VAR_PK) {
/* authentication is mandatory */
if (!(attributes &
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
@@ -328,7 +328,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
if (ret != EFI_SUCCESS)
return ret;
- if (var_type == EFI_AUTH_VAR_PK)
+ if (var_type == EFI_AUTH_VAR_PK || var_type == EFI_AUTH_MODE)
ret = efi_init_secure_state();
As I said, calling efi_init_secure_state() is not a good idea.
The scheme that I have in mind:
* if some event takes place, then trigger the transition.
* efi_transfer_secure_state() handles/take actions for the transition.
Looking at "Figure 32-4 Secure Boot Modes", there are a couple of events
defined:
1) Enroll PKpub
2) Platform Specific PKpub Clear/Delete PKpub
3) Audit := 1
4) DeployedMode := 1
5) Platform Specific DeployedMode Clear
(Please note that "enroll/platform specific" operations should end up
modifying a relevant UEFI variable, any way.)
So, in the case above, we should do like this:
if ("PK" is added/modified)
if (SetupMode)
efi_transfer_secure_state(UserMode)
else (AuditMode)
efi_transfer_secure_state(DeployedMode)
else if ("AuditMode" is set)
if (SetupMode || UserMode)
efi_transfer_secure_state(AuditMode)
else if
and so on
Here we are in efi_set_variable_int(). efi_transfer_secure_state()
itself calls efi_set_variable_int() repeatedly.
Hence we need a way for a user to call SetVariable() with the side
effects you described above and a way to alter the state variables
without side effects.
There are different ways to implement this:
1) As we are on a single threaded system we can use a static
state variable. This is the approach in patch 1.
2) We can add a parameter to efi_set_variable_int() indicating that
the variable change shall not have side effects.
3) We can carve out a function for setting a variable without side
effects.
We have two implementations of efi_set_variable_int():
* One for file based variables in lib/efi_loader/efi_variable.c.
* Another for StMM based variables in lib/efi_loader/efi_variable_tee.c.
Whatever we do must work for both implementations of variables.
lib/efi_loader/efi_variable_tee.c has two calls to
efi_init_secure_state() currently matching the calls in
Re: [PATCH 0/3] common: Add fdt network helper
Hi Simon, On Tue, Aug 17, 2021 at 9:09 AM Simon Glass wrote: > > Hi Tony, > > On Sun, 15 Aug 2021 at 15:28, Tony Dinh wrote: > > > > Hi Simon, > > > > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass wrote: > > > > > > Hi Tony, > > > > > > On Thu, 5 Aug 2021 at 22:49, Tony Dinh wrote: > > > > > > > > > > > > At the moment, there is no common fdt helper function specific to > > > > decoding network related > > > > information from FDTs. This new helper functional group fdt_support_net > > > > is intended to be used > > > > by board-specific code within U-Boot for various network related chores. > > > > > > > > In this patch, create the 1st function fdt_get_phy_addr to parse the > > > > device tree to find > > > > the PHY addess of a specific ethernet device. > > > > > > > > > > > > Tony Dinh (3): > > > > Add fdt network helper header file > > > > Add fdt network helper functions > > > > Add fdt network helper to Makefile > > > > > > > > common/Makefile | 2 +- > > > > common/fdt_support_net.c | 46 +++ > > > > include/fdt_support_net.h | 39 + > > > > 3 files changed, 86 insertions(+), 1 deletion(-) > > > > create mode 100644 common/fdt_support_net.c > > > > create mode 100644 include/fdt_support_net.h > > > > > > Can this use livetre and also have some tests? > > > > I have not enabled livetree for any of the boards I have. So I just > > modeled this using the existing ./common/fdt_support.c! > > > > I do agree we should start using livetree in fdt helpers, if I > > understood it correctly, it should work for both flattree and > > OK good, yes that's right. > > > livetree. Perhaps we could have another patch series after this? I am > > preparing another Kirkwood board support patch that I could hold off > > submitting and enable livetree to use that as a vehicle for testing. > > I think it is better to use livetree in this patch. For testing, you > can use sandbox for testing (see for example test/dm/ofnode.c) > > Regards, > Simon It seems it is too time consuming to implement this using livetree calls (with my limited understanding about livetree). I spent a few hours reading ./include/dm/read.h and ./include/dm/ofnode.h, and it is not apparent to me which functions to use. I see that we have eth_get_dev_by_name(), that's a start. Do you have any objection if I submit this function as a patch to ./common/fdt_support.c? fdt_support.c file is all flatree implementation. And by the way, this new function fdt_get_phy_addr() has been tested with several Kirkwood boards that I have been converting to DM Ethernet. When the time comes that it's mandatory to convert all to livetree calls, I'll be glad to help. Thanks, Tony
Re: [PATCH v2 0/6] efi_loader: fix secure boot mode transitions
On Thu, Aug 26, 2021 at 03:47:59PM +0200, Heinrich Schuchardt wrote: > The UEFI specification 2.9 defines the different modes that secure boot may > be in. > > The patch series adds support for the "Deployed Mode" and the "Setup Mode". This sentence seems to be wrong, or at least inaccurate. "Setup Mode" has been supported from the beginning when I implemented secure boot. In other word, I implemented only the transition between "Setup Mode" and "User Mode" only. -Takahiro Akashi > Furthermore the secure boot signature database must only be loaded from > tamper-resistant storage. So we must not load it from ubootefi.var on the > EFI system partition but only from the preseed variables store or via the > OP-TEE driver for the eMMC replay protected memory partition. > > v2: > correct variable name in lib/efi_loader/efi_variable_tee.c > > Heinrich Schuchardt (6): > efi_loader: stop recursion in efi_init_secure_state > efi_loader: correct determination of secure boot state > efi_loader: don't load signature database from file > efi_loader: correct secure boot state transition > efi_loader: writing AuditMode, DeployedMode > efi_loader: always initialize the secure boot state > > include/efi_variable.h| 6 ++- > lib/efi_loader/efi_var_common.c | 66 +++ > lib/efi_loader/efi_var_file.c | 41 +++ > lib/efi_loader/efi_variable.c | 20 ++ > lib/efi_loader/efi_variable_tee.c | 4 +- > 5 files changed, 95 insertions(+), 42 deletions(-) > > -- > 2.30.2 >
Re: Usage of device-tree for blobs
Hi Mark, On Thu, 26 Aug 2021 at 14:27, Mark Kettenis wrote: > > > From: Simon Glass > > Date: Thu, 26 Aug 2021 14:00:12 -0600 > > > > Hi Mark, > > > > On Thu, 26 Aug 2021 at 06:55, Mark Kettenis wrote: > > > > > > > From: Simon Glass > > > > Date: Wed, 25 Aug 2021 21:15:00 -0600 > > > > > > > > Hi Heinrich, > > > > > > > > On Wed, 25 Aug 2021 at 02:05, Heinrich Schuchardt > > > > wrote: > > > > > > > > > > Hello Simon, > > > > > > > > > > some boards like qemu-riscv64_defconfig do not use any device-tree at > > > > > build time. A device-tree is only supplied at runtime by the prior > > > > > boot > > > > > stage (CONFIG_OF_PRIOR_STAGE=y). > > > > > > > > > > In doc/develop/devicetree/intro.rst you suggest to put binary blobs > > > > > into > > > > > the device-tree. > > > > > > > > > > Could you, please, update the documentation to explain how adding > > > > > blobs > > > > > to the device-tree works in the different scenarios depending on the > > > > > values of: > > > > > > > > > > CONFIG_OF_EMBED > > > > > CONFIG_OF_SEPARATE > > > > > CONFIG_OF_BOARD > > > > > CONFIG_OF_HOSTFILE > > > > > CONFIG_OF_PRIOR_STAGE > > > > > > > > > > It would be especially important to understand how one can develop a > > > > > board independent feature which works for all of these settings. > > > > > > > > OK I will take a crack at this tomorrow. > > > > > > > > But I think there is a disconnect here, since the only options that > > > > matter within U-Boot are OF_SEPARATE and OF_HOSTFILE, which both use a > > > > u-boot.dtb file. There is nothing tricky here. > > > > > > > > The OF_BOARD business is for when the board does special things. > > > > Presumably signing will do special things too. We cannot really know > > > > what those things are because the board as 'opted out' of the standard > > > > options. > > > > > > > > > > > > > > Please, describe CONFIG_OF_PRIOR_STAGE in > > > > > doc/develop/devicetree/control.rst. > > > > > > > > So I'm not allowed to delete that option? :-) It seems to me to be > > > > extremely sparse on documentation. We need an explanation of what it > > > > means and what effect it has on the build system, U-Boot and some > > > > discussion of how qemu works. It seems to have been added as part of > > > > an unrelated broadcom commit. The tags were incorrect so I doubt > > > > anyone noticed it. Since then it has apparently proved useful > > > > elsewhere, but no one has added more docs. > > > > > > > > So perhaps you can help me with my doc by explaining how > > > > OF_PRIOR_STAGE works in qmeu, why the DT in U-Boot cannot be used, and > > > > which project actually hosts the DT that qemu provides? Armed with > > > > that information, I might be able to make sense of it all. > > > > > > Well, QEMU allows configuration of (emulated/paravirtualised) devices > > > from the command line. So it is pretty much impossible for U-Boot to > > > provide a DT that matches that configuration without adding a lot of > > > code the dynamically build it from some sort of configuration > > > specification provided by QEMU to U-Boot. But at that point QEMU > > > might as well provide the DT itself, don't you agree? > > > > Don't these devices have a DT in U-Boot? Is qemu enabling them (status > > = "okay") or actually inventing them?! > > I actually invents them. See the mail I just sent for a reference to > the code. OK I see. > > > > Anyway, replying to this thread since for the Apple M1 support that > > > I'm working on we're in a somewhat similar situation. The Asahi Linux > > > project has implemented m1n1 as a bootloader and plans to use U-Boot > > > as a "payload" to implement booting a standard Linux distribution (and > > > other OSes). In this scenario m1n1 actually provides the DT since it > > > parses Apple's version of the DT (which isn't in the standard DT > > > format) and adds/updates certain properties that depend on the actual > > > hardware it is running on. > > > > Yes I see. Still I'd like a basic DT in U-Boot, even if just for > > discoverability. > > Discoverability of what? Devices that are available / used on the board. > > > > For this code I use CONFIG_OF_BOARD and implement > > > board_fdt_blob_setup() to simple return a pointer to the DT that m1n1 > > > set up. But that seems to be exactly what CONFIG_OF_PRIOR_STAGE > > > does... > > > > Right, and neither is really documented to the level that people can > > understand the purpose. They just come across as hacks to me. > > Well, I think U-Boot by its very nature is a giant hack ;). I'm not even sure where to go with that one... > > > I'd like to see an API for passing a info (including DT) to U-Boot. I > > think riscv has it? I suggest we use a register that points to a > > bloblist. > > Well, having a generic API will be hard. It will depend on the > low-level firmware. On RISC-V the SBI specification defines such an > API. For the Apple M1 I just use the existing code that makes U-Boot > look like a
Re: Usage of device-tree for blobs
Hi Mark, On Thu, 26 Aug 2021 at 14:18, Mark Kettenis wrote: > > > From: Simon Glass > > Date: Thu, 26 Aug 2021 13:54:49 -0600 > > > > Hi Heinrich, > > > > > > On Thu, 26 Aug 2021 at 01:10, Heinrich Schuchardt > > wrote: > > > > > > On 8/26/21 5:15 AM, Simon Glass wrote: > > > > Hi Heinrich, > > > > > > > > On Wed, 25 Aug 2021 at 02:05, Heinrich Schuchardt > > > > wrote: > > > >> > > > >> Hello Simon, > > > >> > > > >> some boards like qemu-riscv64_defconfig do not use any device-tree at > > > >> build time. A device-tree is only supplied at runtime by the prior boot > > > >> stage (CONFIG_OF_PRIOR_STAGE=y). > > > >> > > > >> In doc/develop/devicetree/intro.rst you suggest to put binary blobs > > > >> into > > > >> the device-tree. > > > >> > > > >> Could you, please, update the documentation to explain how adding blobs > > > >> to the device-tree works in the different scenarios depending on the > > > >> values of: > > > >> > > > >> CONFIG_OF_EMBED > > > >> CONFIG_OF_SEPARATE > > > >> CONFIG_OF_BOARD > > > >> CONFIG_OF_HOSTFILE > > > >> CONFIG_OF_PRIOR_STAGE > > > >> > > > >> It would be especially important to understand how one can develop a > > > >> board independent feature which works for all of these settings. > > > > > > > > OK I will take a crack at this tomorrow. > > > > > > > > But I think there is a disconnect here, since the only options that > > > > matter within U-Boot are OF_SEPARATE and OF_HOSTFILE, which both use a > > > > u-boot.dtb file. There is nothing tricky here. > > > > > > The following boards use OF_PRIO_STAGE: > > > > > > * QEMU > > > * bcm7260_defconfig > > > * bcm7445_defconfig > > > * ae350_rv32_defconfig > > > * ae350_rv32_spl_defconfig > > > * ae350_rv64_defconfig > > > * ae350_rv64_spl_defconfig > > > > Most of these seem OK as they have an in-tree DT. But the arm and > > riscv qemus and the bcm builds do not: > > > > bcm7260_defconfig > > bcm7445_defconfig > > configs/qemu_arm64_defconfig > > configs/qemu_arm_defconfig > > configs/qemu-ppce500_defconfig > > configs/qemu-riscv32_defconfig > > configs/qemu-riscv32_smode_defconfig > > configs/qemu-riscv64_defconfig > > configs/qemu-riscv64_smode_defconfig > > > > I think we are going to have to ban this. We are not really testing > > the build at all, and it presumably depends on the version of qemu > > that is used. It's OK to provide the DT to U-Boot as one flow, but not > > to completely drop it from the tree. > > So you want to have a DT in the U-Boot tree that is completely unused? Why would I want it to be unused? It is used by the U-Boot build, at least. Does this all work because the qemu support has been in there for ages and never changes? Otherwise I wonder about version compatibility. > > > Where is the qemu source code that creates these DTs? > > For ARM the code is in: > > https://github.com/qemu/qemu/blob/master/hw/arm/virt.c Gosh that's 3k LOC that I didn't even imagine existed! Thank you for the pointer. I don't see any mention in there about U-Boot. Are you sure that it is passed to U-Boot, or is it just passed to Linux? If U-Boot uses it, someone should make a note of that in the file. > > > > > The OF_BOARD business is for when the board does special things. > > > > Presumably signing will do special things too. We cannot really know > > > > what those things are because the board as 'opted out' of the standard > > > > options. > > > > > > > >> > > > >> Please, describe CONFIG_OF_PRIOR_STAGE in > > > >> doc/develop/devicetree/control.rst. > > > > > > > > So I'm not allowed to delete that option? :-) > > > > > > No. > > > > > > > It seems to me to be > > > > extremely sparse on documentation. We need an explanation of what it > > > > means and what effect it has on the build system, U-Boot and some > > > > discussion of how qemu works. It seems to have been added as part of > > > > an unrelated broadcom commit. The tags were incorrect so I doubt > > > > anyone noticed it. Since then it has apparently proved useful > > > > elsewhere, but no one has added more docs. > > > > > > > > So perhaps you can help me with my doc by explaining how > > > > OF_PRIOR_STAGE works in qmeu, why the DT in U-Boot cannot be used, and > > > > which project actually hosts the DT that qemu provides? Armed with > > > > that information, I might be able to make sense of it all. > > > > > > The DT provided by QEMU is not hosted anywhere. It is generated on the > > > fly by QEMU in dependence of the command line arguments that are used > > > for calling QEMU. The project is hosted at > > > > > > https://github.com/qemu/qemu. > > > > 404 on that. Do you have a link to the code that actually generates the DT? > > > > > > > > On RISC-V the address of the device-tree of the prior bootstage is > > > provided in register t0. > > > > > > On ARM platforms QEMU places the device-tree at 0x4000. > > > > > > QEMU is not the only platform where the prior boot stage supplies the > > > device-tree which is to be used for
Re: [PATCH v2 6/6] efi_loader: always initialize the secure boot state
On Thu, Aug 26, 2021 at 03:48:05PM +0200, Heinrich Schuchardt wrote:
> Even if we cannot read the variable store from disk we still need to
> initialize the secure boot state.
>
> Don't continue to boot if the variable preseed is invalid as this indicates
> that the variable store has been tampered.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> v2:
> no change
> ---
> lib/efi_loader/efi_variable.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 80996d0f47..6d92229e2a 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -427,13 +427,17 @@ efi_status_t efi_init_variables(void)
> if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
> ret = efi_var_restore((struct efi_var_file *)
> __efi_var_file_begin, true);
> - if (ret != EFI_SUCCESS)
> + if (ret != EFI_SUCCESS) {
> log_err("Invalid EFI variable seed\n");
> + return ret;
> + }
> }
> -
> - ret = efi_var_from_file();
> + ret = efi_init_secure_state();
> if (ret != EFI_SUCCESS)
> return ret;
>
> - return efi_init_secure_state();
> + /* Don't stop booting if variable store is not available */
> + efi_var_from_file();
I think we have to think about two different cases:
1) there is no "variable store" file available.
2) it does exists, but reading from it (efi_var_restore()) failed
For (2), we should return with an error as in the case of
CONFIG_EFI_VARIABLES_PRESEED.
Otherwise, the behavior is inconsistent.
- Takahiro Akashi
> +
> + return EFI_SUCCESS;
> }
> --
> 2.30.2
>
Re: [PATCH v2 5/6] efi_loader: writing AuditMode, DeployedMode
Heinrich,
On Thu, Aug 26, 2021 at 03:48:04PM +0200, Heinrich Schuchardt wrote:
> Writing variables AuditMode or Deployed Mode must update the secure boot
> state.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> v2:
> correct variable name in lib/efi_loader/efi_variable_tee.c
> ---
> include/efi_variable.h| 1 +
> lib/efi_loader/efi_var_common.c | 2 ++
> lib/efi_loader/efi_variable.c | 6 +++---
> lib/efi_loader/efi_variable_tee.c | 4 +++-
> 4 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/include/efi_variable.h b/include/efi_variable.h
> index 2d97655e1f..0440d356bc 100644
> --- a/include/efi_variable.h
> +++ b/include/efi_variable.h
> @@ -12,6 +12,7 @@
>
> enum efi_auth_var_type {
> EFI_AUTH_VAR_NONE = 0,
> + EFI_AUTH_MODE,
> EFI_AUTH_VAR_PK,
> EFI_AUTH_VAR_KEK,
> EFI_AUTH_VAR_DB,
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> index 63ad6fea9e..6fabcfe72c 100644
> --- a/lib/efi_loader/efi_var_common.c
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -34,6 +34,8 @@ static const struct efi_auth_var_name_type name_type[] = {
> {u"dbx", _guid_image_security_database, EFI_AUTH_VAR_DBX},
> {u"dbt", _guid_image_security_database, EFI_AUTH_VAR_DBT},
> {u"dbr", _guid_image_security_database, EFI_AUTH_VAR_DBR},
> + {u"AuditMode", _global_variable_guid, EFI_AUTH_MODE},
> + {u"DeployedMode", _global_variable_guid, EFI_AUTH_MODE},
> };
>
> static bool efi_secure_boot;
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index a7d305ffbc..80996d0f47 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -247,7 +247,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name,
> const efi_guid_t *vendor,
> return EFI_WRITE_PROTECTED;
>
> if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
> - if (var_type != EFI_AUTH_VAR_NONE)
> + if (var_type >= EFI_AUTH_VAR_PK)
This change is irrelevant to the purpose of this commit.
> return EFI_WRITE_PROTECTED;
> }
>
> @@ -268,7 +268,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name,
> const efi_guid_t *vendor,
> return EFI_NOT_FOUND;
> }
>
> - if (var_type != EFI_AUTH_VAR_NONE) {
> + if (var_type >= EFI_AUTH_VAR_PK) {
> /* authentication is mandatory */
> if (!(attributes &
> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
> @@ -328,7 +328,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name,
> const efi_guid_t *vendor,
> if (ret != EFI_SUCCESS)
> return ret;
>
> - if (var_type == EFI_AUTH_VAR_PK)
> + if (var_type == EFI_AUTH_VAR_PK || var_type == EFI_AUTH_MODE)
> ret = efi_init_secure_state();
As I said, calling efi_init_secure_state() is not a good idea.
The scheme that I have in mind:
* if some event takes place, then trigger the transition.
* efi_transfer_secure_state() handles/take actions for the transition.
Looking at "Figure 32-4 Secure Boot Modes", there are a couple of events
defined:
1) Enroll PKpub
2) Platform Specific PKpub Clear/Delete PKpub
3) Audit := 1
4) DeployedMode := 1
5) Platform Specific DeployedMode Clear
(Please note that "enroll/platform specific" operations should end up
modifying a relevant UEFI variable, any way.)
So, in the case above, we should do like this:
if ("PK" is added/modified)
if (SetupMode)
efi_transfer_secure_state(UserMode)
else (AuditMode)
efi_transfer_secure_state(DeployedMode)
else if ("AuditMode" is set)
if (SetupMode || UserMode)
efi_transfer_secure_state(AuditMode)
else if
and so on
The logic is clear and the code directly renders what the figure 32-4 shows.
What's more, it will make it much easier for reviewers (and users)
to confirm the code is fully compliant with the specification
in terms of the "conditions" vs. resultant system status.
Then, each of the system's secure status can be always maintained
within efi_transfer_secure_state().
In addition, we will not have to have a hacky "lock" in
efi_init_secure_state().
Those are the reason why I want to stick to the scheme above.
-Takahiro Akashi
> else
> ret = EFI_SUCCESS;
> diff --git a/lib/efi_loader/efi_variable_tee.c
> b/lib/efi_loader/efi_variable_tee.c
> index 51920bcb51..a6d5752045 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -512,6 +512,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name,
> const efi_guid_t *vendor,
> efi_uintn_t payload_size;
> efi_uintn_t name_size;
> u8 *comm_buf = NULL;
> + enum efi_auth_var_type var_type;
> bool ro;
>
> if (!variable_name || variable_name[0] == 0 || !vendor) {
> @@ -590,7 +591,8 @@ efi_status_t
Re: [PATCH v2] board: ea: mx7ulp_com: move setting CONFIG_BOOTCOMMAND to defconfig
On Thu, Aug 26, 2021 at 02:04:24PM +0300, Oleksandr Suvorov wrote: > From: Ricardo Salveti > > Move setting CONFIG_BOOTCOMMAND to the mx7ulp_com_defconfig file. > It also allows replacing the default CONFIG_BOOTCOMMAND without > code modification. > > Signed-off-by: Ricardo Salveti > Signed-off-by: Oleksandr Suvorov Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 4/4] azure/gitlab: Add tests for SiFive Unleashed board
On Thu, Aug 26, 2021 at 11:33:35PM +0800, Bin Meng wrote: > This adds CI tests for SiFive Unleashed board. > > QEMU supports booting exact the same images as used on the real > hardware out of the box, that U-Boot SPL loads U-Boot proper > from either an SD card or the SPI NOR flash, hence we can easily > set up CI to cover these 2 boot flows of SiFive Unleashed board. > > With this, now we can have regression testing of mmc-spi-slot and > sifive spi drivers, as well as mmc and spi-nor subsystems. > > Signed-off-by: Bin Meng > Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 2/4] tools: docker: Build and install genimage
On Thu, Aug 26, 2021 at 11:33:33PM +0800, Bin Meng wrote: > genimage [1] is a tool to create flash/disk images. This is required > by some targets, e.g.: sifive_unleashed, to generate sdcard or spi-nor > images for real hardware, as well as U-Boot CI testing. > > [1] https://github.com/pengutronix/genimage > > Signed-off-by: Bin Meng > Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/4] tools: docker: Bump up QEMU version to 6.1.0
On Thu, Aug 26, 2021 at 11:33:32PM +0800, Bin Meng wrote: > At present U-Boot CI testing is still using QEMU 4.2.0 which is > pretty old. Let's bump up to QEMU 6.1.0. > > ninja-build is added as the prerequisite required by QEMU 6.1.0. > > Note there is a bug in QEMU 6.1.0 Xilinx Zynq UART emulation codes. > A quick fix [1] was posted on QEMU mailing list but it it too late > for 6.1.0 release. Let's manually apply the bug fix on top of the > v6.1.0 release tag at the time being. > > [1] > http://patchwork.ozlabs.org/project/qemu-devel/patch/20210823020813.25192-2-bmeng...@gmail.com/ > > Signed-off-by: Bin Meng > Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v<4>] arm: add initial support for the Phytium Pomelo Board
On Thu, Aug 26, 2021 at 04:31:36PM +0800, nicholas_zh...@outlook.com wrote: > From: weichangzheng > > This adds platform code and the device tree for the Phytium Pomelo Board. > The initial support comprises the UART and the PCIE. > > Signed-off-by: weichangzheng > Changes since v1: > updated to DT > Changes since v2: > Modify some explicit types and macro > Changes since v3: > Modify some SDRAM related macro definitions and distro_bootcmd [snip] > +#include > + > +#define CONFIG_EXTRA_ENV_SETTINGS\ > + "load_kernel=ext4load scsi 0:1 0x9010 uImage\0" \ > + "load_fdt=ext4load scsi 0:1 0x9500 ft-d2000.dtb\0"\ > + "run load_kernel\0"\ > + "run load_fdt\0"\ > + "bootm 0x9010 -:- 0x9500" OK, this is not how you do the distro_bootcmd framework. It is strongly encouraged to enable it, and set BOOT* like other platforms do, to enable it. You can look at commit d4f15ecd4724e633bc7f238b26f98efd5096e632 for an example of how to do this on a given platform. Thanks. -- Tom signature.asc Description: PGP signature
Re: [PATCH u-boot-marvell 00/29] kwboot higher baudrate
On Fri, Aug 27, 2021 at 1:39 PM Marek Behún wrote: > > On Fri, 27 Aug 2021 13:16:25 +1200 > Chris Packham wrote: > > > On Thu, Aug 26, 2021 at 1:46 AM Marek Behún wrote: > > > > > > Hello Stefan and others, > > > > > > this series adds support for booting Marvell platforms via UART (those > > > bootable with kwboot) at higher baudrates. > > > > > > Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than > > > 115200 Bd. > > > > > > The user can direct kwboot to use higher baudrate via the -B option. > > > (BTW this option was there before but did not work with the -b option.) > > > > > > Only the payload part of the KWB image is uploaded at this higher > > > baudrate. The header part is still uploaded at 115200 Bd, since the code > > > that changes baudrate is injected into header as a binary extension. > > > (The payload part makes up majority of the binary, though. On Turris > > > Omnia the payload currently makes ~87%.) > > > > > > The series also contains various other fixes, refactors and improvements > > > of the code, upon which the main change is done. > > > > > > Marek & Pali > > > > What tree/sha is this series based on. I've tried to apply them out of > > patchwork and it fails at "tools: kwbimage: Simplify iteration over > > version 1 optional headers" > > Applies to master and to u-boot-marvell, but you need the 6 > additional patches for kwbimage/kwboot that are on patchwork > but not yet applied to u-boot-marvell: > https://patchwork.ozlabs.org/project/uboot/list/?series=257577 > > https://patchwork.ozlabs.org/project/uboot/patch/20210817050320.11983-1-xypron.g...@gmx.de/ > > https://patchwork.ozlabs.org/project/uboot/patch/20210817051158.13283-1-xypron.g...@gmx.de/ > > Also some more fixes were yet needed, which I will sent in v2. > > To make it simpler for you I pushed v2 into > https://gitlab.nic.cz/turris/turris-omnia-uboot > branch > kwboot-baudrate-improvements > Thanks. I took it for a spin on the x530. In terms of regression testing the default behaviour is good. The higher speed settings weren't so good. I started with 3125000 and that doesn't get onto the 2nd part of the download, same for 1152000 and 400 (I stopped trying higher speeds at that point). Using 230400 and 460800 it does make it through the 2nd part of the download but when I go back on to the console it appears to be unresponsive. All of this could be down to the serial hardware in my system (the x530 uses a real RS232 interface not a TTL) and I have had problems with the MosChip USB-Serial adapter in my test PC in the past. I wouldn't reject this series based on me not being able to get it working, the important thing for me is the default behaviour at the standard baudrate which is good. One usability thing I'd like to see is retaining support for -t (I use that quite a lot when recovering a system). Ideally we'd still be able to drop into the console at 115200 once the download is complete. > Marek
Re: [PATCH v2 1/6] efi_loader: stop recursion in efi_init_secure_state
Heinrich,
On Thu, Aug 26, 2021 at 03:48:00PM +0200, Heinrich Schuchardt wrote:
> efi_init_secure_state() calls efi_transfer_secure_state() which may delete
> variable "PK" which will result in calling efi_init_secure_state() again.
I don't think it is a right thing to do. So I would say nak to this version.
When I first implemented those functions, I intended to call
efi_init_secure_state() only at the system initialization.
Later on, all the transitions should be managed by efi_transfer_secure_state()
as well as its callers.
Calling efi_init_secure_state() in efi_set_variable_int() is a bad idea.
(then you see 'recursion'.)
I will explain more in your patch#5.
-Takahiro Akashi
> Signed-off-by: Heinrich Schuchardt
> ---
> v2:
> no change
> ---
> lib/efi_loader/efi_var_common.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> index 3d92afe2eb..654ce81f9d 100644
> --- a/lib/efi_loader/efi_var_common.c
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -314,11 +314,15 @@ err:
>
> efi_status_t efi_init_secure_state(void)
> {
> + static bool lock;
> enum efi_secure_mode mode = EFI_MODE_SETUP;
> u8 efi_vendor_keys = 0;
> efi_uintn_t size = 0;
> efi_status_t ret;
>
> + if (lock)
> + return EFI_SUCCESS;
> +
> ret = efi_get_variable_int(L"PK", _global_variable_guid,
> NULL, , NULL, NULL);
> if (ret == EFI_BUFFER_TOO_SMALL) {
> @@ -326,7 +330,9 @@ efi_status_t efi_init_secure_state(void)
> mode = EFI_MODE_USER;
> }
>
> + lock = true;
> ret = efi_transfer_secure_state(mode);
> + lock = false;
> if (ret != EFI_SUCCESS)
> return ret;
>
> --
> 2.30.2
>
Re: [PATCH u-boot-marvell 00/29] kwboot higher baudrate
On Fri, 27 Aug 2021 13:16:25 +1200 Chris Packham wrote: > On Thu, Aug 26, 2021 at 1:46 AM Marek Behún wrote: > > > > Hello Stefan and others, > > > > this series adds support for booting Marvell platforms via UART (those > > bootable with kwboot) at higher baudrates. > > > > Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than > > 115200 Bd. > > > > The user can direct kwboot to use higher baudrate via the -B option. > > (BTW this option was there before but did not work with the -b option.) > > > > Only the payload part of the KWB image is uploaded at this higher > > baudrate. The header part is still uploaded at 115200 Bd, since the code > > that changes baudrate is injected into header as a binary extension. > > (The payload part makes up majority of the binary, though. On Turris > > Omnia the payload currently makes ~87%.) > > > > The series also contains various other fixes, refactors and improvements > > of the code, upon which the main change is done. > > > > Marek & Pali > > What tree/sha is this series based on. I've tried to apply them out of > patchwork and it fails at "tools: kwbimage: Simplify iteration over > version 1 optional headers" Applies to master and to u-boot-marvell, but you need the 6 additional patches for kwbimage/kwboot that are on patchwork but not yet applied to u-boot-marvell: https://patchwork.ozlabs.org/project/uboot/list/?series=257577 https://patchwork.ozlabs.org/project/uboot/patch/20210817050320.11983-1-xypron.g...@gmx.de/ https://patchwork.ozlabs.org/project/uboot/patch/20210817051158.13283-1-xypron.g...@gmx.de/ Also some more fixes were yet needed, which I will sent in v2. To make it simpler for you I pushed v2 into https://gitlab.nic.cz/turris/turris-omnia-uboot branch kwboot-baudrate-improvements Marek
Re: [PATCH u-boot-marvell 28/29] doc/kwboot.1: Update man page
On Thu, Aug 26, 2021 at 1:46 AM Marek Behún wrote: > > Update man page for the kwboot utility. > > Signed-off-by: Marek Behún > --- > doc/kwboot.1 | 58 ++-- > 1 file changed, 38 insertions(+), 20 deletions(-) > > diff --git a/doc/kwboot.1 b/doc/kwboot.1 > index 1e9ca268f7..cf113b8b27 100644 > --- a/doc/kwboot.1 > +++ b/doc/kwboot.1 > @@ -1,21 +1,22 @@ > -.TH KWBOOT 1 "2012-05-19" > +.TH KWBOOT 1 "2021-08-25" > > .SH NAME > -kwboot \- Boot Marvell Kirkwood SoCs over a serial link. > +kwboot \- Boot Marvell Kirkwood (and others 32-bit) SoCs over a serial link. > .SH SYNOPSIS > .B kwboot > .RB [ "-b \fIimage\fP" ] > -.RB [ "-p" ] > .RB [ "-t" ] I notice there's no mention of -d might be good to add it while we're here. > .RB [ "-B \fIbaudrate\fP" ] > .RB \fITTY\fP > .SH "DESCRIPTION" > > -The \fBmkimage\fP program boots boards based on Marvell's Kirkwood > -platform over their integrated UART. Boot image files will typically > +The \fBkwboot\fP program boots boards based on Marvell's 32-bit > +platforms including Orion, Kirkwood, Dove, Discovery, AXP, A37x, A38x > +and A39x over their integrated UART. Boot image files will typically > contain a second stage boot loader, such as U-Boot. The image file > must conform to Marvell's BootROM firmware image format > -(\fIkwbimage\fP), created using a tool such as \fBmkimage\fP. > +(\fIkwbimage v0\fP or \fIv1\fP), created using a tool such as > +\fBmkimage\fP. > > Following power-up or a system reset, system BootROM code polls the > UART for a brief period of time, sensing a handshake message which > @@ -36,25 +37,23 @@ by the second-stage loader. > Handshake; then upload file \fIimage\fP over \fITTY\fP. > > Note that for the encapsulated boot code to be executed, \fIimage\fP > -must be of type "UART boot" (0x69). Boot images of different types, > -such as backup images of vendor firmware downloaded from flash memory > -(type 0x8B), will not work (or not as expected). See \fB-p\fP for a > -workaround. > +must be of type "UART boot" (0x69). The \fBkwboot\fP program changes > +this type automatically, unless the \fIimage\fP is signed, in which > +case it cannot be changed. > > This mode writes handshake status and upload progress indication to > -stdout. > +stdout. It is possible that \fIimage\fP contains an optional binary > +code in it's header which may also print some output via UART (for > +example U-Boot SPL does this). In such a case, this output is also > +written to stdout after the header is sent. > > .TP > .BI "\-p" > -In combination with \fB-b\fP, patches the header in \fIimage\fP prior > -to upload, to "UART boot" type. > +Obsolete. Does nothing. > > -This option attempts on-the-fly conversion of some none-UART image > -types, such as images which were originally formatted to be stored in > -flash memory. > - > -Conversion is performed in memory. The contents of \fIimage\fP will > -not be altered. > +In the past, when this option was used, the program patched the header > +in the image prior upload, to "UART boot" type. This is now done by > +default. > > .TP > .BI "\-t" > @@ -69,7 +68,22 @@ after receiving 'ctrl-\\' followed by 'c' from console > input. > > .TP > .BI "\-B \fIbaudrate\fP" > -Adjust the baud rate on \fITTY\fP. Default rate is 115200. > +If used in combination with \fB-b\fP, inject into the image header > +code that changes baud rate to \fIbaudrate\fP after uploading image > +header, and code that changes the baud rate back to the default > +(115200 Bd) before executing payload, and also adjust the baud rate > +on \fITTY\fP correspondingly. This can make the upload significantly > +faster. > + > +If used in combination with \fB-t\fP, adjust the baud rate to > +\fIbaudrate\fP on \fITTY\fP before starting terminal. > + > +If both \fB-b\fP and \fB-t\fP are used, the baud rate is changed > +back to 115200 after the upload. > + > +Tested values for \fIbaudrate\fP for Armada 38x include: 115200, > +230400, 460800, 50, 576000, 921600, 100, 1152000, 150, > +200, 250, 3125000, 400 and 515. > > .SH "SEE ALSO" > .PP > @@ -82,3 +96,7 @@ Daniel Stodden > Luka Perkov > .br > David Purdy > +.br > +Pali Rohár > +.br > +Marek Behún > -- > 2.31.1 >
Re: [PATCH u-boot-marvell 00/29] kwboot higher baudrate
On Fri, Aug 27, 2021 at 1:16 PM Chris Packham wrote: > > On Thu, Aug 26, 2021 at 1:46 AM Marek Behún wrote: > > > > Hello Stefan and others, > > > > this series adds support for booting Marvell platforms via UART (those > > bootable with kwboot) at higher baudrates. > > > > Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than > > 115200 Bd. > > > > The user can direct kwboot to use higher baudrate via the -B option. > > (BTW this option was there before but did not work with the -b option.) > > > > Only the payload part of the KWB image is uploaded at this higher > > baudrate. The header part is still uploaded at 115200 Bd, since the code > > that changes baudrate is injected into header as a binary extension. > > (The payload part makes up majority of the binary, though. On Turris > > Omnia the payload currently makes ~87%.) > > > > The series also contains various other fixes, refactors and improvements > > of the code, upon which the main change is done. > > > > Marek & Pali > > What tree/sha is this series based on. I've tried to apply them out of > patchwork and it fails at "tools: kwbimage: Simplify iteration over > version 1 optional headers" > Ah it needs http://patchwork.ozlabs.org/project/uboot/list/?series=257577 > > > > Marek Behún (13): > > tools: kwbimage: Fix printf format warning > > tools: kwboot: Fix buffer overflow in kwboot_terminal() > > tools: kwboot: Make the quit sequence buffer const > > tools: kwboot: Refactor and fix writing buffer > > tools: kwboot: Fix comparison of integers with different size > > tools: kwboot: Use a function to check whether received byte is a > > Xmodem reply > > tools: kwboot: Print new line after SPL output > > tools: kwboot: Allow greater timeout when executing header code > > tools: kwbimage: Simplify iteration over version 1 optional headers > > tools: kwbimage: Refactor image_version() > > tools: kwbimage: Refactor kwbimage header size determination > > doc/kwboot.1: Update man page > > MAINTAINERS: Add entry for kwbimage / kwboot tools > > > > Pali Rohár (16): > > tools: kwboot: Print version information header > > tools: kwboot: Fix kwboot_xm_sendblock() function when > > kwboot_tty_recv() fails > > tools: kwboot: Fix return type of kwboot_xm_makeblock() function > > tools: kwboot: Fix printing progress > > tools: kwboot: Print newline on error when progress was not completed > > tools: kwboot: Split sending image into header and data stages > > tools: kwboot: Allow non-xmodem text output from BootROM only in a > > specific case > > tools: kwboot: Properly finish xmodem transfer > > tools: kwboot: Always call kwboot_img_patch_hdr() > > tools: kwboot: Don't patch image header if signed > > tools: kwboot: Patch source address in image header > > tools: kwboot: Patch destination address to DDR area for SPI image > > tools: kwboot: Round up header size to 128 B when patching > > tools: kwboot: Support higher baudrates when booting via UART > > tools: kwboot: Allow any baudrate on Linux > > tools: kwboot: Add Pali and Marek as authors > > > > MAINTAINERS | 10 + > > doc/kwboot.1 | 58 ++- > > tools/kwbimage.c | 127 ++--- > > tools/kwbimage.h | 93 +++- > > tools/kwboot.c| 1103 +++-- > > tools/termios_linux.h | 170 +++ > > 6 files changed, 1289 insertions(+), 272 deletions(-) > > create mode 100644 tools/termios_linux.h > > > > -- > > 2.31.1 > >
Re: [PATCH u-boot-marvell 00/29] kwboot higher baudrate
On Thu, Aug 26, 2021 at 1:46 AM Marek Behún wrote: > > Hello Stefan and others, > > this series adds support for booting Marvell platforms via UART (those > bootable with kwboot) at higher baudrates. > > Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than > 115200 Bd. > > The user can direct kwboot to use higher baudrate via the -B option. > (BTW this option was there before but did not work with the -b option.) > > Only the payload part of the KWB image is uploaded at this higher > baudrate. The header part is still uploaded at 115200 Bd, since the code > that changes baudrate is injected into header as a binary extension. > (The payload part makes up majority of the binary, though. On Turris > Omnia the payload currently makes ~87%.) > > The series also contains various other fixes, refactors and improvements > of the code, upon which the main change is done. > > Marek & Pali What tree/sha is this series based on. I've tried to apply them out of patchwork and it fails at "tools: kwbimage: Simplify iteration over version 1 optional headers" > > Marek Behún (13): > tools: kwbimage: Fix printf format warning > tools: kwboot: Fix buffer overflow in kwboot_terminal() > tools: kwboot: Make the quit sequence buffer const > tools: kwboot: Refactor and fix writing buffer > tools: kwboot: Fix comparison of integers with different size > tools: kwboot: Use a function to check whether received byte is a > Xmodem reply > tools: kwboot: Print new line after SPL output > tools: kwboot: Allow greater timeout when executing header code > tools: kwbimage: Simplify iteration over version 1 optional headers > tools: kwbimage: Refactor image_version() > tools: kwbimage: Refactor kwbimage header size determination > doc/kwboot.1: Update man page > MAINTAINERS: Add entry for kwbimage / kwboot tools > > Pali Rohár (16): > tools: kwboot: Print version information header > tools: kwboot: Fix kwboot_xm_sendblock() function when > kwboot_tty_recv() fails > tools: kwboot: Fix return type of kwboot_xm_makeblock() function > tools: kwboot: Fix printing progress > tools: kwboot: Print newline on error when progress was not completed > tools: kwboot: Split sending image into header and data stages > tools: kwboot: Allow non-xmodem text output from BootROM only in a > specific case > tools: kwboot: Properly finish xmodem transfer > tools: kwboot: Always call kwboot_img_patch_hdr() > tools: kwboot: Don't patch image header if signed > tools: kwboot: Patch source address in image header > tools: kwboot: Patch destination address to DDR area for SPI image > tools: kwboot: Round up header size to 128 B when patching > tools: kwboot: Support higher baudrates when booting via UART > tools: kwboot: Allow any baudrate on Linux > tools: kwboot: Add Pali and Marek as authors > > MAINTAINERS | 10 + > doc/kwboot.1 | 58 ++- > tools/kwbimage.c | 127 ++--- > tools/kwbimage.h | 93 +++- > tools/kwboot.c| 1103 +++-- > tools/termios_linux.h | 170 +++ > 6 files changed, 1289 insertions(+), 272 deletions(-) > create mode 100644 tools/termios_linux.h > > -- > 2.31.1 >
Re: [PATCH] Finish converting CONFIG_SYS_CACHELINE_SIZE to Kconfig
On 8/26/21 11:47 AM, Tom Rini wrote: We move the SYS_CACHE_SHIFT_N options from arch/arm/Kconfig to arch/Kconfig, and introduce SYS_CACHE_SHIFT_4 to provide a size of 16. Introduce select statements for other architectures based on current usage. For MIPS, we take the existing arch-specific symbol and migrate to the generic symbol. This lets us remove a little bit of otherwise unused code. Cc: Alexey Brodkin Cc: Anup Patel Cc: Atish Patra Cc: Bin Meng Cc: Daniel Schwierzeck Cc: Leo Cc: Palmer Dabbelt Cc: Paul Walmsley Cc: Rick Chen Cc: Sean Anderson Cc: Simon Glass Signed-off-by: Tom Rini --- I'm Cc'ing a bunch of RISC-V folks since that's where I'm least confident and just put it per-board for now. --- arch/Kconfig | 25 + arch/arc/include/asm/cache.h | 3 --- arch/arm/Kconfig | 15 --- arch/mips/Kconfig | 26 +++--- arch/mips/include/asm/cache.h | 12 +--- arch/mips/mach-bmips/Kconfig | 20 ++-- arch/mips/mach-mtmips/Kconfig | 4 ++-- arch/mips/mach-pic32/Kconfig | 2 +- arch/powerpc/cpu/mpc83xx/Kconfig | 6 ++ arch/powerpc/cpu/mpc85xx/Kconfig | 15 +++ arch/powerpc/cpu/mpc8xx/Kconfig| 2 ++ arch/powerpc/include/asm/cache.h | 7 --- arch/riscv/Kconfig | 2 ++ arch/sandbox/include/asm/cache.h | 1 - arch/x86/include/asm/cache.h | 7 +-- include/configs/M5208EVBE.h| 1 - include/configs/M5235EVB.h | 1 - include/configs/M5249EVB.h | 1 - include/configs/M5253DEMO.h| 1 - include/configs/M5272C3.h | 1 - include/configs/M5275EVB.h | 1 - include/configs/M5282EVB.h | 1 - include/configs/M53017EVB.h| 1 - include/configs/M5329EVB.h | 1 - include/configs/M5373EVB.h | 1 - include/configs/amcore.h | 1 - include/configs/astro_mcf5373l.h | 1 - include/configs/cobra5272.h| 1 - include/configs/eb_cpu5282.h | 1 - include/configs/mx7ulp_evk.h | 2 -- include/configs/rk3188_common.h| 2 -- include/configs/rk3368_common.h| 2 -- include/configs/sifive-unmatched.h | 2 -- include/configs/sipeed-maix.h | 1 - include/configs/stmark2.h | 1 - 35 files changed, 68 insertions(+), 103 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index b6f9e177b645..25f4a15b19f9 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -7,6 +7,27 @@ config HAVE_ARCH_IOREMAP config NEEDS_MANUAL_RELOC bool +config SYS_CACHE_SHIFT_4 + bool + +config SYS_CACHE_SHIFT_5 + bool + +config SYS_CACHE_SHIFT_6 + bool + +config SYS_CACHE_SHIFT_7 + bool + +config SYS_CACHELINE_SIZE + int + default 128 if SYS_CACHE_SHIFT_7 + default 64 if SYS_CACHE_SHIFT_6 + default 32 if SYS_CACHE_SHIFT_5 + default 16 if SYS_CACHE_SHIFT_4 + # Fall-back for MIPS + default 32 if MIPS + config LINKER_LIST_ALIGN int default 32 if SANDBOX @@ -29,6 +50,7 @@ config ARC select DM select HAVE_PRIVATE_LIBGCC select SUPPORT_OF_CONTROL + select SYS_CACHE_SHIFT_7 select TIMER config ARM @@ -44,6 +66,7 @@ config M68K select NEEDS_MANUAL_RELOC select SYS_BOOT_GET_CMDLINE select SYS_BOOT_GET_KBD + select SYS_CACHE_SHIFT_4 select SUPPORT_OF_CONTROL config MICROBLAZE @@ -122,6 +145,7 @@ config SANDBOX select SPI select SUPPORT_OF_CONTROL select SYSRESET_CMD_POWEROFF + select SYS_CACHE_SHIFT_4 select IRQ select SUPPORT_EXTENSION_SCAN imply BITREVERSE @@ -188,6 +212,7 @@ config X86 select OF_CONTROL select PCI select SUPPORT_OF_CONTROL + select SYS_CACHE_SHIFT_6 select TIMER select USE_PRIVATE_LIBGCC select X86_TSC_TIMER diff --git a/arch/arc/include/asm/cache.h b/arch/arc/include/asm/cache.h index ab61846b5ab9..a48e1aec6889 100644 --- a/arch/arc/include/asm/cache.h +++ b/arch/arc/include/asm/cache.h @@ -16,9 +16,6 @@ */ #define ARCH_DMA_MINALIGN 128 -/* CONFIG_SYS_CACHELINE_SIZE is used a lot in drivers */ -#define CONFIG_SYS_CACHELINE_SIZE ARCH_DMA_MINALIGN - #if defined(ARC_MMU_ABSENT) #define CONFIG_ARC_MMU_VER 0 #elif defined(CONFIG_ARC_MMU_V2) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index d692139199c4..e478b9088a09 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -332,21 +332,6 @@ config SYS_ARM_ARCH default 4 if CPU_SA1100 default 8 if ARM64 -config SYS_CACHE_SHIFT_5 - bool - -config SYS_CACHE_SHIFT_6 - bool - -config SYS_CACHE_SHIFT_7 - bool - -config SYS_CACHELINE_SIZE - int - default 128 if SYS_CACHE_SHIFT_7 - default 64 if SYS_CACHE_SHIFT_6 - default 32
Re: [PATCH 04/10] fdt_support: Implement fdt_ethernet_set_macaddr()
Hi Sean,
On 8/26/21 6:35 PM, Sean Anderson wrote:
On 8/26/21 5:42 PM, Alexandru Gagniuc wrote:
Oftentimes we have MAC address information stored in a ROM or OTP. The
way to add that to the FDT would be through the u-boot environment,
and then fdt_fixup_ethernet(). This is not very useful in SPL.
It would be more helpful to be able to "set interface x to MAC y".
This is where fdt_ethernet_set_macaddr() comes in. It is similar in
function to fdt_fixup_ethernet(), but only updates one interface,
without using the u-boot env, and without string processing.
Have you considered adopting the nvmem-cells property for ethernet
controllers (added in Linux commit 0e839df92cf3 ("net: ethernet: provide
nvmem_get_mac_address()"))?
Obviously I haven't. It sounds like a great idea. Thank you for pointing
me to it.
Alex
--Sean
Signed-off-by: Alexandru Gagniuc
---
common/fdt_support.c | 30 ++
include/fdt_support.h | 17 +
2 files changed, 47 insertions(+)
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 4341d84bd5..c4cbd4060e 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -592,6 +592,36 @@ void fdt_fixup_ethernet(void *fdt)
}
}
+int fdt_ethernet_set_macaddr(void *fdt, int ethnum, const uint8_t
*mac_addr)
+{
+ const char *path, *name;
+ int prop, aliases_node;
+ char eth_name[16] = "ethernet";
+
+ aliases_node = fdt_path_offset(fdt, "/aliases");
+ if (aliases_node < 0)
+ return aliases_node;
+
+ if (ethnum >= 0)
+ sprintf(eth_name, "ethernet%d", ethnum);
+
+ fdt_for_each_property_offset(prop, fdt, aliases_node) {
+ path = fdt_getprop_by_offset(fdt, prop, , NULL);
+ if (!strcmp(name, eth_name))
+ break;
+
+ path = NULL;
+ }
+
+ if (!path)
+ return -FDT_ERR_NOTFOUND;
+
+ do_fixup_by_path(fdt, path, "mac-address", mac_addr, 6, 0);
+ do_fixup_by_path(fdt, path, "local-mac-address", mac_addr, 6, 1);
+
+ return 0;
+}
+
int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/include/fdt_support.h b/include/fdt_support.h
index f6f46bb8e9..3f0bcb5a00 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -119,6 +119,23 @@ static inline int fdt_fixup_memory_banks(void
*blob, u64 start[], u64 size[],
#endif
void fdt_fixup_ethernet(void *fdt);
+
+/**
+ * Set the "mac-address" and "local-mac-address" of ethernet node
+ * The ethernet node is located from the "/aliases" section of the
fdt. When
+ * 'ethnum' is positive, then the name is matched exactly, e.g
"ethernet0".
+ * When ethnum is negative, the first ethernet alias is updated.
+ * Unlike fdt_fixup_ethernet(), this function only updates one
ethernet node,
+ * and soes not use the "ethaddr" from the u-boot environment. This
is useful,
+ * for example, in SPL, when the environment is not initialized or
available.
+ *
+ * @param fdt FDT blob to update
+ * @param ethnum Ethernet device index, or negative for any ethernet
+ * @param mac_addr Pointer to 6-byte array containing the MAC address
+ *
+ * @return 0 if ok, or -FDT_ERR_... on error
+ */
+int fdt_ethernet_set_macaddr(void *fdt, int ethnum, const uint8_t
*mac_addr);
int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
const void *val, int len, int create);
void fdt_fixup_qe_firmware(void *fdt);
Re: [PATCH 04/10] fdt_support: Implement fdt_ethernet_set_macaddr()
On 8/26/21 5:42 PM, Alexandru Gagniuc wrote:
Oftentimes we have MAC address information stored in a ROM or OTP. The
way to add that to the FDT would be through the u-boot environment,
and then fdt_fixup_ethernet(). This is not very useful in SPL.
It would be more helpful to be able to "set interface x to MAC y".
This is where fdt_ethernet_set_macaddr() comes in. It is similar in
function to fdt_fixup_ethernet(), but only updates one interface,
without using the u-boot env, and without string processing.
Have you considered adopting the nvmem-cells property for ethernet
controllers (added in Linux commit 0e839df92cf3 ("net: ethernet: provide
nvmem_get_mac_address()"))?
--Sean
Signed-off-by: Alexandru Gagniuc
---
common/fdt_support.c | 30 ++
include/fdt_support.h | 17 +
2 files changed, 47 insertions(+)
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 4341d84bd5..c4cbd4060e 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -592,6 +592,36 @@ void fdt_fixup_ethernet(void *fdt)
}
}
+int fdt_ethernet_set_macaddr(void *fdt, int ethnum, const uint8_t *mac_addr)
+{
+ const char *path, *name;
+ int prop, aliases_node;
+ char eth_name[16] = "ethernet";
+
+ aliases_node = fdt_path_offset(fdt, "/aliases");
+ if (aliases_node < 0)
+ return aliases_node;
+
+ if (ethnum >= 0)
+ sprintf(eth_name, "ethernet%d", ethnum);
+
+ fdt_for_each_property_offset(prop, fdt, aliases_node) {
+ path = fdt_getprop_by_offset(fdt, prop, , NULL);
+ if (!strcmp(name, eth_name))
+ break;
+
+ path = NULL;
+ }
+
+ if (!path)
+ return -FDT_ERR_NOTFOUND;
+
+ do_fixup_by_path(fdt, path, "mac-address", mac_addr, 6, 0);
+ do_fixup_by_path(fdt, path, "local-mac-address", mac_addr, 6, 1);
+
+ return 0;
+}
+
int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/include/fdt_support.h b/include/fdt_support.h
index f6f46bb8e9..3f0bcb5a00 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -119,6 +119,23 @@ static inline int fdt_fixup_memory_banks(void *blob, u64
start[], u64 size[],
#endif
void fdt_fixup_ethernet(void *fdt);
+
+/**
+ * Set the "mac-address" and "local-mac-address" of ethernet node
+ * The ethernet node is located from the "/aliases" section of the fdt. When
+ * 'ethnum' is positive, then the name is matched exactly, e.g "ethernet0".
+ * When ethnum is negative, the first ethernet alias is updated.
+ * Unlike fdt_fixup_ethernet(), this function only updates one ethernet node,
+ * and soes not use the "ethaddr" from the u-boot environment. This is useful,
+ * for example, in SPL, when the environment is not initialized or available.
+ *
+ * @param fdt FDT blob to update
+ * @param ethnum Ethernet device index, or negative for any ethernet
+ * @param mac_addr Pointer to 6-byte array containing the MAC address
+ *
+ * @return 0 if ok, or -FDT_ERR_... on error
+ */
+int fdt_ethernet_set_macaddr(void *fdt, int ethnum, const uint8_t *mac_addr);
int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
const void *val, int len, int create);
void fdt_fixup_qe_firmware(void *fdt);
[PATCH] mkimage: Don't disable encryption based on CONFIG_FIT_CIPHER
We want the u-boot tools to be target agnostic, as explained in commit
cb9faa6f98ae (" tools: Use a single target-independent config to
enable OpenSSL")
Making mkimage features depend on CONFIG_FIT_CIPHER is contrary to
that goal. Thus, always enable cihper features in mkimage, and ignore
the value of CONFIG_FIT_CIPHER for host-only code.
Signed-off-by: Alexandru Gagniuc
---
include/image.h| 5 -
tools/image-host.c | 2 +-
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/include/image.h b/include/image.h
index e20f0b69d5..381ee91eb5 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1432,14 +1432,9 @@ static inline int fit_image_check_target_arch(const void
*fdt, int node)
* device
*/
#if defined(USE_HOSTCC)
-# if defined(CONFIG_FIT_CIPHER)
# define IMAGE_ENABLE_ENCRYPT 1
# define IMAGE_ENABLE_DECRYPT 1
# include
-# else
-# define IMAGE_ENABLE_ENCRYPT 0
-# define IMAGE_ENABLE_DECRYPT 0
-# endif
#else
# define IMAGE_ENABLE_ENCRYPT 0
# define IMAGE_ENABLE_DECRYPT CONFIG_IS_ENABLED(FIT_CIPHER)
diff --git a/tools/image-host.c b/tools/image-host.c
index d3a882ec29..82cb238f54 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -562,7 +562,7 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
printf("Failure getting cipher node\n");
return -1;
}
- if (!IMAGE_ENABLE_ENCRYPT || !keydir)
+ if (keydir)
return 0;
return fit_image_process_cipher(keydir, keydest, fit, image_name,
image_noffset, cipher_node_offset, data, size, cmdname);
--
2.31.1
[RFC PATCH] stm32mp1: Replace STM32IMAGE config with TFABOOT_FIP
Hi Patrick,
I proposing a better fix fir the issues I outlined earlier, I made a
classification of the currently supported boot modes.
1) BL1 -> SPL -> u-boot
2) BL1 -> SPL -> OP-TEE
-
| 3) BL1 -> TF-A -> u-boot |
| 4) BL1 -> TF-A -> OP-TEE |
| _ |
|| 5) BL1 -> TF-A -> FIP container ||
|| CONFIG_TFABOOT_FIP ||
||_||
| |
| CONFIG_TFABOOT|
-
Here, I'm looking at FIP as a new boot mode. In order to avoid
breakage, any changes to support FIP it should naturally be done only
to this new path.
This proposal contains several changes, but I've squashed them into
one for ease of discussion.
This better matches the boot mode classification above.
This config mixes boot path (2) with paths (3) and (4), and thus is
contrary to our goal of making changes only to the new FIP path.
Because it mixes and matches SPL assumptions with TF-A assumptions,
I've had a hard time figuting it out. I suspect it would be just as
confusing for others in the future.
I've had issues with tee_find_device() in the past when using SPL as
the FSBL. As u-boot was running in secure mode and did not have a
handler, it would result in a CPU exception and crash.
The second argument against this is that stm32mp1 is the only platform
to call tee_find_device() with the intent of detecting the presence of
OP-TEE.
Have there been issues with not callinf this in the past, or was this
more of a "seems nice to have" ?
"stm32mp15_defconfig" implies that would be the correct configuraion
for STM32MP1. New contributor chooses this config, tries to run SPL
+ u-boot, which is what u-boot user expects is the default. Things
likely fail miserably. A lot of u-boot users don't know what FIP is.
It's an extra concept that is not strictly necessary in u-boot.
So I think this name is vague, as it doesn't really describe what is
going on. If we change it to "stm32mp15_tfaboot_fip_defconfig", then
it very accurately describes the boot scenario, and avoids the
confusion above.
We're setting CONFIG_MTDPARTS_xxx based on TFABOOT_FIP_CONTAINER now,
so I don't think we need any ifdefs here. This part needs the most
scrutiny, as I don't have a way to test if I've broken anything.
---
arch/arm/Kconfig | 14 +++
arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 7 ++--
arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi | 9 ++---
arch/arm/mach-stm32mp/Kconfig | 7
.../cmd_stm32prog/cmd_stm32prog.c | 2 -
.../mach-stm32mp/cmd_stm32prog/stm32prog.c| 4 --
.../mach-stm32mp/cmd_stm32prog/stm32prog.h| 2 -
arch/arm/mach-stm32mp/config.mk | 2 +-
arch/arm/mach-stm32mp/fdt.c | 37 ---
.../arm/mach-stm32mp/include/mach/stm32prog.h | 2 -
board/st/common/Kconfig | 20 +-
board/st/common/stm32mp_mtdparts.c| 18 -
board/st/stm32mp1/stm32mp1.c | 6 +--
...config => stm32mp15_tfaboot_fip_defconfig} | 1 +
configs/stm32mp15_trusted_defconfig | 1 -
15 files changed, 35 insertions(+), 97 deletions(-)
rename configs/{stm32mp15_defconfig => stm32mp15_tfaboot_fip_defconfig} (99%)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index d692139199..4c6f7ab3de 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1905,6 +1905,20 @@ config TFABOOT
Enabling this option will make a U-Boot binary that is relying
on other firmware layers to provide secure functionality.
+config TFABOOT_FIP_CONTAINER
+ bool "Support for booting from TF-A inside a FIP container"
+ depends on TFABOOT
+ default n
+ help
+ TF-A has its own container format, named FIP (not to be confused with
+ FIT). When u-boot is started from a FIP, it sometimes needs to make
+ different assumptions than it would with a non-FIP boot. Although
+ those could be resolved with dynamic devicetree patching, TF-A is
+ either can't patch devicetrees, or is unwilling to do so.
+ Enabling this option will tell u-boot platform code that it is okay
+ to assume U-Boot will be started from a FIP container, even if such
+ assumptions would break things in a more normal setting.
+
config TI_SECURE_DEVICE
bool "HS Device Type Support"
depends on ARCH_KEYSTONE || ARCH_OMAP2PLUS || ARCH_K3
diff --git a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
[PATCH 08/10] lib: Makefile: Make optee library available in SPL
We want the optee_copy_fdt_nodes symbols in SPL. This is for cases when booting an OPTEE payload directly. Signed-off-by: Alexandru Gagniuc --- lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Makefile b/lib/Makefile index 8ba745faa0..73dacbb01b 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -16,7 +16,6 @@ obj-$(CONFIG_FIT) += libfdt/ obj-$(CONFIG_OF_LIVE) += of_live.o obj-$(CONFIG_CMD_DHRYSTONE) += dhry/ obj-$(CONFIG_ARCH_AT91) += at91/ -obj-$(CONFIG_OPTEE) += optee/ obj-$(CONFIG_ASN1_DECODER) += asn1_decoder.o obj-y += crypto/ @@ -74,6 +73,7 @@ obj-$(CONFIG_$(SPL_)GZIP) += gunzip.o obj-$(CONFIG_$(SPL_)LZO) += lzo/ obj-$(CONFIG_$(SPL_)LZMA) += lzma/ obj-$(CONFIG_$(SPL_)LZ4) += lz4_wrapper.o +obj-$(CONFIG_OPTEE) += optee/ obj-$(CONFIG_$(SPL_)LIB_RATIONAL) += rational.o -- 2.31.1
[PATCH 10/10] stm32mp1: spl: Copy optee nodes to target FDT for OP-TEE payloads
OP-TEE does not take a devicetree for its own use. However, it does
pass the devicetree to the normal world OS. In most cases that will
be some other devicetree-bearing platform, such as linux.
As in other cases where there's an OPTEE payload (e.g. BOOTM_OPTEE),
it is required to copy the optee nodes to he target's FDT. Do this as
part of spl_board_prepare_for_optee().
Signed-off-by: Alexandru Gagniuc
---
arch/arm/mach-stm32mp/spl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
index d9fdc5926c..94fbb45cf9 100644
--- a/arch/arm/mach-stm32mp/spl.c
+++ b/arch/arm/mach-stm32mp/spl.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
u32 spl_boot_device(void)
{
@@ -182,6 +183,7 @@ void stm32_init_tzc_for_optee(void)
void spl_board_prepare_for_optee(void *fdt)
{
stm32_fdt_setup_mac_addr(fdt);
+ optee_copy_fdt_nodes(fdt);
stm32_init_tzc_for_optee();
}
--
2.31.1
[PATCH 07/10] stm32mp1: spl: Configure MAC address when booting OP-TEE
When OP-TEE is booted as the SPL payload, the stage after OP-TEE is
not guaranteed to be u-boot. Thus the FDT patching in u-boot is not
guaranteed to occur. Add this step to SPL.
The patching by stm32_fdt_setup_mac_addr() is done in SPL, and patches
the target FDT directly. This differs is different from
setup_mac_address(), which sets the "ethaddr" env variable, and does
not work in SPL.
Signed-off-by: Alexandru Gagniuc
---
arch/arm/mach-stm32mp/cpu.c | 22 +++
.../arm/mach-stm32mp/include/mach/sys_proto.h | 3 +++
arch/arm/mach-stm32mp/spl.c | 1 +
3 files changed, 26 insertions(+)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
index 8727de513c..2b8b67bb40 100644
--- a/arch/arm/mach-stm32mp/cpu.c
+++ b/arch/arm/mach-stm32mp/cpu.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -646,6 +647,27 @@ __weak int setup_mac_address(void)
return 0;
}
+int stm32_fdt_setup_mac_addr(void *fdt)
+{
+ int ret;
+ uchar enetaddr[ARP_HLEN];
+
+ ret = stm32_read_otp_mac(enetaddr);
+ if (ret < 0)
+ return ret;
+
+ if (!is_valid_ethaddr(enetaddr)) {
+ printf("invalid MAC address in OTP\n");
+ return -EINVAL;
+ }
+
+ ret = fdt_ethernet_set_macaddr(fdt, 0, enetaddr);
+ if (ret)
+ debug("Failed to set mac address from OTP: %d\n", ret);
+
+ return ret;
+}
+
static int setup_serial_number(void)
{
char serial_string[25];
diff --git a/arch/arm/mach-stm32mp/include/mach/sys_proto.h
b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
index 4149d3a133..2d24cfee3f 100644
--- a/arch/arm/mach-stm32mp/include/mach/sys_proto.h
+++ b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
@@ -47,7 +47,10 @@ void get_soc_name(char name[SOC_NAME_SIZE]);
/* return boot mode */
u32 get_bootmode(void);
+/* Set 'ethaddr' env variable with MAC from OTP (useful for u-boot proper) */
int setup_mac_address(void);
+/* Patch the first 'ethernet' node of FDT with MAC from OTP (useful for SPL) */
+int stm32_fdt_setup_mac_addr(void *fdt);
/* board power management : configure vddcore according OPP */
void board_vddcore_init(u32 voltage_mv);
diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
index 405eff68a3..d9fdc5926c 100644
--- a/arch/arm/mach-stm32mp/spl.c
+++ b/arch/arm/mach-stm32mp/spl.c
@@ -181,6 +181,7 @@ void stm32_init_tzc_for_optee(void)
void spl_board_prepare_for_optee(void *fdt)
{
+ stm32_fdt_setup_mac_addr(fdt);
stm32_init_tzc_for_optee();
}
--
2.31.1
[PATCH 06/10] arm: stm32mp: Factor out reading MAC address from OTP
Move the reading the OTP into a separate function. This is
required for a subsequent change which sets the MAC in SPL.
Signed-off-by: Alexandru Gagniuc
---
arch/arm/mach-stm32mp/cpu.c | 37 +++--
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
index eb79f3ffd2..8727de513c 100644
--- a/arch/arm/mach-stm32mp/cpu.c
+++ b/arch/arm/mach-stm32mp/cpu.c
@@ -593,6 +593,28 @@ static void setup_boot_mode(void)
clrsetbits_le32(TAMP_BOOT_CONTEXT, TAMP_BOOT_FORCED_MASK, BOOT_NORMAL);
}
+static int stm32_read_otp_mac(uint8_t enetaddr[ARP_HLEN])
+{
+ struct udevice *dev;
+ int ret, i;
+ u32 otp[2];
+
+ ret = uclass_get_device_by_driver(UCLASS_MISC,
+ DM_DRIVER_GET(stm32mp_bsec),
+ );
+ if (ret)
+ return ret;
+
+ ret = misc_read(dev, STM32_BSEC_SHADOW(BSEC_OTP_MAC), otp, sizeof(otp));
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < ARP_HLEN; i++)
+ enetaddr[i] = ((uint8_t *))[i];
+
+ return 0;
+}
+
/*
* If there is no MAC address in the environment, then it will be initialized
* (silently) from the value in the OTP.
@@ -601,29 +623,16 @@ __weak int setup_mac_address(void)
{
#if defined(CONFIG_NET)
int ret;
- int i;
- u32 otp[2];
uchar enetaddr[6];
- struct udevice *dev;
/* MAC already in environment */
if (eth_env_get_enetaddr("ethaddr", enetaddr))
return 0;
- ret = uclass_get_device_by_driver(UCLASS_MISC,
- DM_DRIVER_GET(stm32mp_bsec),
- );
- if (ret)
- return ret;
-
- ret = misc_read(dev, STM32_BSEC_SHADOW(BSEC_OTP_MAC),
- otp, sizeof(otp));
+ ret = stm32_read_otp_mac(enetaddr);
if (ret < 0)
return ret;
- for (i = 0; i < 6; i++)
- enetaddr[i] = ((uint8_t *))[i];
-
if (!is_valid_ethaddr(enetaddr)) {
log_err("invalid MAC address in OTP %pM\n", enetaddr);
return -EINVAL;
--
2.31.1
[PATCH 09/10] ARM: dts: stm32mp: Add OP-TEE "/firmware" node to SPL dtb
This node is required in SPL when booting an OP-TEE payload. Add it to
the SPL devicetree.
Signed-off-by: Alexandru Gagniuc
---
arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
index 0101962ea5..2e65b9b4d5 100644
--- a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
@@ -31,6 +31,7 @@
optee {
compatible = "linaro,optee-tz";
method = "smc";
+ u-boot,dm-spl;
};
};
--
2.31.1
[PATCH 04/10] fdt_support: Implement fdt_ethernet_set_macaddr()
Oftentimes we have MAC address information stored in a ROM or OTP. The
way to add that to the FDT would be through the u-boot environment,
and then fdt_fixup_ethernet(). This is not very useful in SPL.
It would be more helpful to be able to "set interface x to MAC y".
This is where fdt_ethernet_set_macaddr() comes in. It is similar in
function to fdt_fixup_ethernet(), but only updates one interface,
without using the u-boot env, and without string processing.
Signed-off-by: Alexandru Gagniuc
---
common/fdt_support.c | 30 ++
include/fdt_support.h | 17 +
2 files changed, 47 insertions(+)
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 4341d84bd5..c4cbd4060e 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -592,6 +592,36 @@ void fdt_fixup_ethernet(void *fdt)
}
}
+int fdt_ethernet_set_macaddr(void *fdt, int ethnum, const uint8_t *mac_addr)
+{
+ const char *path, *name;
+ int prop, aliases_node;
+ char eth_name[16] = "ethernet";
+
+ aliases_node = fdt_path_offset(fdt, "/aliases");
+ if (aliases_node < 0)
+ return aliases_node;
+
+ if (ethnum >= 0)
+ sprintf(eth_name, "ethernet%d", ethnum);
+
+ fdt_for_each_property_offset(prop, fdt, aliases_node) {
+ path = fdt_getprop_by_offset(fdt, prop, , NULL);
+ if (!strcmp(name, eth_name))
+ break;
+
+ path = NULL;
+ }
+
+ if (!path)
+ return -FDT_ERR_NOTFOUND;
+
+ do_fixup_by_path(fdt, path, "mac-address", mac_addr, 6, 0);
+ do_fixup_by_path(fdt, path, "local-mac-address", mac_addr, 6, 1);
+
+ return 0;
+}
+
int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/include/fdt_support.h b/include/fdt_support.h
index f6f46bb8e9..3f0bcb5a00 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -119,6 +119,23 @@ static inline int fdt_fixup_memory_banks(void *blob, u64
start[], u64 size[],
#endif
void fdt_fixup_ethernet(void *fdt);
+
+/**
+ * Set the "mac-address" and "local-mac-address" of ethernet node
+ * The ethernet node is located from the "/aliases" section of the fdt. When
+ * 'ethnum' is positive, then the name is matched exactly, e.g "ethernet0".
+ * When ethnum is negative, the first ethernet alias is updated.
+ * Unlike fdt_fixup_ethernet(), this function only updates one ethernet node,
+ * and soes not use the "ethaddr" from the u-boot environment. This is useful,
+ * for example, in SPL, when the environment is not initialized or available.
+ *
+ * @param fdt FDT blob to update
+ * @param ethnum Ethernet device index, or negative for any ethernet
+ * @param mac_addr Pointer to 6-byte array containing the MAC address
+ *
+ * @return 0 if ok, or -FDT_ERR_... on error
+ */
+int fdt_ethernet_set_macaddr(void *fdt, int ethnum, const uint8_t *mac_addr);
int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
const void *val, int len, int create);
void fdt_fixup_qe_firmware(void *fdt);
--
2.31.1
[PATCH 05/10] arm: stm32mp: bsec: Do not skip .probe() for SPL
stm32mp_bsec_probe() was skipped for TFABOOT and SPL_BUILD. The idea
of skipping probe() is that we can't access BSEC from the normal
world. This is true with TFABOOT. However, in SPL, we are in the
secure world, so skipping probe is incorrect. In fact, SPL is not
even built when TFABOOT is selected.
Thus, only skip probe with TFABOOT, but not SPL_BUILD.
Signed-off-by: Alexandru Gagniuc
---
arch/arm/mach-stm32mp/bsec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-stm32mp/bsec.c b/arch/arm/mach-stm32mp/bsec.c
index fe39bd80cf..a02d19c1b9 100644
--- a/arch/arm/mach-stm32mp/bsec.c
+++ b/arch/arm/mach-stm32mp/bsec.c
@@ -506,7 +506,7 @@ static int stm32mp_bsec_probe(struct udevice *dev)
* only executed in U-Boot proper when TF-A is not used
*/
- if (!IS_ENABLED(CONFIG_TFABOOT) && !IS_ENABLED(CONFIG_SPL_BUILD)) {
+ if (!IS_ENABLED(CONFIG_TFABOOT)) {
plat = dev_get_plat(dev);
for (otp = 57; otp <= BSEC_OTP_MAX_VALUE; otp++)
--
2.31.1
[PATCH 03/10] board: stm32mp1: Implement board_fit_config_name_match() for SPL
This function is needed when loading a FIT image from SPL. It selects
the correct configuration node for the current board. Implement it.
Signed-off-by: Alexandru Gagniuc
---
board/st/stm32mp1/spl.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/board/st/stm32mp1/spl.c b/board/st/stm32mp1/spl.c
index bb210d7727..543c037ad8 100644
--- a/board/st/stm32mp1/spl.c
+++ b/board/st/stm32mp1/spl.c
@@ -5,6 +5,7 @@
#include
#include
+#include
#include
#include
#include
@@ -92,3 +93,12 @@ void board_debug_uart_init(void)
#endif
}
#endif
+
+int board_fit_config_name_match(const char *name)
+{
+ if (of_machine_is_compatible("st,stm32mp157c-dk2"))
+ return !strstr(name, "stm32mp157c-dk2");
+
+ /* Okay, it's most likely an EV board */
+ return !strstr(name, "stm32mp157") + !strstr(name, "-ev");
+}
--
2.31.1
[PATCH 02/10] stm32mp1: Add support for falcon mode boot from SD card
Falcon mode requires a board-specific mechanism to select between
fast and normal boot. This is done via spl_start_uboot()
Use the B2 button as the selection mechanism. This is connected to
GPIO PA13. Incidentally, this GPIO is already accessible via the
"st,fastboot-gpios" devicetree node.
Offsets for raw MMC loading are defined. These point to the partition
after "ssbl".
Signed-off-by: Alexandru Gagniuc
---
board/st/stm32mp1/spl.c| 39 ++
include/configs/stm32mp1.h | 13 +
2 files changed, 52 insertions(+)
diff --git a/board/st/stm32mp1/spl.c b/board/st/stm32mp1/spl.c
index 8e4549a1b3..bb210d7727 100644
--- a/board/st/stm32mp1/spl.c
+++ b/board/st/stm32mp1/spl.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include "../common/stpmic1.h"
@@ -29,6 +30,44 @@ int board_early_init_f(void)
return 0;
}
+#if IS_ENABLED(CONFIG_SPL_OS_BOOT)
+int spl_start_uboot(void)
+{
+ ofnode node;
+ struct gpio_desc gpio;
+ int boot_uboot = 1;
+
+ node = ofnode_path("/config");
+ if (!ofnode_valid(node)) {
+ pr_warn("%s: no /config node?\n", __func__);
+ return 0;
+ }
+
+ if (gpio_request_by_name_nodev(node, "st,fastboot-gpios", 0, ,
+ GPIOD_IS_IN)) {
+ pr_warn("%s: could not find a /config/st,fastboot-gpios\n",
+ __func__);
+ return 1;
+ }
+
+ boot_uboot = dm_gpio_get_value();
+ dm_gpio_free(NULL, );
+
+ return boot_uboot;
+}
+
+#if IS_ENABLED(CONFIG_ARMV7_NONSEC)
+/*
+ * A bit of a hack, but armv7_boot_nonsec() is provided by bootm.c. This is not
+ * available in SPL, so we have to provide an implementation.
+ */
+bool armv7_boot_nonsec(void)
+{
+ return 0;
+}
+#endif /* CONFIG_ARMV7_NONSEC */
+#endif /* CONFIG_SPL_OS_BOOT */
+
#ifdef CONFIG_DEBUG_UART_BOARD_INIT
void board_debug_uart_init(void)
{
diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
index 9fcd60285a..0849a1bddb 100644
--- a/include/configs/stm32mp1.h
+++ b/include/configs/stm32mp1.h
@@ -10,6 +10,19 @@
#include
#include
+/*
+ * Arguments if falcon mode is used
+ * CONFIG_SYS_MMCSD_RAW_MODE_KERNEL_SECTOR is the partition after "ssbl"
+ * CONFIG_SYS_SPL_ARGS_ADDR is not used, but needs to point to valid RAM.
+ */
+#define CONFIG_SYS_MMCSD_RAW_MODE_KERNEL_SECTOR 5154
+#define CONFIG_SYS_SPL_ARGS_ADDR 0xc400
+
+/* Falcon mode from SPI is not supported, but the defines are needed */
+#define CONFIG_SYS_SPI_KERNEL_OFFS (~0)
+#define CONFIG_SYS_SPI_ARGS_OFFS (~0)
+#define CONFIG_SYS_SPI_ARGS_SIZE 0
+
#ifndef CONFIG_TFABOOT
/* PSCI support */
#define CONFIG_ARMV7_SECURE_BASE STM32_SYSRAM_BASE
--
2.31.1
[PATCH 01/10] stm32mp1: Add support for baudrates higher than 115200
The UART can reliably go up to 200 baud when connected to the
on-board st-link. Unfortunately u-boot will fall back to 115200 unless
higher rates are declared via CONFIG_SYS_BAUDRATE_TABLE.
Signed-off-by: Alexandru Gagniuc
---
include/configs/stm32mp1.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
index b372838be8..9fcd60285a 100644
--- a/include/configs/stm32mp1.h
+++ b/include/configs/stm32mp1.h
@@ -16,6 +16,10 @@
#define CONFIG_ARMV7_SECURE_MAX_SIZE STM32_SYSRAM_SIZE
#endif
+#define CONFIG_SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, \
+230400, 460800, 921600, \
+100, 200 }
+
/*
* Configuration of the external SRAM memory used by U-Boot
*/
--
2.31.1
[PATCH 00/10] stm32mp1: Support falcon mode with OP-TEE payloads
My goal when I started on this project a year ago was to get to linux userspace within a second from power on. Oh, and it had to be secure! Contrast that to the two minutes it took the STLinux demo to come up. It was obvious that the accepted way of running an FSBL, then SSBL was going to blow the time budget. There really wasn't a good solution, and traditional falcon mode with "spl export" command was not secure. I chose to use SPL with a FIT payload. We have to add certain logic to SPL, as well as some FDT modifications that would be normally done in u-boot. The boot flow is SPL -> OP-TEE -> Linux Incidentally, these patches are some of the earlier ones I wrote for this project. It didn't make sense to publish them at the time, as the supporting infrastructure was not in place then I decided not to separate these patches into mini-series. Alexandru Gagniuc (10): stm32mp1: Add support for baudrates higher than 115200 stm32mp1: Add support for falcon mode boot from SD card board: stm32mp1: Implement board_fit_config_name_match() for SPL fdt_support: Implement fdt_ethernet_set_macaddr() arm: stm32mp: bsec: Do not skip .probe() for SPL arm: stm32mp: Factor out reading MAC address from OTP stm32mp1: spl: Configure MAC address when booting OP-TEE lib: Makefile: Make optee library available in SPL ARM: dts: stm32mp: Add OP-TEE "/firmware" node to SPL dtb stm32mp1: spl: Copy optee nodes to target FDT for OP-TEE payloads arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi | 1 + arch/arm/mach-stm32mp/bsec.c | 2 +- arch/arm/mach-stm32mp/cpu.c | 59 ++- .../arm/mach-stm32mp/include/mach/sys_proto.h | 3 + arch/arm/mach-stm32mp/spl.c | 3 + board/st/stm32mp1/spl.c | 49 +++ common/fdt_support.c | 30 ++ include/configs/stm32mp1.h| 17 ++ include/fdt_support.h | 17 ++ lib/Makefile | 2 +- 10 files changed, 167 insertions(+), 16 deletions(-) -- 2.31.1
Re: Usage of device-tree for blobs
On 8/26/21 3:54 PM, Simon Glass wrote: Hi Heinrich, On Thu, 26 Aug 2021 at 01:10, Heinrich Schuchardt wrote: On 8/26/21 5:15 AM, Simon Glass wrote: > Hi Heinrich, > > On Wed, 25 Aug 2021 at 02:05, Heinrich Schuchardt wrote: >> >> Hello Simon, >> >> some boards like qemu-riscv64_defconfig do not use any device-tree at >> build time. A device-tree is only supplied at runtime by the prior boot >> stage (CONFIG_OF_PRIOR_STAGE=y). >> >> In doc/develop/devicetree/intro.rst you suggest to put binary blobs into >> the device-tree. >> >> Could you, please, update the documentation to explain how adding blobs >> to the device-tree works in the different scenarios depending on the >> values of: >> >> CONFIG_OF_EMBED >> CONFIG_OF_SEPARATE >> CONFIG_OF_BOARD >> CONFIG_OF_HOSTFILE >> CONFIG_OF_PRIOR_STAGE >> >> It would be especially important to understand how one can develop a >> board independent feature which works for all of these settings. > > OK I will take a crack at this tomorrow. > > But I think there is a disconnect here, since the only options that > matter within U-Boot are OF_SEPARATE and OF_HOSTFILE, which both use a > u-boot.dtb file. There is nothing tricky here. The following boards use OF_PRIO_STAGE: * QEMU * bcm7260_defconfig * bcm7445_defconfig * ae350_rv32_defconfig * ae350_rv32_spl_defconfig * ae350_rv64_defconfig * ae350_rv64_spl_defconfig Most of these seem OK as they have an in-tree DT. But the arm and riscv qemus and the bcm builds do not: bcm7260_defconfig bcm7445_defconfig configs/qemu_arm64_defconfig configs/qemu_arm_defconfig configs/qemu-ppce500_defconfig configs/qemu-riscv32_defconfig configs/qemu-riscv32_smode_defconfig configs/qemu-riscv64_defconfig configs/qemu-riscv64_smode_defconfig I think we are going to have to ban this. We are not really testing the build at all, and it presumably depends on the version of qemu that is used. It's OK to provide the DT to U-Boot as one flow, but not to completely drop it from the tree. Where is the qemu source code that creates these DTs? > > The OF_BOARD business is for when the board does special things. > Presumably signing will do special things too. We cannot really know > what those things are because the board as 'opted out' of the standard > options. > >> >> Please, describe CONFIG_OF_PRIOR_STAGE in >> doc/develop/devicetree/control.rst. > > So I'm not allowed to delete that option? :-) No. > It seems to me to be > extremely sparse on documentation. We need an explanation of what it > means and what effect it has on the build system, U-Boot and some > discussion of how qemu works. It seems to have been added as part of > an unrelated broadcom commit. The tags were incorrect so I doubt > anyone noticed it. Since then it has apparently proved useful > elsewhere, but no one has added more docs. > > So perhaps you can help me with my doc by explaining how > OF_PRIOR_STAGE works in qmeu, why the DT in U-Boot cannot be used, and > which project actually hosts the DT that qemu provides? Armed with > that information, I might be able to make sense of it all. The DT provided by QEMU is not hosted anywhere. It is generated on the fly by QEMU in dependence of the command line arguments that are used for calling QEMU. The project is hosted at https://github.com/qemu/qemu. 404 on that. Do you have a link to the code that actually generates the DT? https://github.com/qemu/qemu/blob/master/hw/riscv/virt.c#L179 On RISC-V the address of the device-tree of the prior bootstage is provided in register t0. On ARM platforms QEMU places the device-tree at 0x4000. QEMU is not the only platform where the prior boot stage supplies the device-tree which is to be used for booting the operating system. Any platform using TF-A or OpenSBI can be setup in this way. Generally it would be preferable that the prior boot stage provides the device-tree. But unfortunately Linux is not always backwards compatible. Don't expect the device-tree of the prior stage to contain any U-Boot specific quirks. This is my concern. I think every board in U-Boot needs at least a basic DT, even if only a skeleton, so we have places to put things. Any packaging at all needs a binman node, for example. U-Boot also has its own options for certain things. Also, where does the environment come from on these boards? Having the env in one tree and the DT in another must make things very interesting. Regards, Simon
Re: Usage of device-tree for blobs
> From: Simon Glass > Date: Thu, 26 Aug 2021 14:00:12 -0600 > > Hi Mark, > > On Thu, 26 Aug 2021 at 06:55, Mark Kettenis wrote: > > > > > From: Simon Glass > > > Date: Wed, 25 Aug 2021 21:15:00 -0600 > > > > > > Hi Heinrich, > > > > > > On Wed, 25 Aug 2021 at 02:05, Heinrich Schuchardt > > > wrote: > > > > > > > > Hello Simon, > > > > > > > > some boards like qemu-riscv64_defconfig do not use any device-tree at > > > > build time. A device-tree is only supplied at runtime by the prior boot > > > > stage (CONFIG_OF_PRIOR_STAGE=y). > > > > > > > > In doc/develop/devicetree/intro.rst you suggest to put binary blobs into > > > > the device-tree. > > > > > > > > Could you, please, update the documentation to explain how adding blobs > > > > to the device-tree works in the different scenarios depending on the > > > > values of: > > > > > > > > CONFIG_OF_EMBED > > > > CONFIG_OF_SEPARATE > > > > CONFIG_OF_BOARD > > > > CONFIG_OF_HOSTFILE > > > > CONFIG_OF_PRIOR_STAGE > > > > > > > > It would be especially important to understand how one can develop a > > > > board independent feature which works for all of these settings. > > > > > > OK I will take a crack at this tomorrow. > > > > > > But I think there is a disconnect here, since the only options that > > > matter within U-Boot are OF_SEPARATE and OF_HOSTFILE, which both use a > > > u-boot.dtb file. There is nothing tricky here. > > > > > > The OF_BOARD business is for when the board does special things. > > > Presumably signing will do special things too. We cannot really know > > > what those things are because the board as 'opted out' of the standard > > > options. > > > > > > > > > > > Please, describe CONFIG_OF_PRIOR_STAGE in > > > > doc/develop/devicetree/control.rst. > > > > > > So I'm not allowed to delete that option? :-) It seems to me to be > > > extremely sparse on documentation. We need an explanation of what it > > > means and what effect it has on the build system, U-Boot and some > > > discussion of how qemu works. It seems to have been added as part of > > > an unrelated broadcom commit. The tags were incorrect so I doubt > > > anyone noticed it. Since then it has apparently proved useful > > > elsewhere, but no one has added more docs. > > > > > > So perhaps you can help me with my doc by explaining how > > > OF_PRIOR_STAGE works in qmeu, why the DT in U-Boot cannot be used, and > > > which project actually hosts the DT that qemu provides? Armed with > > > that information, I might be able to make sense of it all. > > > > Well, QEMU allows configuration of (emulated/paravirtualised) devices > > from the command line. So it is pretty much impossible for U-Boot to > > provide a DT that matches that configuration without adding a lot of > > code the dynamically build it from some sort of configuration > > specification provided by QEMU to U-Boot. But at that point QEMU > > might as well provide the DT itself, don't you agree? > > Don't these devices have a DT in U-Boot? Is qemu enabling them (status > = "okay") or actually inventing them?! I actually invents them. See the mail I just sent for a reference to the code. > > Anyway, replying to this thread since for the Apple M1 support that > > I'm working on we're in a somewhat similar situation. The Asahi Linux > > project has implemented m1n1 as a bootloader and plans to use U-Boot > > as a "payload" to implement booting a standard Linux distribution (and > > other OSes). In this scenario m1n1 actually provides the DT since it > > parses Apple's version of the DT (which isn't in the standard DT > > format) and adds/updates certain properties that depend on the actual > > hardware it is running on. > > Yes I see. Still I'd like a basic DT in U-Boot, even if just for > discoverability. Discoverability of what? > > For this code I use CONFIG_OF_BOARD and implement > > board_fdt_blob_setup() to simple return a pointer to the DT that m1n1 > > set up. But that seems to be exactly what CONFIG_OF_PRIOR_STAGE > > does... > > Right, and neither is really documented to the level that people can > understand the purpose. They just come across as hacks to me. Well, I think U-Boot by its very nature is a giant hack ;). > I'd like to see an API for passing a info (including DT) to U-Boot. I > think riscv has it? I suggest we use a register that points to a > bloblist. Well, having a generic API will be hard. It will depend on the low-level firmware. On RISC-V the SBI specification defines such an API. For the Apple M1 I just use the existing code that makes U-Boot look like a Linux kernel and use the same API that is used to pass the DTB to the kernel. The Raspberry Pi code does something similar. In the end it just boils down to passing a pointer to the DT in a specific register.
Re: Usage of device-tree for blobs
> From: Simon Glass > Date: Thu, 26 Aug 2021 13:54:49 -0600 > > Hi Heinrich, > > > On Thu, 26 Aug 2021 at 01:10, Heinrich Schuchardt wrote: > > > > On 8/26/21 5:15 AM, Simon Glass wrote: > > > Hi Heinrich, > > > > > > On Wed, 25 Aug 2021 at 02:05, Heinrich Schuchardt > > > wrote: > > >> > > >> Hello Simon, > > >> > > >> some boards like qemu-riscv64_defconfig do not use any device-tree at > > >> build time. A device-tree is only supplied at runtime by the prior boot > > >> stage (CONFIG_OF_PRIOR_STAGE=y). > > >> > > >> In doc/develop/devicetree/intro.rst you suggest to put binary blobs into > > >> the device-tree. > > >> > > >> Could you, please, update the documentation to explain how adding blobs > > >> to the device-tree works in the different scenarios depending on the > > >> values of: > > >> > > >> CONFIG_OF_EMBED > > >> CONFIG_OF_SEPARATE > > >> CONFIG_OF_BOARD > > >> CONFIG_OF_HOSTFILE > > >> CONFIG_OF_PRIOR_STAGE > > >> > > >> It would be especially important to understand how one can develop a > > >> board independent feature which works for all of these settings. > > > > > > OK I will take a crack at this tomorrow. > > > > > > But I think there is a disconnect here, since the only options that > > > matter within U-Boot are OF_SEPARATE and OF_HOSTFILE, which both use a > > > u-boot.dtb file. There is nothing tricky here. > > > > The following boards use OF_PRIO_STAGE: > > > > * QEMU > > * bcm7260_defconfig > > * bcm7445_defconfig > > * ae350_rv32_defconfig > > * ae350_rv32_spl_defconfig > > * ae350_rv64_defconfig > > * ae350_rv64_spl_defconfig > > Most of these seem OK as they have an in-tree DT. But the arm and > riscv qemus and the bcm builds do not: > > bcm7260_defconfig > bcm7445_defconfig > configs/qemu_arm64_defconfig > configs/qemu_arm_defconfig > configs/qemu-ppce500_defconfig > configs/qemu-riscv32_defconfig > configs/qemu-riscv32_smode_defconfig > configs/qemu-riscv64_defconfig > configs/qemu-riscv64_smode_defconfig > > I think we are going to have to ban this. We are not really testing > the build at all, and it presumably depends on the version of qemu > that is used. It's OK to provide the DT to U-Boot as one flow, but not > to completely drop it from the tree. So you want to have a DT in the U-Boot tree that is completely unused? > Where is the qemu source code that creates these DTs? For ARM the code is in: https://github.com/qemu/qemu/blob/master/hw/arm/virt.c > > > The OF_BOARD business is for when the board does special things. > > > Presumably signing will do special things too. We cannot really know > > > what those things are because the board as 'opted out' of the standard > > > options. > > > > > >> > > >> Please, describe CONFIG_OF_PRIOR_STAGE in > > >> doc/develop/devicetree/control.rst. > > > > > > So I'm not allowed to delete that option? :-) > > > > No. > > > > > It seems to me to be > > > extremely sparse on documentation. We need an explanation of what it > > > means and what effect it has on the build system, U-Boot and some > > > discussion of how qemu works. It seems to have been added as part of > > > an unrelated broadcom commit. The tags were incorrect so I doubt > > > anyone noticed it. Since then it has apparently proved useful > > > elsewhere, but no one has added more docs. > > > > > > So perhaps you can help me with my doc by explaining how > > > OF_PRIOR_STAGE works in qmeu, why the DT in U-Boot cannot be used, and > > > which project actually hosts the DT that qemu provides? Armed with > > > that information, I might be able to make sense of it all. > > > > The DT provided by QEMU is not hosted anywhere. It is generated on the > > fly by QEMU in dependence of the command line arguments that are used > > for calling QEMU. The project is hosted at > > > > https://github.com/qemu/qemu. > > 404 on that. Do you have a link to the code that actually generates the DT? > > > > > On RISC-V the address of the device-tree of the prior bootstage is > > provided in register t0. > > > > On ARM platforms QEMU places the device-tree at 0x4000. > > > > QEMU is not the only platform where the prior boot stage supplies the > > device-tree which is to be used for booting the operating system. Any > > platform using TF-A or OpenSBI can be setup in this way. > > > > Generally it would be preferable that the prior boot stage provides the > > device-tree. But unfortunately Linux is not always backwards compatible. > > > > Don't expect the device-tree of the prior stage to contain any U-Boot > > specific quirks. > > This is my concern. I think every board in U-Boot needs at least a > basic DT, even if only a skeleton, so we have places to put things. > Any packaging at all needs a binman node, for example. U-Boot also has > its own options for certain things. Most of the options are for SPL though, and these platforms don't have that as there is other firmware that does the low-level bringup. And these platforms typically don't need packaging. I
Re: Usage of device-tree for blobs
Hi Mark, On Thu, 26 Aug 2021 at 06:55, Mark Kettenis wrote: > > > From: Simon Glass > > Date: Wed, 25 Aug 2021 21:15:00 -0600 > > > > Hi Heinrich, > > > > On Wed, 25 Aug 2021 at 02:05, Heinrich Schuchardt > > wrote: > > > > > > Hello Simon, > > > > > > some boards like qemu-riscv64_defconfig do not use any device-tree at > > > build time. A device-tree is only supplied at runtime by the prior boot > > > stage (CONFIG_OF_PRIOR_STAGE=y). > > > > > > In doc/develop/devicetree/intro.rst you suggest to put binary blobs into > > > the device-tree. > > > > > > Could you, please, update the documentation to explain how adding blobs > > > to the device-tree works in the different scenarios depending on the > > > values of: > > > > > > CONFIG_OF_EMBED > > > CONFIG_OF_SEPARATE > > > CONFIG_OF_BOARD > > > CONFIG_OF_HOSTFILE > > > CONFIG_OF_PRIOR_STAGE > > > > > > It would be especially important to understand how one can develop a > > > board independent feature which works for all of these settings. > > > > OK I will take a crack at this tomorrow. > > > > But I think there is a disconnect here, since the only options that > > matter within U-Boot are OF_SEPARATE and OF_HOSTFILE, which both use a > > u-boot.dtb file. There is nothing tricky here. > > > > The OF_BOARD business is for when the board does special things. > > Presumably signing will do special things too. We cannot really know > > what those things are because the board as 'opted out' of the standard > > options. > > > > > > > > Please, describe CONFIG_OF_PRIOR_STAGE in > > > doc/develop/devicetree/control.rst. > > > > So I'm not allowed to delete that option? :-) It seems to me to be > > extremely sparse on documentation. We need an explanation of what it > > means and what effect it has on the build system, U-Boot and some > > discussion of how qemu works. It seems to have been added as part of > > an unrelated broadcom commit. The tags were incorrect so I doubt > > anyone noticed it. Since then it has apparently proved useful > > elsewhere, but no one has added more docs. > > > > So perhaps you can help me with my doc by explaining how > > OF_PRIOR_STAGE works in qmeu, why the DT in U-Boot cannot be used, and > > which project actually hosts the DT that qemu provides? Armed with > > that information, I might be able to make sense of it all. > > Well, QEMU allows configuration of (emulated/paravirtualised) devices > from the command line. So it is pretty much impossible for U-Boot to > provide a DT that matches that configuration without adding a lot of > code the dynamically build it from some sort of configuration > specification provided by QEMU to U-Boot. But at that point QEMU > might as well provide the DT itself, don't you agree? Don't these devices have a DT in U-Boot? Is qemu enabling them (status = "okay") or actually inventing them?! > > Anyway, replying to this thread since for the Apple M1 support that > I'm working on we're in a somewhat similar situation. The Asahi Linux > project has implemented m1n1 as a bootloader and plans to use U-Boot > as a "payload" to implement booting a standard Linux distribution (and > other OSes). In this scenario m1n1 actually provides the DT since it > parses Apple's version of the DT (which isn't in the standard DT > format) and adds/updates certain properties that depend on the actual > hardware it is running on. Yes I see. Still I'd like a basic DT in U-Boot, even if just for discoverability. > > For this code I use CONFIG_OF_BOARD and implement > board_fdt_blob_setup() to simple return a pointer to the DT that m1n1 > set up. But that seems to be exactly what CONFIG_OF_PRIOR_STAGE > does... Right, and neither is really documented to the level that people can understand the purpose. They just come across as hacks to me. I'd like to see an API for passing a info (including DT) to U-Boot. I think riscv has it? I suggest we use a register that points to a bloblist. Regards, Simon
Re: Usage of device-tree for blobs
Hi Heinrich, On Thu, 26 Aug 2021 at 01:10, Heinrich Schuchardt wrote: > > On 8/26/21 5:15 AM, Simon Glass wrote: > > Hi Heinrich, > > > > On Wed, 25 Aug 2021 at 02:05, Heinrich Schuchardt > > wrote: > >> > >> Hello Simon, > >> > >> some boards like qemu-riscv64_defconfig do not use any device-tree at > >> build time. A device-tree is only supplied at runtime by the prior boot > >> stage (CONFIG_OF_PRIOR_STAGE=y). > >> > >> In doc/develop/devicetree/intro.rst you suggest to put binary blobs into > >> the device-tree. > >> > >> Could you, please, update the documentation to explain how adding blobs > >> to the device-tree works in the different scenarios depending on the > >> values of: > >> > >> CONFIG_OF_EMBED > >> CONFIG_OF_SEPARATE > >> CONFIG_OF_BOARD > >> CONFIG_OF_HOSTFILE > >> CONFIG_OF_PRIOR_STAGE > >> > >> It would be especially important to understand how one can develop a > >> board independent feature which works for all of these settings. > > > > OK I will take a crack at this tomorrow. > > > > But I think there is a disconnect here, since the only options that > > matter within U-Boot are OF_SEPARATE and OF_HOSTFILE, which both use a > > u-boot.dtb file. There is nothing tricky here. > > The following boards use OF_PRIO_STAGE: > > * QEMU > * bcm7260_defconfig > * bcm7445_defconfig > * ae350_rv32_defconfig > * ae350_rv32_spl_defconfig > * ae350_rv64_defconfig > * ae350_rv64_spl_defconfig Most of these seem OK as they have an in-tree DT. But the arm and riscv qemus and the bcm builds do not: bcm7260_defconfig bcm7445_defconfig configs/qemu_arm64_defconfig configs/qemu_arm_defconfig configs/qemu-ppce500_defconfig configs/qemu-riscv32_defconfig configs/qemu-riscv32_smode_defconfig configs/qemu-riscv64_defconfig configs/qemu-riscv64_smode_defconfig I think we are going to have to ban this. We are not really testing the build at all, and it presumably depends on the version of qemu that is used. It's OK to provide the DT to U-Boot as one flow, but not to completely drop it from the tree. Where is the qemu source code that creates these DTs? > > > > > The OF_BOARD business is for when the board does special things. > > Presumably signing will do special things too. We cannot really know > > what those things are because the board as 'opted out' of the standard > > options. > > > >> > >> Please, describe CONFIG_OF_PRIOR_STAGE in > >> doc/develop/devicetree/control.rst. > > > > So I'm not allowed to delete that option? :-) > > No. > > > It seems to me to be > > extremely sparse on documentation. We need an explanation of what it > > means and what effect it has on the build system, U-Boot and some > > discussion of how qemu works. It seems to have been added as part of > > an unrelated broadcom commit. The tags were incorrect so I doubt > > anyone noticed it. Since then it has apparently proved useful > > elsewhere, but no one has added more docs. > > > > So perhaps you can help me with my doc by explaining how > > OF_PRIOR_STAGE works in qmeu, why the DT in U-Boot cannot be used, and > > which project actually hosts the DT that qemu provides? Armed with > > that information, I might be able to make sense of it all. > > The DT provided by QEMU is not hosted anywhere. It is generated on the > fly by QEMU in dependence of the command line arguments that are used > for calling QEMU. The project is hosted at > > https://github.com/qemu/qemu. 404 on that. Do you have a link to the code that actually generates the DT? > > On RISC-V the address of the device-tree of the prior bootstage is > provided in register t0. > > On ARM platforms QEMU places the device-tree at 0x4000. > > QEMU is not the only platform where the prior boot stage supplies the > device-tree which is to be used for booting the operating system. Any > platform using TF-A or OpenSBI can be setup in this way. > > Generally it would be preferable that the prior boot stage provides the > device-tree. But unfortunately Linux is not always backwards compatible. > > Don't expect the device-tree of the prior stage to contain any U-Boot > specific quirks. This is my concern. I think every board in U-Boot needs at least a basic DT, even if only a skeleton, so we have places to put things. Any packaging at all needs a binman node, for example. U-Boot also has its own options for certain things. Also, where does the environment come from on these boards? Having the env in one tree and the DT in another must make things very interesting. Regards, Simon
Re: imx8mm memory env in U-Boot
On Thu, Aug 26, 2021 at 09:39:20AM -0700, Tim Harvey wrote: > Greetings, > > I'm trying to understand what the best memory usage is in U-Boot for > IMX8M boards for generic distro configs such as: loadaddr, > kernel_addr_r, fdt_addr_r, ramdisk_addr, scriptaddr. > > My understanding is that the following is a good rule of thumb: > loadaddr = DDR start + 32MB (as FIT images may load kernel at DDR > start; but this only allows for a 32MB kernel) > kernel_addr_r = $loadaddr > fdt_addr_r = $kernel_addr_r + 128MB (allows you up to 128MB for your > kernel; handy if you want a kernel with internal ramdisk) > ramdisk_addr = fdt_addr_r + 512KB (512KB should be plenty for a dt) > scriptaddr = $loadaddr Missing from the list here is bootm_size, so that we make sure everything that does need relocation is relocated within a specific size range. Where much of this comes from is (or should be) the huge comment in ti_armv7_common.h that's based off of the Linux kernel arm "booting" document (now converted to rST): /* * We setup defaults based on constraints from the Linux kernel, which should * also be safe elsewhere. We have the default load at 32MB into DDR (for * the kernel), FDT above 128MB (the maximum location for the end of the * kernel), and the ramdisk 512KB above that (allowing for hopefully never * seen large trees). We say all of this must be within the first 256MB * as that will normally be within the kernel lowmem and thus visible via * bootm_size and we only run on platforms with 256MB or more of memory. * * As a temporary storage for DTBO blobs (which should be applied into DTB * blob), we use the location 15.5 MB above the ramdisk. If someone wants to * use ramdisk bigger than 15.5 MB, then DTBO can be loaded and applied to DTB * blob before loading the ramdisk, as DTBO location is only used as a temporary * storage, and can be re-used after 'fdt apply' command is done. */ At this point, re-reading and referencing both: https://www.kernel.org/doc/Documentation/arm64/booting.rst https://www.kernel.org/doc/Documentation/arm/Booting would be good, and note that there's not currently a similar document for RISC-V, they often follow the same guidelines. And I know you're talking about imx8 specifically right now but due to the copy/paste nature of these kinds of values, I like to err on the side of maximal safety. Which means that we should bump the DTB size to 2MB, per arm64. It also doesn't cover kernel_comp_addr_r / kernel_comp_size for automatic decompression of Image files, but should. Note that I believe (but would have to think on and re-read a bunch of stuff to be sure), it's not that saying the kernel address is at 32MB from the start of memory limits us to 32MB, but that it makes life easier all around. > Looking at the various imx8mm boards upstream they are kind of all > over the place but do follow some patterns likely due to some of us > just going with what prior boards used. > > While I'm at it I've encountered a couple other questions: > - why on IMX8MM is CONFIG_LOADADDR is 0x4048 when DDR starts at > 0x4000. Why the 4608KB offset? > - what is CONFIG_SYS_INIT_RAM_SIZE? Most boards are setting this to > 2MB but a couple (cl-iot-gate/phycore) set it to 512KB I feel like it's pretty likely CONFIG_SYS_INIT_RAM_SIZE has been copy-pasted around as part of setting CONFIG_SYS_INIT_SP_OFFSET which is unused. A lot of unused (outside of m68k / PowerPC generally) options in that area. > - what are people using for the load address for the kernel within FIT > images? I expect start of DDR is appropriate (0x4000) however for > whatever reason I've been using 0x4020. This plays into the env as > you can't overlap where you loaded the FIT image with where you told > the FIT image to relocate the kernel to. Getting some documentation under doc/ about both the environment variables and optimal FIT layout would be good. Since we're talking about arm64 here (but this is true for RISC-V too, same header). Reading over booti_setup(), the entry point we set is (so long as 2MB aligned) where we relocate to. So the 0x4020 would be base + 2MB, and there were points in history on arm64 where it had to be at that offset, I believe. -- Tom signature.asc Description: PGP signature
Re: U-Boot ECDSA Software Implementation Status
Hi Ryan, I'm only aware of the work that Tim has published. I don't think anyone is actively working on it. Alex On 8/26/21 9:00 AM, Pabis, Ryan wrote: I see that Tim was working to add a non-platform specific implementation of the ECDSA algorithm to u-boot back in February. I would like to use this feature as well and was wondering if this work has been completed and where I can find the patch. Thanks, Ryan
imx8mm memory env in U-Boot
Greetings, I'm trying to understand what the best memory usage is in U-Boot for IMX8M boards for generic distro configs such as: loadaddr, kernel_addr_r, fdt_addr_r, ramdisk_addr, scriptaddr. My understanding is that the following is a good rule of thumb: loadaddr = DDR start + 32MB (as FIT images may load kernel at DDR start; but this only allows for a 32MB kernel) kernel_addr_r = $loadaddr fdt_addr_r = $kernel_addr_r + 128MB (allows you up to 128MB for your kernel; handy if you want a kernel with internal ramdisk) ramdisk_addr = fdt_addr_r + 512KB (512KB should be plenty for a dt) scriptaddr = $loadaddr Looking at the various imx8mm boards upstream they are kind of all over the place but do follow some patterns likely due to some of us just going with what prior boards used. While I'm at it I've encountered a couple other questions: - why on IMX8MM is CONFIG_LOADADDR is 0x4048 when DDR starts at 0x4000. Why the 4608KB offset? - what is CONFIG_SYS_INIT_RAM_SIZE? Most boards are setting this to 2MB but a couple (cl-iot-gate/phycore) set it to 512KB - what are people using for the load address for the kernel within FIT images? I expect start of DDR is appropriate (0x4000) however for whatever reason I've been using 0x4020. This plays into the env as you can't overlap where you loaded the FIT image with where you told the FIT image to relocate the kernel to. Best regards, Tim
Re: incompatible device trees between u-boot and linux
On Thu, Aug 26, 2021 at 09:35:12AM +0200, Michael Walle wrote: > Am 2021-08-26 01:03, schrieb Vladimir Oltean: > > On Wed, Aug 25, 2021 at 04:09:50PM -0400, Tom Rini wrote: > > In any case, it doesn't sound absurd at all, with a bit of passion it > > could be done on all Layerscapes. I would be absolutely glad to help on > > the Ethernet / DSA side of things (which I believe is the reason why > > Michael summoned me into this thread), > > ;) and because I thought you might be interested in the answer to the > initial question. After all, you also worked on the device trees in linux > and u-boot. > > > but I don't believe that's where > > the problem is right now. When I added the DM_DSA uclass to U-Boot I did > > my best to pick a reasonable subset of Linux DSA, and with compatible > > device tree bindings. Also maintaining the Linux side of things, I did > > provide feedback to Tim Harvey for the Microchip KSZ switches as to > > what is the subset supported by U-Boot that would also be DT-compatible > > with Linux. If it turns out that I failed at that, I am willing to > > rework what we have. > > I started to convert the u-boot device tree yesterday - and it doesn't > look too bad for now. I was already able to copy the kernel soc dtsi > and u-boot is still booting and working. > > Theres still one catch at the moment, AFAIK in linux you can put the > PHYs either in the mdio controller node or in a "mdio" subnode within > the ethernet controller node. I'm not sure wether the latter works in > u-boot, but [1] looks promising. At least, linux dtbs are using the > mdio subnodes and u-boot put the phys into the mdio controller node. Please change Linux for that, move the PHYs from the per-port MDIO node to the PF 3 central MDIO controller node. Due to hardware reasons, the per-port MDIO controller registers are in fact de-featured and should be hidden from new LS1028A reference manuals. > Maybe sharing the device tree between linux and u-boot isn't that hard > for the ls1028a after all and its just that nobody did it for now. > Renaming the reference here and there and introducing the linux > compatible strings may do it. I'll come back to you if there are > problems with ethernet (or DSA). > > -michael > > > I have been known on a few occasions to say "U-Boot does not parse this > > part of the device tree, you can just strip it away", but I will keep my > > mouth shut from now on. > > [1] https://lists.denx.de/pipermail/u-boot/2020-May/410169.html
Re: [RFC PATCH v1 2/5] arm64: dts: imx8mm: add common -binman.dtsi
Hi Tim On Thu, 2021-08-26 at 07:57 -0700, Tim Harvey wrote: > ... > Marcel, > > This would break imx8mm-venice. Sure, that's why I made it an RFC. I was just missing some context. > The of-list, @fdt-SEQ and @config-SEQ are required to support > automatic generation of fdt and config nodes when you have multiple > device-trees. See tools/binman/entries.rst for details. Thanks for that pointer, that explains a lot. > Currently, imx8mm-venice is the only board that uses multiple dtbs in > CONFIG_OF_LIST so this would be the only board broken by your patch. > I'm curious if just using the of-list, @fdt-SEQ and @config-SEQ as > above works fine for boards with a single dtb? Otherwise I suppose > boards like mine can include your common "imx8mm-binman.dtsi" but will > have to leave the node override. Yes, let me investigate further and find a suitable solution. Expect a v2 with that shortly. > Best regards, > > Tim Cheers Marcel
[PATCH v2 3/3] configs: j72*_evm: Define the buffer sizes for dfu
On J721e R5 SPL, dfu buffer for loading sysfw.itb image gets allocated before DRAM gets initialized. So, the buffer gets allocated in MCU L3 RAM. The current buffer size to be allocated is 256KB and the available total heap memory is 0x7 (448KB). This leads to NOMEM errors during allocation. In other cases when constraints such as above are not present fix the size of buffers to the sector size in OSPI for proper functioning. Also, if CONFIG_SYS_DFU_DATA_BUF_SIZE is defined and CONFIG_SYS_DFU_MAX_FILE_SIZE is not defined then the max file size for dfu transfer is defined as CONFIG_SYS_DFU_DATA_BUF_SIZE. Fix these by setting appropriate buffer sizes in their respective defconfig files and defining the max file size as 8 MB which is the default dfu buffer size. Signed-off-by: Aswath Govindraju --- configs/j7200_evm_a72_defconfig | 2 ++ configs/j7200_evm_r5_defconfig | 1 + configs/j721e_evm_a72_defconfig | 2 ++ configs/j721e_evm_r5_defconfig | 1 + include/configs/j721e_evm.h | 2 -- 5 files changed, 6 insertions(+), 2 deletions(-) diff --git a/configs/j7200_evm_a72_defconfig b/configs/j7200_evm_a72_defconfig index 6fc7cabd6035..a449705a4055 100644 --- a/configs/j7200_evm_a72_defconfig +++ b/configs/j7200_evm_a72_defconfig @@ -97,6 +97,8 @@ CONFIG_CLK=y CONFIG_SPL_CLK=y CONFIG_CLK_CCF=y CONFIG_CLK_TI_SCI=y +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x4 +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x80 CONFIG_DFU_MMC=y CONFIG_DFU_RAM=y CONFIG_DFU_SF=y diff --git a/configs/j7200_evm_r5_defconfig b/configs/j7200_evm_r5_defconfig index f9fe41a434ab..b12a6414ce0e 100644 --- a/configs/j7200_evm_r5_defconfig +++ b/configs/j7200_evm_r5_defconfig @@ -53,6 +53,7 @@ CONFIG_SPL_SPI_LOAD=y CONFIG_SYS_SPI_U_BOOT_OFFS=0x8 CONFIG_SPL_USB_GADGET=y CONFIG_SPL_DFU=y +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x4 CONFIG_SPL_YMODEM_SUPPORT=y CONFIG_HUSH_PARSER=y CONFIG_CMD_DFU=y diff --git a/configs/j721e_evm_a72_defconfig b/configs/j721e_evm_a72_defconfig index d01bacdea19f..ea83fec69f1c 100644 --- a/configs/j721e_evm_a72_defconfig +++ b/configs/j721e_evm_a72_defconfig @@ -96,6 +96,8 @@ CONFIG_CLK_TI_SCI=y CONFIG_DFU_MMC=y CONFIG_DFU_RAM=y CONFIG_DFU_SF=y +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x2 +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x80 CONFIG_DMA_CHANNELS=y CONFIG_TI_K3_NAVSS_UDMA=y CONFIG_USB_FUNCTION_FASTBOOT=y diff --git a/configs/j721e_evm_r5_defconfig b/configs/j721e_evm_r5_defconfig index e8841ba953cc..cd72d27b6392 100644 --- a/configs/j721e_evm_r5_defconfig +++ b/configs/j721e_evm_r5_defconfig @@ -51,6 +51,7 @@ CONFIG_SPL_SPI_LOAD=y CONFIG_SYS_SPI_U_BOOT_OFFS=0x8 CONFIG_SPL_USB_GADGET=y CONFIG_SPL_DFU=y +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000 CONFIG_SPL_YMODEM_SUPPORT=y CONFIG_HUSH_PARSER=y CONFIG_CMD_DFU=y diff --git a/include/configs/j721e_evm.h b/include/configs/j721e_evm.h index 18b80ef8ce7d..10555d1a6ca7 100644 --- a/include/configs/j721e_evm.h +++ b/include/configs/j721e_evm.h @@ -155,9 +155,7 @@ #define EXTRA_ENV_CONFIG_MAIN_CPSW0_QSGMII_PHY #endif -/* set default dfu_bufsiz to 128KB (sector size of OSPI) */ #define EXTRA_ENV_DFUARGS \ - "dfu_bufsiz=0x2\0" \ DFU_ALT_INFO_MMC \ DFU_ALT_INFO_EMMC \ DFU_ALT_INFO_RAM \ -- 2.17.1
[PATCH v2 2/3] environment: ti: k3_dfu: Increase the size allocated for bootloader images in dfu_alt_info_ram
The size of u-boot.img is above 1MB and that of tispl.bin is close to 1MB, in case of j721e. Therefore, increase the sizes allocated for tispl.bin and u-boot.img to 2 MB and 4 MB respectively, in dfu_alt_info_ram environment variable. Signed-off-by: Aswath Govindraju --- include/environment/ti/k3_dfu.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/environment/ti/k3_dfu.h b/include/environment/ti/k3_dfu.h index 2f503b8de880..a16a3adecaf9 100644 --- a/include/environment/ti/k3_dfu.h +++ b/include/environment/ti/k3_dfu.h @@ -40,7 +40,7 @@ #define DFU_ALT_INFO_RAM \ "dfu_alt_info_ram=" \ - "tispl.bin ram 0x8008 0x10;" \ - "u-boot.img ram 0x8100 0x10\0" \ + "tispl.bin ram 0x8008 0x20;" \ + "u-boot.img ram 0x8100 0x40\0" \ #endif /* __TI_DFU_H */ -- 2.17.1
[PATCH v2 1/3] arm: dts: k3-j721e-r5-*.dts: Fix clock-names property in the usb0 instance
In the cdns3 usb driver, the clock name looked for is ref. Therefore, fix the clock-names property in usb0 instance for proper initialization of cdns3 usb gadget driver. Signed-off-by: Aswath Govindraju --- arch/arm/dts/k3-j721e-r5-common-proc-board.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/dts/k3-j721e-r5-common-proc-board.dts b/arch/arm/dts/k3-j721e-r5-common-proc-board.dts index a12607dc2f75..4b2362a5dd05 100644 --- a/arch/arm/dts/k3-j721e-r5-common-proc-board.dts +++ b/arch/arm/dts/k3-j721e-r5-common-proc-board.dts @@ -280,7 +280,7 @@ /delete-property/ assigned-clocks; /delete-property/ assigned-clock-parents; clocks = <_19_2mhz>; - clock-names = "usb2_refclk"; + clock-names = "ref"; pinctrl-names = "default"; pinctrl-0 = <_usbss0_pins_default>; ti,vbus-divider; -- 2.17.1
[PATCH v2 0/3] J7200/J721E: Fix USB DFU Boot mode
The following series of patches fix USB DFU Boot mode in J7200 and J721E SoC's. changes since v1: - synced up the size allocated for tispl.bin and u-boot.img in dfu ram env variable with that of OSPI and eMMC dfu env variables Aswath Govindraju (3): arm: dts: k3-j721e-r5-*.dts: Fix clock-names property in the usb0 instance environment: ti: k3_dfu: Increase the size allocated for bootloader images in dfu_alt_info_ram configs: j72*_evm: Define the buffer sizes for dfu arch/arm/dts/k3-j721e-r5-common-proc-board.dts | 2 +- configs/j7200_evm_a72_defconfig| 2 ++ configs/j7200_evm_r5_defconfig | 1 + configs/j721e_evm_a72_defconfig| 2 ++ configs/j721e_evm_r5_defconfig | 1 + include/configs/j721e_evm.h| 2 -- include/environment/ti/k3_dfu.h| 4 ++-- 7 files changed, 9 insertions(+), 5 deletions(-) -- 2.17.1
[PATCH] Finish converting CONFIG_SYS_CACHELINE_SIZE to Kconfig
We move the SYS_CACHE_SHIFT_N options from arch/arm/Kconfig to arch/Kconfig, and introduce SYS_CACHE_SHIFT_4 to provide a size of 16. Introduce select statements for other architectures based on current usage. For MIPS, we take the existing arch-specific symbol and migrate to the generic symbol. This lets us remove a little bit of otherwise unused code. Cc: Alexey Brodkin Cc: Anup Patel Cc: Atish Patra Cc: Bin Meng Cc: Daniel Schwierzeck Cc: Leo Cc: Palmer Dabbelt Cc: Paul Walmsley Cc: Rick Chen Cc: Sean Anderson Cc: Simon Glass Signed-off-by: Tom Rini --- I'm Cc'ing a bunch of RISC-V folks since that's where I'm least confident and just put it per-board for now. --- arch/Kconfig | 25 + arch/arc/include/asm/cache.h | 3 --- arch/arm/Kconfig | 15 --- arch/mips/Kconfig | 26 +++--- arch/mips/include/asm/cache.h | 12 +--- arch/mips/mach-bmips/Kconfig | 20 ++-- arch/mips/mach-mtmips/Kconfig | 4 ++-- arch/mips/mach-pic32/Kconfig | 2 +- arch/powerpc/cpu/mpc83xx/Kconfig | 6 ++ arch/powerpc/cpu/mpc85xx/Kconfig | 15 +++ arch/powerpc/cpu/mpc8xx/Kconfig| 2 ++ arch/powerpc/include/asm/cache.h | 7 --- arch/riscv/Kconfig | 2 ++ arch/sandbox/include/asm/cache.h | 1 - arch/x86/include/asm/cache.h | 7 +-- include/configs/M5208EVBE.h| 1 - include/configs/M5235EVB.h | 1 - include/configs/M5249EVB.h | 1 - include/configs/M5253DEMO.h| 1 - include/configs/M5272C3.h | 1 - include/configs/M5275EVB.h | 1 - include/configs/M5282EVB.h | 1 - include/configs/M53017EVB.h| 1 - include/configs/M5329EVB.h | 1 - include/configs/M5373EVB.h | 1 - include/configs/amcore.h | 1 - include/configs/astro_mcf5373l.h | 1 - include/configs/cobra5272.h| 1 - include/configs/eb_cpu5282.h | 1 - include/configs/mx7ulp_evk.h | 2 -- include/configs/rk3188_common.h| 2 -- include/configs/rk3368_common.h| 2 -- include/configs/sifive-unmatched.h | 2 -- include/configs/sipeed-maix.h | 1 - include/configs/stmark2.h | 1 - 35 files changed, 68 insertions(+), 103 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index b6f9e177b645..25f4a15b19f9 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -7,6 +7,27 @@ config HAVE_ARCH_IOREMAP config NEEDS_MANUAL_RELOC bool +config SYS_CACHE_SHIFT_4 + bool + +config SYS_CACHE_SHIFT_5 + bool + +config SYS_CACHE_SHIFT_6 + bool + +config SYS_CACHE_SHIFT_7 + bool + +config SYS_CACHELINE_SIZE + int + default 128 if SYS_CACHE_SHIFT_7 + default 64 if SYS_CACHE_SHIFT_6 + default 32 if SYS_CACHE_SHIFT_5 + default 16 if SYS_CACHE_SHIFT_4 + # Fall-back for MIPS + default 32 if MIPS + config LINKER_LIST_ALIGN int default 32 if SANDBOX @@ -29,6 +50,7 @@ config ARC select DM select HAVE_PRIVATE_LIBGCC select SUPPORT_OF_CONTROL + select SYS_CACHE_SHIFT_7 select TIMER config ARM @@ -44,6 +66,7 @@ config M68K select NEEDS_MANUAL_RELOC select SYS_BOOT_GET_CMDLINE select SYS_BOOT_GET_KBD + select SYS_CACHE_SHIFT_4 select SUPPORT_OF_CONTROL config MICROBLAZE @@ -122,6 +145,7 @@ config SANDBOX select SPI select SUPPORT_OF_CONTROL select SYSRESET_CMD_POWEROFF + select SYS_CACHE_SHIFT_4 select IRQ select SUPPORT_EXTENSION_SCAN imply BITREVERSE @@ -188,6 +212,7 @@ config X86 select OF_CONTROL select PCI select SUPPORT_OF_CONTROL + select SYS_CACHE_SHIFT_6 select TIMER select USE_PRIVATE_LIBGCC select X86_TSC_TIMER diff --git a/arch/arc/include/asm/cache.h b/arch/arc/include/asm/cache.h index ab61846b5ab9..a48e1aec6889 100644 --- a/arch/arc/include/asm/cache.h +++ b/arch/arc/include/asm/cache.h @@ -16,9 +16,6 @@ */ #define ARCH_DMA_MINALIGN 128 -/* CONFIG_SYS_CACHELINE_SIZE is used a lot in drivers */ -#define CONFIG_SYS_CACHELINE_SIZE ARCH_DMA_MINALIGN - #if defined(ARC_MMU_ABSENT) #define CONFIG_ARC_MMU_VER 0 #elif defined(CONFIG_ARC_MMU_V2) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index d692139199c4..e478b9088a09 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -332,21 +332,6 @@ config SYS_ARM_ARCH default 4 if CPU_SA1100 default 8 if ARM64 -config SYS_CACHE_SHIFT_5 - bool - -config SYS_CACHE_SHIFT_6 - bool - -config SYS_CACHE_SHIFT_7 - bool - -config SYS_CACHELINE_SIZE - int - default 128 if SYS_CACHE_SHIFT_7 - default 64 if SYS_CACHE_SHIFT_6 - default 32 if SYS_CACHE_SHIFT_5 - choice prompt "Select the ARM data write cache policy"
[PATCH v2 3/4] riscv: sifive: unleashed: Add genimage config files
This adds genimage [1] config files for generating SD card and spi-nor
images, which can be programmed to an SD card or SPI flash and boot
from there.
The same images will be used for U-Boot CI testing for this board.
[1] https://github.com/pengutronix/genimage
Signed-off-by: Bin Meng
---
(no changes since v1)
board/sifive/unleashed/genimage_sdcard.cfg | 19 +++
board/sifive/unleashed/genimage_spi-nor.cfg | 19 +++
2 files changed, 38 insertions(+)
create mode 100644 board/sifive/unleashed/genimage_sdcard.cfg
create mode 100644 board/sifive/unleashed/genimage_spi-nor.cfg
diff --git a/board/sifive/unleashed/genimage_sdcard.cfg
b/board/sifive/unleashed/genimage_sdcard.cfg
new file mode 100644
index 00..91c53bf855
--- /dev/null
+++ b/board/sifive/unleashed/genimage_sdcard.cfg
@@ -0,0 +1,19 @@
+image sdcard.img {
+ size = 128M
+
+ hdimage {
+ gpt = true
+ }
+
+ partition u-boot-spl {
+ image = "u-boot-spl.bin"
+ offset = 17K
+ partition-type-uuid = 5B193300-FC78-40CD-8002-E86C45580B47
+ }
+
+ partition u-boot {
+ image = "u-boot.itb"
+ offset = 1041K
+ partition-type-uuid = 2E54B353-1271-4842-806F-E436D6AF6985
+ }
+}
diff --git a/board/sifive/unleashed/genimage_spi-nor.cfg
b/board/sifive/unleashed/genimage_spi-nor.cfg
new file mode 100644
index 00..2e5d89bfe8
--- /dev/null
+++ b/board/sifive/unleashed/genimage_spi-nor.cfg
@@ -0,0 +1,19 @@
+image spi-nor.img {
+ size = 32M
+
+ hdimage {
+ gpt = true
+ }
+
+ partition u-boot-spl {
+ image = "u-boot-spl.bin"
+ offset = 20K
+ partition-type-uuid = 5B193300-FC78-40CD-8002-E86C45580B47
+ }
+
+ partition u-boot {
+ image = "u-boot.itb"
+ offset = 1044K
+ partition-type-uuid = 2E54B353-1271-4842-806F-E436D6AF6985
+ }
+}
--
2.25.1
[PATCH v2 4/4] azure/gitlab: Add tests for SiFive Unleashed board
This adds CI tests for SiFive Unleashed board.
QEMU supports booting exact the same images as used on the real
hardware out of the box, that U-Boot SPL loads U-Boot proper
from either an SD card or the SPI NOR flash, hence we can easily
set up CI to cover these 2 boot flows of SiFive Unleashed board.
With this, now we can have regression testing of mmc-spi-slot and
sifive spi drivers, as well as mmc and spi-nor subsystems.
Signed-off-by: Bin Meng
---
(no changes since v1)
.azure-pipelines.yml | 20 +++-
.gitlab-ci.yml | 26 +-
2 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 15507a7357..2e9c1fb4fd 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -254,6 +254,12 @@ jobs:
r2dplus_tulip:
TEST_PY_BD: "r2dplus"
TEST_PY_ID: "--id tulip_qemu"
+sifive_unleashed_sdcard:
+ TEST_PY_BD: "sifive_unleashed"
+ TEST_PY_ID: "--id sdcard_qemu"
+sifive_unleashed_spi-nor:
+ TEST_PY_BD: "sifive_unleashed"
+ TEST_PY_ID: "--id spi-nor_qemu"
xilinx_zynq_virt:
TEST_PY_BD: "xilinx_zynq_virt"
TEST_PY_ID: "--id qemu"
@@ -289,7 +295,7 @@ jobs:
wget -O -
https://github.com/riscv/opensbi/releases/download/v0.9/opensbi-0.9-rv-bin.tar.xz
| tar -C /tmp -xJ;
export
OPENSBI=/tmp/opensbi-0.9-rv-bin/share/opensbi/ilp32/generic/firmware/fw_dynamic.bin;
fi
- if [[ "${TEST_PY_BD}" == "qemu-riscv64_spl" ]]; then
+ if [[ "${TEST_PY_BD}" == "qemu-riscv64_spl" ]] || [[ "${TEST_PY_BD}"
== "sifive_unleashed" ]]; then
wget -O -
https://github.com/riscv/opensbi/releases/download/v0.9/opensbi-0.9-rv-bin.tar.xz
| tar -C /tmp -xJ;
export
OPENSBI=/tmp/opensbi-0.9-rv-bin/share/opensbi/lp64/generic/firmware/fw_dynamic.bin;
fi
@@ -302,6 +308,18 @@ jobs:
cp /opt/grub/grubriscv64.efi
${UBOOT_TRAVIS_BUILD_DIR}/grub_riscv64.efi
cp /opt/grub/grubaa64.efi ${UBOOT_TRAVIS_BUILD_DIR}/grub_arm64.efi
cp /opt/grub/grubarm.efi ${UBOOT_TRAVIS_BUILD_DIR}/grub_arm.efi
+ # create sdcard / spi-nor images for sifive unleashed using genimage
+ if [[ "${TEST_PY_BD}" == "sifive_unleashed" ]]; then
+ mkdir -p root;
+ cp ${UBOOT_TRAVIS_BUILD_DIR}/spl/u-boot-spl.bin .;
+ cp ${UBOOT_TRAVIS_BUILD_DIR}/u-boot.itb .;
+ rm -rf tmp;
+ genimage --inputpath . --config
board/sifive/unleashed/genimage_sdcard.cfg;
+ cp images/sdcard.img ${UBOOT_TRAVIS_BUILD_DIR}/;
+ rm -rf tmp;
+ genimage --inputpath . --config
board/sifive/unleashed/genimage_spi-nor.cfg;
+ cp images/spi-nor.img ${UBOOT_TRAVIS_BUILD_DIR}/;
+ fi
virtualenv -p /usr/bin/python3 /tmp/venv
. /tmp/venv/bin/activate
pip install -r test/py/requirements.txt
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ffdeaae5a8..599de6e506 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -23,7 +23,7 @@ stages:
wget -O -
https://github.com/riscv/opensbi/releases/download/v0.9/opensbi-0.9-rv-bin.tar.xz
| tar -C /tmp -xJ;
export
OPENSBI=/tmp/opensbi-0.9-rv-bin/share/opensbi/ilp32/generic/firmware/fw_dynamic.bin;
fi
-- if [[ "${TEST_PY_BD}" == "qemu-riscv64_spl" ]]; then
+- if [[ "${TEST_PY_BD}" == "qemu-riscv64_spl" ]] || [[ "${TEST_PY_BD}" ==
"sifive_unleashed" ]]; then
wget -O -
https://github.com/riscv/opensbi/releases/download/v0.9/opensbi-0.9-rv-bin.tar.xz
| tar -C /tmp -xJ;
export
OPENSBI=/tmp/opensbi-0.9-rv-bin/share/opensbi/lp64/generic/firmware/fw_dynamic.bin;
fi
@@ -40,6 +40,18 @@ stages:
- cp /opt/grub/grubriscv64.efi $UBOOT_TRAVIS_BUILD_DIR/grub_riscv64.efi
- cp /opt/grub/grubaa64.efi $UBOOT_TRAVIS_BUILD_DIR/grub_arm64.efi
- cp /opt/grub/grubarm.efi $UBOOT_TRAVIS_BUILD_DIR/grub_arm.efi
+# create sdcard / spi-nor images for sifive unleashed using genimage
+- if [[ "${TEST_PY_BD}" == "sifive_unleashed" ]]; then
+mkdir -p root;
+cp ${UBOOT_TRAVIS_BUILD_DIR}/spl/u-boot-spl.bin .;
+cp ${UBOOT_TRAVIS_BUILD_DIR}/u-boot.itb .;
+rm -rf tmp;
+genimage --inputpath . --config
board/sifive/unleashed/genimage_sdcard.cfg;
+cp images/sdcard.img ${UBOOT_TRAVIS_BUILD_DIR}/;
+rm -rf tmp;
+genimage --inputpath . --config
board/sifive/unleashed/genimage_spi-nor.cfg;
+cp images/spi-nor.img ${UBOOT_TRAVIS_BUILD_DIR}/;
+ fi
- virtualenv -p /usr/bin/python3 /tmp/venv
- . /tmp/venv/bin/activate
- pip install -r test/py/requirements.txt
@@ -317,6 +329,18 @@ r2dplus_tulip test.py:
TEST_PY_ID: "--id tulip_qemu"
<<: *buildman_and_testpy_dfn
+sifive_unleashed_sdcard test.py:
+ variables:
+TEST_PY_BD:
[PATCH v2 2/4] tools: docker: Build and install genimage
genimage [1] is a tool to create flash/disk images. This is required by some targets, e.g.: sifive_unleashed, to generate sdcard or spi-nor images for real hardware, as well as U-Boot CI testing. [1] https://github.com/pengutronix/genimage Signed-off-by: Bin Meng --- Changes in v2: - Build genimage in the /tmp directory, just like others tools/docker/Dockerfile | 9 + 1 file changed, 9 insertions(+) diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index 4d7c3c6679..dfc4c6e91b 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -60,6 +60,7 @@ RUN apt-get update && apt-get install -y \ iasl \ imagemagick \ iputils-ping \ + libconfuse-dev \ libgit2-dev \ libguestfs-tools \ liblz4-tool \ @@ -177,6 +178,14 @@ RUN git clone git://git.qemu.org/qemu.git /tmp/qemu && \ make -j$(nproc) all install && \ rm -rf /tmp/qemu +# Build genimage (required by some targets to generate disk images) +RUN wget -O - https://github.com/pengutronix/genimage/releases/download/v14/genimage-14.tar.xz | tar -C /tmp -xJ && \ + cd /tmp/genimage-14 && \ + ./configure && \ + make -j$(nproc) && \ + make install && \ + rm -rf /tmp/genimage-14 + # Create our user/group RUN echo uboot ALL=NOPASSWD: ALL > /etc/sudoers.d/uboot RUN useradd -m -U uboot -- 2.25.1
[PATCH v2 1/4] tools: docker: Bump up QEMU version to 6.1.0
At present U-Boot CI testing is still using QEMU 4.2.0 which is pretty old. Let's bump up to QEMU 6.1.0. ninja-build is added as the prerequisite required by QEMU 6.1.0. Note there is a bug in QEMU 6.1.0 Xilinx Zynq UART emulation codes. A quick fix [1] was posted on QEMU mailing list but it it too late for 6.1.0 release. Let's manually apply the bug fix on top of the v6.1.0 release tag at the time being. [1] http://patchwork.ozlabs.org/project/qemu-devel/patch/20210823020813.25192-2-bmeng...@gmail.com/ Signed-off-by: Bin Meng --- Changes in v2: - Use QEMU official git repo, and 'git am' the bug fix manually tools/docker/Dockerfile | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index 0195456dfe..4d7c3c6679 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -76,6 +76,7 @@ RUN apt-get update && apt-get install -y \ mount \ mtd-utils \ mtools \ + ninja-build \ openssl \ picocom \ parted \ @@ -166,7 +167,12 @@ RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \ RUN git clone git://git.qemu.org/qemu.git /tmp/qemu && \ cd /tmp/qemu && \ git submodule update --init dtc && \ - git checkout v4.2.0 && \ + git checkout v6.1.0 && \ + # config user.name and user.email to make 'git am' happy + git config user.name u-boot && \ + git config user.email u-b...@denx.de && \ + # manually apply the bug fix for QEMU 6.1.0 Xilinx Zynq UART emulation codes + wget -O - http://patchwork.ozlabs.org/project/qemu-devel/patch/20210823020813.25192-2-bmeng...@gmail.com/mbox/ | git am && \ ./configure --prefix=/opt/qemu --target-list="aarch64-softmmu,arm-softmmu,i386-softmmu,mips-softmmu,mips64-softmmu,mips64el-softmmu,mipsel-softmmu,ppc-softmmu,riscv32-softmmu,riscv64-softmmu,sh4-softmmu,x86_64-softmmu,xtensa-softmmu" && \ make -j$(nproc) all install && \ rm -rf /tmp/qemu -- 2.25.1
Re: [RFC PATCH v1 0/5] arm64: dts: imx8mm: add common -binman.dtsi and further clean-up
On Thu, Aug 26, 2021 at 5:27 AM Marcel Ziswiler wrote:
>
> From: Marcel Ziswiler
>
>
> With the move to using binman to generate SPL aka u-boot-spl-ddr.bin and
> U-Boot proper aka u-boot.itb every board now covers such configuration
> in its own U-Boot specific device tree include. Introduce a new common
> imx8mm-binman.dtsi which covers the common part of that configuration.
>
> The initial patch fixes an issue with intermediate binary naming for the
> imx8mm-cl-iot-gate. And subsequent patches further clean up that dtsi.
>
> This series is based on Peng's binman conversion of late [1], my Verdin
> iMX8M Mini target refresh [2], Fabio's generating a single bootable
> flash.bin again for imx8mm-evk [3], Tim's switching imx8mm_venice to
> using binman to pack images [4] and Frieder's support for Kontron
> Electronics i.MX6UL/ULL and i.MX8MM SoMs [5].
>
> This series has been run-time tested on Verdin iMX8M Mini. The other
> targets were only compile tested.
>
> Please note that for now, I left out the following intricacies of
> imx8mm-venice-u-boot.dtsi. Not quite sure what exactly those are used
> for.
>
> fit,fdt-list = "of-list";
> ...
> @fdt-SEQ {
> ...
> default = "@config-DEFAULT-SEQ";
> ...
> @config-SEQ {
> ...
> fdt = "fdt-SEQ";
>
Marcell,
The of-list, @fdt-SEQ and @config-SEQ are required to support
automatic generation of fdt and config nodes when you have multiple
device-trees. See tools/binman/entries.rst for details.
Currently imx8mm-venice is the only imx8mm board that uses
CONFIG_OF_LIST to support multiple dtbs.
Tim
Re: [RFC PATCH v1 2/5] arm64: dts: imx8mm: add common -binman.dtsi
On Thu, Aug 26, 2021 at 5:14 AM Marcel Ziswiler wrote:
>
> From: Marcel Ziswiler
>
> With the move to using binman to generate SPL aka u-boot-spl-ddr.bin and
> U-Boot proper aka u-boot.itb every board now covers such configuration
> in its own U-Boot specific device tree include. Introduce a new common
> imx8mm-binman.dtsi which covers the common part of that configuration.
>
> Signed-off-by: Marcel Ziswiler
>
> ---
>
> arch/arm/dts/imx8mm-binman.dtsi | 136 ++
> arch/arm/dts/imx8mm-cl-iot-gate-u-boot.dtsi | 126 ++--
> arch/arm/dts/imx8mm-evk-u-boot.dtsi | 124 +---
> .../dts/imx8mm-kontron-n801x-s-u-boot.dtsi| 123 +---
> arch/arm/dts/imx8mm-venice-u-boot.dtsi| 120 +---
> arch/arm/dts/imx8mm-verdin-u-boot.dtsi| 123 +---
> 6 files changed, 156 insertions(+), 596 deletions(-)
> create mode 100644 arch/arm/dts/imx8mm-binman.dtsi
>
> diff --git a/arch/arm/dts/imx8mm-binman.dtsi b/arch/arm/dts/imx8mm-binman.dtsi
> new file mode 100644
> index 000..2d98c1ef577
> --- /dev/null
> +++ b/arch/arm/dts/imx8mm-binman.dtsi
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2021 Toradex
> + */
> +
> +/ {
> + binman: binman {
> + multiple-images;
> + };
> +};
> +
> + {
> + u-boot-spl-ddr {
> + filename = "u-boot-spl-ddr.bin";
> + pad-byte = <0xff>;
> + align-size = <4>;
> + align = <4>;
> +
> + u-boot-spl {
> + align-end = <4>;
> + };
> +
> + blob_1: blob-ext@1 {
> + filename = "lpddr4_pmu_train_1d_imem.bin";
> + size = <0x8000>;
> + };
> +
> + blob_2: blob-ext@2 {
> + filename = "lpddr4_pmu_train_1d_dmem.bin";
> + size = <0x4000>;
> + };
> +
> + blob_3: blob-ext@3 {
> + filename = "lpddr4_pmu_train_2d_imem.bin";
> + size = <0x8000>;
> + };
> +
> + blob_4: blob-ext@4 {
> + filename = "lpddr4_pmu_train_2d_dmem.bin";
> + size = <0x4000>;
> + };
> + };
> +
> + spl {
> + filename = "spl.bin";
> +
> + mkimage {
> + args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e
> 0x7e1000";
> +
> + blob {
> + filename = "u-boot-spl-ddr.bin";
> + };
> + };
> + };
> +
> + itb {
> + filename = "u-boot.itb";
> +
> + fit {
> + description = "Configuration to load ATF before
> U-Boot";
> + #address-cells = <1>;
> + fit,external-offset = ;
> +
> + images {
> + uboot {
> + description = "U-Boot (64-bit)";
> + type = "standalone";
> + arch = "arm64";
> + compression = "none";
> + load = ;
> +
> + uboot_blob: blob-ext {
> + filename = "u-boot-nodtb.bin";
> + };
> + };
> +
> + atf {
> + description = "ARM Trusted Firmware";
> + type = "firmware";
> + arch = "arm64";
> + compression = "none";
> + load = <0x92>;
> + entry = <0x92>;
> +
> + atf_blob: blob-ext {
> + filename = "bl31.bin";
> + };
> + };
> +
> + binman_fip: fip {
> + description = "Trusted Firmware FIP";
> + type = "firmware";
> + arch = "arm64";
> + compression = "none";
> + load = <0x4031>;
> + };
> +
> + fdt {
> + description = "NAME";
> + type = "flat_dt";
> + compression = "none";
> +
> + uboot_fdt_blob: blob-ext {
> +
U-Boot ECDSA Software Implementation Status
I see that Tim was working to add a non-platform specific implementation of the ECDSA algorithm to u-boot back in February. I would like to use this feature as well and was wondering if this work has been completed and where I can find the patch. Thanks, Ryan
Re: [PATCH 00/28] Initial implementation of bootmethod/bootflow
Hi Mark,
> > > > > > > > > > >
[...]
> > > > > > > > > > > Well, there's "find the next stage", which is
> > > > > > > > > > > boot_targets environment
> > > > > > > > > > > variable, and then "where that next stage looks for
> > > > > > > > > > > stuff" which is
> > > > > > > > > > > OS-dependent. Sometimes the ESP grub.cfg file is just
> > > > > > > > > > > enough to tell
> > > > > > > > > > > grub to find the full grub.cfg file elsewhere, and
> > > > > > > > > > > sometimes it's a full
> > > > > > > > > > > grub.cfg file. I think Mark is talking about the former,
> > > > > > > > > > > and you've
> > > > > > > > > > > said it's not part of this series, yet, but on the TODO
> > > > > > > > > > > list.
> > > > > > > > > >
> > > > > > > > > > Right. With the current distroboot code the order of the
> > > > > > > > > > devices that
> > > > > > > > > > appears in boot_targets is determined by
> > > > > > > > > > per-board/SOC/machine config
> > > > > > > > > > files and the order isn't the same for all of them. Users
> > > > > > > > > > can change
> > > > > > > > > > the order if necessary by modifying the environment
> > > > > > > > > > variable and
> > > > > > > > > > saving the environment. And for a one-off boot from a
> > > > > > > > > > different
> > > > > > > > > > device they can simply run an appropriate boot command. The
> > > > > > > > > > boot_targets variable in particular is documented in
> > > > > > > > > > various install
> > > > > > > > > > documents so it would probably be good of the new
> > > > > > > > > > "bootmethod" code
> > > > > > > > > > would respect this variable.
> > > > > > > > > >
> > > > > > > > > > For OpenBSD I'm not really interested in the bootflow part.
> > > > > > > > > > As I
> > > > > > > > > > explained in the past, that part of the problem is solved
> > > > > > > > > > in a
> > > > > > > > > > (mostly) uniform way across platforms by the OpenBSD
> > > > > > > > > > bootloader which
> > > > > > > > > > can read an /etc/boot.conf that allows bootflow
> > > > > > > > > > customization. So as
> > > > > > > > > > long as the default of the new code still results in
> > > > > > > > > > \EFI\BOOT\BOOT{machine type short-name}.EFI being loaded
> > > > > > > > > > and run if
> > > > > > > > > > there is no U-Boot specific bootflow configured, I'm happy.
> > > > > > > > >
> > > > > > > > > Mostly the same for FreeBSD, as long as the efi
> > > > > > > > > boot.efi is
> > > > > > > > > loaded and run by default (respecting the boot_targets order)
> > > > > > > > > we will
> > > > > > > > > be fine.
> > > > > > > >
> > > > > > > > OK thanks for the info. My expectation is that
> > > > > > > > bootmethod/bootflow can
> > > > > > > > support this easily enough (it is actually simpler than distro
> > > > > > > > boot).
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > I can't speak for the other BSDs, but my impression is that
> > > > > > > > > > they are
> > > > > > > > > > pretty much in the same position. The FreeBSD bootloader
> > > > > > > > > > for example
> > > > > > > > > > supports a high-degree of "bootflow" customization and I
> > > > > > > > > > doubt that
> > > > > > > > > > taking it out of the loop is a viable option for most users.
> > > > > > > >
> > > > > > > > I think the same may happen with grub. E.g. with Ubuntu I see
> > > > > > > > quite a
> > > > > > > > bit of code in the grub.cfg file and it's not clear to me that
> > > > > > > > it can
> > > > > > > > be replaced with a 'data instead of code' approach. Still, a
> > > > > > > > valid
> > > > > > > > bootflow is simply to jump to an EFI app, which seems to be
> > > > > > > > what is
> > > > > > > > happening here. The bootflow side is really just about
> > > > > > > > describing what
> > > > > > > > to do, and this case is no different. For now I see three types
> > > > > > > > of
> > > > > > > > bootflow, PXE/syslinux, EFI boot manager and EFI app.
> > > > > > >
> > > > > > > By "EFI app", do you mean a way of booting "/efi/boot/bootXX.efi"
> > > > > > > (default file name in case that no image path is specified)?
> > > > > > >
> > > > > > > In fact, this behavior, or removable media support, is defined
> > > > > > > as part of UEFI boot manager in UEFI specification. (See section
> > > > > > > 3.5)
> > > > > > > What this means is that the boot order, including a removable
> > > > > > > media
> > > > > > > case and user-provided Boot cases, should be controlled solely
> > > > > > > by "BootOrder" variable.
> > > > > > > So the current combination of distro_bootcmd + UEFI boot manger
> > > > > > > doesn't
> > > > > > > fully comply with the specification.
> > > > > > >
> > > > > > > Even if those two cases are integrated, I don't know how
> > > > > > > "BootOrder"
> > > > > > > semantics can be preserved in your approach.
> > > > > >
> > > > > > I think the high level answer is that whereas today part of
> > > > > > distro_bootcmd (and so
[PATCH v2 1/6] efi_loader: stop recursion in efi_init_secure_state
efi_init_secure_state() calls efi_transfer_secure_state() which may delete
variable "PK" which will result in calling efi_init_secure_state() again.
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
lib/efi_loader/efi_var_common.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 3d92afe2eb..654ce81f9d 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -314,11 +314,15 @@ err:
efi_status_t efi_init_secure_state(void)
{
+ static bool lock;
enum efi_secure_mode mode = EFI_MODE_SETUP;
u8 efi_vendor_keys = 0;
efi_uintn_t size = 0;
efi_status_t ret;
+ if (lock)
+ return EFI_SUCCESS;
+
ret = efi_get_variable_int(L"PK", _global_variable_guid,
NULL, , NULL, NULL);
if (ret == EFI_BUFFER_TOO_SMALL) {
@@ -326,7 +330,9 @@ efi_status_t efi_init_secure_state(void)
mode = EFI_MODE_USER;
}
+ lock = true;
ret = efi_transfer_secure_state(mode);
+ lock = false;
if (ret != EFI_SUCCESS)
return ret;
--
2.30.2
[PATCH v2 5/6] efi_loader: writing AuditMode, DeployedMode
Writing variables AuditMode or Deployed Mode must update the secure boot
state.
Signed-off-by: Heinrich Schuchardt
---
v2:
correct variable name in lib/efi_loader/efi_variable_tee.c
---
include/efi_variable.h| 1 +
lib/efi_loader/efi_var_common.c | 2 ++
lib/efi_loader/efi_variable.c | 6 +++---
lib/efi_loader/efi_variable_tee.c | 4 +++-
4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h
index 2d97655e1f..0440d356bc 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -12,6 +12,7 @@
enum efi_auth_var_type {
EFI_AUTH_VAR_NONE = 0,
+ EFI_AUTH_MODE,
EFI_AUTH_VAR_PK,
EFI_AUTH_VAR_KEK,
EFI_AUTH_VAR_DB,
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 63ad6fea9e..6fabcfe72c 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -34,6 +34,8 @@ static const struct efi_auth_var_name_type name_type[] = {
{u"dbx", _guid_image_security_database, EFI_AUTH_VAR_DBX},
{u"dbt", _guid_image_security_database, EFI_AUTH_VAR_DBT},
{u"dbr", _guid_image_security_database, EFI_AUTH_VAR_DBR},
+ {u"AuditMode", _global_variable_guid, EFI_AUTH_MODE},
+ {u"DeployedMode", _global_variable_guid, EFI_AUTH_MODE},
};
static bool efi_secure_boot;
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index a7d305ffbc..80996d0f47 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -247,7 +247,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
return EFI_WRITE_PROTECTED;
if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
- if (var_type != EFI_AUTH_VAR_NONE)
+ if (var_type >= EFI_AUTH_VAR_PK)
return EFI_WRITE_PROTECTED;
}
@@ -268,7 +268,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
return EFI_NOT_FOUND;
}
- if (var_type != EFI_AUTH_VAR_NONE) {
+ if (var_type >= EFI_AUTH_VAR_PK) {
/* authentication is mandatory */
if (!(attributes &
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
@@ -328,7 +328,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
if (ret != EFI_SUCCESS)
return ret;
- if (var_type == EFI_AUTH_VAR_PK)
+ if (var_type == EFI_AUTH_VAR_PK || var_type == EFI_AUTH_MODE)
ret = efi_init_secure_state();
else
ret = EFI_SUCCESS;
diff --git a/lib/efi_loader/efi_variable_tee.c
b/lib/efi_loader/efi_variable_tee.c
index 51920bcb51..a6d5752045 100644
--- a/lib/efi_loader/efi_variable_tee.c
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -512,6 +512,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
efi_uintn_t payload_size;
efi_uintn_t name_size;
u8 *comm_buf = NULL;
+ enum efi_auth_var_type var_type;
bool ro;
if (!variable_name || variable_name[0] == 0 || !vendor) {
@@ -590,7 +591,8 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
if (alt_ret != EFI_SUCCESS)
goto out;
- if (!u16_strcmp(variable_name, L"PK"))
+ var_type = efi_auth_var_get_type(variable_name, vendor);
+ if (var_type == EFI_AUTH_VAR_PK || var_type == EFI_AUTH_MODE)
alt_ret = efi_init_secure_state();
out:
free(comm_buf);
--
2.30.2
[PATCH v2 3/6] efi_loader: don't load signature database from file
The UEFI specification requires that the signature database may only be
stored in tamper-resistant storage. So these variable may not be read
from an unsigned file.
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
include/efi_variable.h | 5 +++-
lib/efi_loader/efi_var_common.c | 2 --
lib/efi_loader/efi_var_file.c | 41 -
lib/efi_loader/efi_variable.c | 2 +-
4 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h
index 4623a64142..2d97655e1f 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct
efi_var_file **bufp, loff_t *
/**
* efi_var_restore() - restore EFI variables from buffer
*
+ * Only if @safe is set secure boot related variables will be restored.
+ *
* @buf: buffer
+ * @safe: restoring from tamper-resistant storage
* Return: status code
*/
-efi_status_t efi_var_restore(struct efi_var_file *buf);
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
/**
* efi_var_from_file() - read variables from file
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index cf7afecd60..b0c5b672c5 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[] = {
{u"KEK", _global_variable_guid, EFI_AUTH_VAR_KEK},
{u"db", _guid_image_security_database, EFI_AUTH_VAR_DB},
{u"dbx", _guid_image_security_database, EFI_AUTH_VAR_DBX},
- /* not used yet
{u"dbt", _guid_image_security_database, EFI_AUTH_VAR_DBT},
{u"dbr", _guid_image_security_database, EFI_AUTH_VAR_DBR},
- */
};
static bool efi_secure_boot;
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index de076b8cbc..c7c6805ed0 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -148,9 +148,10 @@ error:
#endif
}
-efi_status_t efi_var_restore(struct efi_var_file *buf)
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
{
struct efi_var_entry *var, *last_var;
+ u16 *data;
efi_status_t ret;
if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
@@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file *buf)
return EFI_INVALID_PARAMETER;
}
- var = buf->var;
last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
- while (var < last_var) {
- u16 *data = var->name + u16_strlen(var->name) + 1;
-
- if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
- ret = efi_var_mem_ins(var->name, >guid, var->attr,
- var->length, data, 0, NULL,
- var->time);
- if (ret != EFI_SUCCESS)
- log_err("Failed to set EFI variable %ls\n",
- var->name);
- }
- var = (struct efi_var_entry *)
- ALIGN((uintptr_t)data + var->length, 8);
+ for (var = buf->var; var < last_var;
+var = (struct efi_var_entry *)
+ ALIGN((uintptr_t)data + var->length, 8)) {
+
+ data = var->name + u16_strlen(var->name) + 1;
+
+ /*
+* Secure boot related and non-volatile variables shall only be
+* restored from U-Boot's preseed.
+*/
+ if (!safe &&
+ (efi_auth_var_get_type(var->name, >guid) !=
+EFI_AUTH_VAR_NONE ||
+!(var->attr & EFI_VARIABLE_NON_VOLATILE)))
+ continue;
+ if (!var->length)
+ continue;
+ ret = efi_var_mem_ins(var->name, >guid, var->attr,
+ var->length, data, 0, NULL,
+ var->time);
+ if (ret != EFI_SUCCESS)
+ log_err("Failed to set EFI variable %ls\n", var->name);
}
return EFI_SUCCESS;
}
@@ -213,7 +222,7 @@ efi_status_t efi_var_from_file(void)
log_err("Failed to load EFI variables\n");
goto error;
}
- if (buf->length != len || efi_var_restore(buf) != EFI_SUCCESS)
+ if (buf->length != len || efi_var_restore(buf, false) != EFI_SUCCESS)
log_err("Invalid EFI variables file\n");
error:
free(buf);
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index ba0874e9e7..a7d305ffbc 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -426,7 +426,7 @@ efi_status_t efi_init_variables(void)
if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
[PATCH v2 2/6] efi_loader: correct determination of secure boot state
When U-Boot is started we have to use the existing variables to determine
in which secure boot state we are.
* If a platform key PK is present and DeployedMode=1, we are in deployed
mode.
* If no platform key PK is present and AuditMode=1, we are in audit mode.
* Otherwise if a platform key is present, we are in user mode.
* Otherwise if no platform key is present, we are in setup mode.
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
lib/efi_loader/efi_var_common.c | 39 ++---
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 654ce81f9d..cf7afecd60 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -315,20 +315,43 @@ err:
efi_status_t efi_init_secure_state(void)
{
static bool lock;
- enum efi_secure_mode mode = EFI_MODE_SETUP;
+ enum efi_secure_mode mode;
u8 efi_vendor_keys = 0;
- efi_uintn_t size = 0;
+ efi_uintn_t size;
efi_status_t ret;
if (lock)
return EFI_SUCCESS;
-
- ret = efi_get_variable_int(L"PK", _global_variable_guid,
- NULL, , NULL, NULL);
- if (ret == EFI_BUFFER_TOO_SMALL) {
- if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT))
- mode = EFI_MODE_USER;
+ u8 deployed_mode = 0;
+ u8 audit_mode = 0;
+ u8 setup_mode = 1;
+
+ if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) {
+ size = sizeof(deployed_mode);
+ ret = efi_get_variable_int(u"DeployedMode",
_global_variable_guid,
+ NULL, , _mode, NULL);
+ size = sizeof(audit_mode);
+ ret = efi_get_variable_int(u"AuditMode",
_global_variable_guid,
+ NULL, , _mode, NULL);
+ size = 0;
+ ret = efi_get_variable_int(u"PK", _global_variable_guid,
+ NULL, , NULL, NULL);
+ if (ret == EFI_BUFFER_TOO_SMALL) {
+ setup_mode = 0;
+ audit_mode = 0;
+ } else {
+ setup_mode = 1;
+ deployed_mode = 0;
+ }
}
+ if (deployed_mode)
+ mode = EFI_MODE_DEPLOYED;
+ else if (audit_mode)
+ mode = EFI_MODE_AUDIT;
+ else if (setup_mode)
+ mode = EFI_MODE_SETUP;
+ else
+ mode = EFI_MODE_USER;
lock = true;
ret = efi_transfer_secure_state(mode);
--
2.30.2
[PATCH v2 6/6] efi_loader: always initialize the secure boot state
Even if we cannot read the variable store from disk we still need to
initialize the secure boot state.
Don't continue to boot if the variable preseed is invalid as this indicates
that the variable store has been tampered.
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
lib/efi_loader/efi_variable.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 80996d0f47..6d92229e2a 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -427,13 +427,17 @@ efi_status_t efi_init_variables(void)
if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
ret = efi_var_restore((struct efi_var_file *)
__efi_var_file_begin, true);
- if (ret != EFI_SUCCESS)
+ if (ret != EFI_SUCCESS) {
log_err("Invalid EFI variable seed\n");
+ return ret;
+ }
}
-
- ret = efi_var_from_file();
+ ret = efi_init_secure_state();
if (ret != EFI_SUCCESS)
return ret;
- return efi_init_secure_state();
+ /* Don't stop booting if variable store is not available */
+ efi_var_from_file();
+
+ return EFI_SUCCESS;
}
--
2.30.2
[PATCH v2 4/6] efi_loader: correct secure boot state transition
Variable PK must be deleted when switching either to setup mode or to audit
mode.
Variable AuditMode must be writable in setup mode and user mode.
Variable DeployedMode must only be writable in user mode; simplify the
logic.
Signed-off-by: Heinrich Schuchardt
---
v2:
no change
---
lib/efi_loader/efi_var_common.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index b0c5b672c5..63ad6fea9e 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -240,7 +240,7 @@ static efi_status_t efi_set_secure_state(u8 secure_boot, u8
setup_mode,
goto err;
ret = efi_set_variable_int(L"AuditMode", _global_variable_guid,
- audit_mode || setup_mode ?
+ audit_mode || deployed_mode ?
attributes_ro : attributes_rw,
sizeof(audit_mode), _mode, false);
if (ret != EFI_SUCCESS)
@@ -248,7 +248,7 @@ static efi_status_t efi_set_secure_state(u8 secure_boot, u8
setup_mode,
ret = efi_set_variable_int(L"DeployedMode",
_global_variable_guid,
- audit_mode || deployed_mode || setup_mode ?
+ deployed_mode || setup_mode ?
attributes_ro : attributes_rw,
sizeof(deployed_mode), _mode,
false);
@@ -273,17 +273,20 @@ static efi_status_t efi_transfer_secure_state(enum
efi_secure_mode mode)
EFI_PRINT("Switching secure state from %d to %d\n", efi_secure_mode,
mode);
- if (mode == EFI_MODE_DEPLOYED) {
- ret = efi_set_secure_state(1, 0, 0, 1);
- if (ret != EFI_SUCCESS)
- goto err;
- } else if (mode == EFI_MODE_AUDIT) {
+ if (mode == EFI_MODE_SETUP || mode == EFI_MODE_AUDIT) {
ret = efi_set_variable_int(L"PK", _global_variable_guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS,
0, NULL, false);
+ if (ret != EFI_NOT_FOUND && ret != EFI_SUCCESS)
+ goto err;
+ }
+
+ if (mode == EFI_MODE_DEPLOYED) {
+ ret = efi_set_secure_state(1, 0, 0, 1);
if (ret != EFI_SUCCESS)
goto err;
+ } else if (mode == EFI_MODE_AUDIT) {
ret = efi_set_secure_state(0, 1, 1, 0);
if (ret != EFI_SUCCESS)
--
2.30.2
[PATCH v2 0/6] efi_loader: fix secure boot mode transitions
The UEFI specification 2.9 defines the different modes that secure boot may be in. The patch series adds support for the "Deployed Mode" and the "Setup Mode". Furthermore the secure boot signature database must only be loaded from tamper-resistant storage. So we must not load it from ubootefi.var on the EFI system partition but only from the preseed variables store or via the OP-TEE driver for the eMMC replay protected memory partition. v2: correct variable name in lib/efi_loader/efi_variable_tee.c Heinrich Schuchardt (6): efi_loader: stop recursion in efi_init_secure_state efi_loader: correct determination of secure boot state efi_loader: don't load signature database from file efi_loader: correct secure boot state transition efi_loader: writing AuditMode, DeployedMode efi_loader: always initialize the secure boot state include/efi_variable.h| 6 ++- lib/efi_loader/efi_var_common.c | 66 +++ lib/efi_loader/efi_var_file.c | 41 +++ lib/efi_loader/efi_variable.c | 20 ++ lib/efi_loader/efi_variable_tee.c | 4 +- 5 files changed, 95 insertions(+), 42 deletions(-) -- 2.30.2
Re: [PATCH 00/28] Initial implementation of bootmethod/bootflow
> Date: Thu, 26 Aug 2021 09:00:01 -0400 > From: Tom Rini > > On Thu, Aug 26, 2021 at 02:01:07PM +0200, Mark Kettenis wrote: > > > Date: Wed, 25 Aug 2021 18:06:05 -0400 > > > From: Tom Rini > > > > > > On Wed, Aug 25, 2021 at 11:54:58PM +0200, Mark Kettenis wrote: > > > > > Date: Wed, 25 Aug 2021 10:56:35 -0400 > > > > > From: Tom Rini > > > > > > > > > > On Wed, Aug 25, 2021 at 11:42:51PM +0900, AKASHI Takahiro wrote: > > > > > > Simon, > > > > > > > > > > > > On Wed, Aug 25, 2021 at 07:11:44AM -0600, Simon Glass wrote: > > > > > > > Hi, > > > > > > > > > > > > > > On Wed, 25 Aug 2021 at 04:45, Emmanuel Vadot > > > > > > > wrote: > > > > > > > > > > > > > > > > On Tue, 24 Aug 2021 12:22:42 +0200 (CEST) > > > > > > > > Mark Kettenis wrote: > > > > > > > > > > > > > > > > > > Date: Mon, 23 Aug 2021 16:01:46 -0400 > > > > > > > > > > From: Tom Rini > > > > > > > > > > > > > > > > > > > > On Mon, Aug 23, 2021 at 11:25:42AM -0600, Simon Glass wrote: > > > > > > > > > > > Hi Mark, > > > > > > > > > > > > > > > > > > > > > > On Mon, 23 Aug 2021 at 05:54, Mark Kettenis > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > From: Simon Glass > > > > > > > > > > > > > Date: Wed, 18 Aug 2021 21:45:33 -0600 > > > > > > > > > > > > > > > > > > > > > > > > > > Bootmethod and bootflow provide a built-in way for > > > > > > > > > > > > > U-Boot to automatically boot > > > > > > > > > > > > > an Operating System without custom scripting and > > > > > > > > > > > > > other customisation: > > > > > > > > > > > > > > > > > > > > > > > > > > - bootmethod - a method to scan a device to find > > > > > > > > > > > > > bootflows (owned by U-Boot) > > > > > > > > > > > > > - bootflow - a description of how to boot (owned by > > > > > > > > > > > > > the distro) > > > > > > > > > > > > > > > > > > > > > > > > > > This series provides an initial implementation of > > > > > > > > > > > > > these, enable to scan > > > > > > > > > > > > > for bootflows from MMC and Ethernet. The only > > > > > > > > > > > > > bootflow supported is > > > > > > > > > > > > > distro boot, i.e. an extlinux.conf file included on a > > > > > > > > > > > > > filesystem or > > > > > > > > > > > > > tftp server. It works similiarly to the existing > > > > > > > > > > > > > script-based approach, > > > > > > > > > > > > > but is native to U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > > > With this we can boot on a Raspberry Pi 3 with just > > > > > > > > > > > > > one command: > > > > > > > > > > > > > > > > > > > > > > > > > >bootflow scan -lb > > > > > > > > > > > > > > > > > > > > > > > > > > which means to scan, listing (-l) each bootflow and > > > > > > > > > > > > > trying to boot each > > > > > > > > > > > > > one (-b). The final patch shows this. > > > > > > > > > > > > > > > > > > > > > > > > > > It is intended that this approach be expanded to > > > > > > > > > > > > > support mechanisms other > > > > > > > > > > > > > than distro boot, including EFI-related ones. With a > > > > > > > > > > > > > standard way to > > > > > > > > > > > > > identify boot devices, these features become easier. > > > > > > > > > > > > > It also should > > > > > > > > > > > > > support U-Boot scripts, for backwards compatibility > > > > > > > > > > > > > only. > > > > > > > > > > > > > > > > > > > > > > > > > > The first patch of this series moves boot-related > > > > > > > > > > > > > code out of common/ and > > > > > > > > > > > > > into a new boot/ directory. This helps to collect > > > > > > > > > > > > > these related files > > > > > > > > > > > > > in one place, as common/ is quite large. > > > > > > > > > > > > > > > > > > > > > > > > > > Like sysboot, this feature makes use of the existing > > > > > > > > > > > > > PXE implementation. > > > > > > > > > > > > > Much of this series consists of cleaning up that code > > > > > > > > > > > > > and refactoring it > > > > > > > > > > > > > into something closer to a module that can be called, > > > > > > > > > > > > > teasing apart its > > > > > > > > > > > > > reliance on the command-line interpreter to access > > > > > > > > > > > > > filesystems and the > > > > > > > > > > > > > like. Also it now uses function arguments and its own > > > > > > > > > > > > > context struct > > > > > > > > > > > > > internally rather than environment variables, which > > > > > > > > > > > > > is very hard to > > > > > > > > > > > > > follow. No core functional change is included in the > > > > > > > > > > > > > included PXE patches. > > > > > > > > > > > > > > > > > > > > > > > > > > For documentation, see the 'doc' patch. > > > > > > > > > > > > > > > > > > > > > > > > > > There is quite a long list of future work included in > > > > > > > > > > > > > the documentation. > > > > > > > > > > > > > One question is the choice of naming. Since this is a > > > > > > > > > > > > > bootloader, should > > > > > > > > > > > > > we just call this a 'method' and a
Re: [PATCH u-boot-marvell 25/29] tools: kwboot: Support higher baudrates when booting via UART
OK, there were yet some other issues with execaddr when higheer baudrate is used. I will send v2 in a few days, to wait for some more reactions for v1. Marek
Re: [PATCH 00/28] Initial implementation of bootmethod/bootflow
On Thu, Aug 26, 2021 at 03:33:07PM +0900, AKASHI Takahiro wrote: > Mark, Tom, > > On Wed, Aug 25, 2021 at 06:06:05PM -0400, Tom Rini wrote: > > On Wed, Aug 25, 2021 at 11:54:58PM +0200, Mark Kettenis wrote: > > > > Date: Wed, 25 Aug 2021 10:56:35 -0400 > > > > From: Tom Rini > > > > > > > > On Wed, Aug 25, 2021 at 11:42:51PM +0900, AKASHI Takahiro wrote: > > > > > Simon, > > > > > > > > > > On Wed, Aug 25, 2021 at 07:11:44AM -0600, Simon Glass wrote: > > > > > > Hi, > > > > > > > > > > > > On Wed, 25 Aug 2021 at 04:45, Emmanuel Vadot > > > > > > wrote: > > > > > > > > > > > > > > On Tue, 24 Aug 2021 12:22:42 +0200 (CEST) > > > > > > > Mark Kettenis wrote: > > > > > > > > > > > > > > > > Date: Mon, 23 Aug 2021 16:01:46 -0400 > > > > > > > > > From: Tom Rini > > > > > > > > > > > > > > > > > > On Mon, Aug 23, 2021 at 11:25:42AM -0600, Simon Glass wrote: > > > > > > > > > > Hi Mark, > > > > > > > > > > > > > > > > > > > > On Mon, 23 Aug 2021 at 05:54, Mark Kettenis > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > From: Simon Glass > > > > > > > > > > > > Date: Wed, 18 Aug 2021 21:45:33 -0600 > > > > > > > > > > > > > > > > > > > > > > > > Bootmethod and bootflow provide a built-in way for > > > > > > > > > > > > U-Boot to automatically boot > > > > > > > > > > > > an Operating System without custom scripting and other > > > > > > > > > > > > customisation: > > > > > > > > > > > > > > > > > > > > > > > > - bootmethod - a method to scan a device to find > > > > > > > > > > > > bootflows (owned by U-Boot) > > > > > > > > > > > > - bootflow - a description of how to boot (owned by > > > > > > > > > > > > the distro) > > > > > > > > > > > > > > > > > > > > > > > > This series provides an initial implementation of > > > > > > > > > > > > these, enable to scan > > > > > > > > > > > > for bootflows from MMC and Ethernet. The only bootflow > > > > > > > > > > > > supported is > > > > > > > > > > > > distro boot, i.e. an extlinux.conf file included on a > > > > > > > > > > > > filesystem or > > > > > > > > > > > > tftp server. It works similiarly to the existing > > > > > > > > > > > > script-based approach, > > > > > > > > > > > > but is native to U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > With this we can boot on a Raspberry Pi 3 with just one > > > > > > > > > > > > command: > > > > > > > > > > > > > > > > > > > > > > > >bootflow scan -lb > > > > > > > > > > > > > > > > > > > > > > > > which means to scan, listing (-l) each bootflow and > > > > > > > > > > > > trying to boot each > > > > > > > > > > > > one (-b). The final patch shows this. > > > > > > > > > > > > > > > > > > > > > > > > It is intended that this approach be expanded to > > > > > > > > > > > > support mechanisms other > > > > > > > > > > > > than distro boot, including EFI-related ones. With a > > > > > > > > > > > > standard way to > > > > > > > > > > > > identify boot devices, these features become easier. It > > > > > > > > > > > > also should > > > > > > > > > > > > support U-Boot scripts, for backwards compatibility > > > > > > > > > > > > only. > > > > > > > > > > > > > > > > > > > > > > > > The first patch of this series moves boot-related code > > > > > > > > > > > > out of common/ and > > > > > > > > > > > > into a new boot/ directory. This helps to collect these > > > > > > > > > > > > related files > > > > > > > > > > > > in one place, as common/ is quite large. > > > > > > > > > > > > > > > > > > > > > > > > Like sysboot, this feature makes use of the existing > > > > > > > > > > > > PXE implementation. > > > > > > > > > > > > Much of this series consists of cleaning up that code > > > > > > > > > > > > and refactoring it > > > > > > > > > > > > into something closer to a module that can be called, > > > > > > > > > > > > teasing apart its > > > > > > > > > > > > reliance on the command-line interpreter to access > > > > > > > > > > > > filesystems and the > > > > > > > > > > > > like. Also it now uses function arguments and its own > > > > > > > > > > > > context struct > > > > > > > > > > > > internally rather than environment variables, which is > > > > > > > > > > > > very hard to > > > > > > > > > > > > follow. No core functional change is included in the > > > > > > > > > > > > included PXE patches. > > > > > > > > > > > > > > > > > > > > > > > > For documentation, see the 'doc' patch. > > > > > > > > > > > > > > > > > > > > > > > > There is quite a long list of future work included in > > > > > > > > > > > > the documentation. > > > > > > > > > > > > One question is the choice of naming. Since this is a > > > > > > > > > > > > bootloader, should > > > > > > > > > > > > we just call this a 'method' and a 'flow' ? The 'boot' > > > > > > > > > > > > prefix is already > > > > > > > > > > > > shared by other commands like bootm, booti, etc. > > > > > > > > > > > > > > > > > > > > > > > > The design is described here: > > > > > > > > > > > > > > > > > > >
Re: [PATCH 00/28] Initial implementation of bootmethod/bootflow
On Thu, Aug 26, 2021 at 02:01:07PM +0200, Mark Kettenis wrote: > > Date: Wed, 25 Aug 2021 18:06:05 -0400 > > From: Tom Rini > > > > On Wed, Aug 25, 2021 at 11:54:58PM +0200, Mark Kettenis wrote: > > > > Date: Wed, 25 Aug 2021 10:56:35 -0400 > > > > From: Tom Rini > > > > > > > > On Wed, Aug 25, 2021 at 11:42:51PM +0900, AKASHI Takahiro wrote: > > > > > Simon, > > > > > > > > > > On Wed, Aug 25, 2021 at 07:11:44AM -0600, Simon Glass wrote: > > > > > > Hi, > > > > > > > > > > > > On Wed, 25 Aug 2021 at 04:45, Emmanuel Vadot > > > > > > wrote: > > > > > > > > > > > > > > On Tue, 24 Aug 2021 12:22:42 +0200 (CEST) > > > > > > > Mark Kettenis wrote: > > > > > > > > > > > > > > > > Date: Mon, 23 Aug 2021 16:01:46 -0400 > > > > > > > > > From: Tom Rini > > > > > > > > > > > > > > > > > > On Mon, Aug 23, 2021 at 11:25:42AM -0600, Simon Glass wrote: > > > > > > > > > > Hi Mark, > > > > > > > > > > > > > > > > > > > > On Mon, 23 Aug 2021 at 05:54, Mark Kettenis > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > From: Simon Glass > > > > > > > > > > > > Date: Wed, 18 Aug 2021 21:45:33 -0600 > > > > > > > > > > > > > > > > > > > > > > > > Bootmethod and bootflow provide a built-in way for > > > > > > > > > > > > U-Boot to automatically boot > > > > > > > > > > > > an Operating System without custom scripting and other > > > > > > > > > > > > customisation: > > > > > > > > > > > > > > > > > > > > > > > > - bootmethod - a method to scan a device to find > > > > > > > > > > > > bootflows (owned by U-Boot) > > > > > > > > > > > > - bootflow - a description of how to boot (owned by > > > > > > > > > > > > the distro) > > > > > > > > > > > > > > > > > > > > > > > > This series provides an initial implementation of > > > > > > > > > > > > these, enable to scan > > > > > > > > > > > > for bootflows from MMC and Ethernet. The only bootflow > > > > > > > > > > > > supported is > > > > > > > > > > > > distro boot, i.e. an extlinux.conf file included on a > > > > > > > > > > > > filesystem or > > > > > > > > > > > > tftp server. It works similiarly to the existing > > > > > > > > > > > > script-based approach, > > > > > > > > > > > > but is native to U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > With this we can boot on a Raspberry Pi 3 with just one > > > > > > > > > > > > command: > > > > > > > > > > > > > > > > > > > > > > > >bootflow scan -lb > > > > > > > > > > > > > > > > > > > > > > > > which means to scan, listing (-l) each bootflow and > > > > > > > > > > > > trying to boot each > > > > > > > > > > > > one (-b). The final patch shows this. > > > > > > > > > > > > > > > > > > > > > > > > It is intended that this approach be expanded to > > > > > > > > > > > > support mechanisms other > > > > > > > > > > > > than distro boot, including EFI-related ones. With a > > > > > > > > > > > > standard way to > > > > > > > > > > > > identify boot devices, these features become easier. It > > > > > > > > > > > > also should > > > > > > > > > > > > support U-Boot scripts, for backwards compatibility > > > > > > > > > > > > only. > > > > > > > > > > > > > > > > > > > > > > > > The first patch of this series moves boot-related code > > > > > > > > > > > > out of common/ and > > > > > > > > > > > > into a new boot/ directory. This helps to collect these > > > > > > > > > > > > related files > > > > > > > > > > > > in one place, as common/ is quite large. > > > > > > > > > > > > > > > > > > > > > > > > Like sysboot, this feature makes use of the existing > > > > > > > > > > > > PXE implementation. > > > > > > > > > > > > Much of this series consists of cleaning up that code > > > > > > > > > > > > and refactoring it > > > > > > > > > > > > into something closer to a module that can be called, > > > > > > > > > > > > teasing apart its > > > > > > > > > > > > reliance on the command-line interpreter to access > > > > > > > > > > > > filesystems and the > > > > > > > > > > > > like. Also it now uses function arguments and its own > > > > > > > > > > > > context struct > > > > > > > > > > > > internally rather than environment variables, which is > > > > > > > > > > > > very hard to > > > > > > > > > > > > follow. No core functional change is included in the > > > > > > > > > > > > included PXE patches. > > > > > > > > > > > > > > > > > > > > > > > > For documentation, see the 'doc' patch. > > > > > > > > > > > > > > > > > > > > > > > > There is quite a long list of future work included in > > > > > > > > > > > > the documentation. > > > > > > > > > > > > One question is the choice of naming. Since this is a > > > > > > > > > > > > bootloader, should > > > > > > > > > > > > we just call this a 'method' and a 'flow' ? The 'boot' > > > > > > > > > > > > prefix is already > > > > > > > > > > > > shared by other commands like bootm, booti, etc. > > > > > > > > > > > > > > > > > > > > > > > > The design is described here: > > > > > > > > > > > > > > > > > > > > > > > >
Re: Usage of device-tree for blobs
> From: Simon Glass > Date: Wed, 25 Aug 2021 21:15:00 -0600 > > Hi Heinrich, > > On Wed, 25 Aug 2021 at 02:05, Heinrich Schuchardt wrote: > > > > Hello Simon, > > > > some boards like qemu-riscv64_defconfig do not use any device-tree at > > build time. A device-tree is only supplied at runtime by the prior boot > > stage (CONFIG_OF_PRIOR_STAGE=y). > > > > In doc/develop/devicetree/intro.rst you suggest to put binary blobs into > > the device-tree. > > > > Could you, please, update the documentation to explain how adding blobs > > to the device-tree works in the different scenarios depending on the > > values of: > > > > CONFIG_OF_EMBED > > CONFIG_OF_SEPARATE > > CONFIG_OF_BOARD > > CONFIG_OF_HOSTFILE > > CONFIG_OF_PRIOR_STAGE > > > > It would be especially important to understand how one can develop a > > board independent feature which works for all of these settings. > > OK I will take a crack at this tomorrow. > > But I think there is a disconnect here, since the only options that > matter within U-Boot are OF_SEPARATE and OF_HOSTFILE, which both use a > u-boot.dtb file. There is nothing tricky here. > > The OF_BOARD business is for when the board does special things. > Presumably signing will do special things too. We cannot really know > what those things are because the board as 'opted out' of the standard > options. > > > > > Please, describe CONFIG_OF_PRIOR_STAGE in > > doc/develop/devicetree/control.rst. > > So I'm not allowed to delete that option? :-) It seems to me to be > extremely sparse on documentation. We need an explanation of what it > means and what effect it has on the build system, U-Boot and some > discussion of how qemu works. It seems to have been added as part of > an unrelated broadcom commit. The tags were incorrect so I doubt > anyone noticed it. Since then it has apparently proved useful > elsewhere, but no one has added more docs. > > So perhaps you can help me with my doc by explaining how > OF_PRIOR_STAGE works in qmeu, why the DT in U-Boot cannot be used, and > which project actually hosts the DT that qemu provides? Armed with > that information, I might be able to make sense of it all. Well, QEMU allows configuration of (emulated/paravirtualised) devices from the command line. So it is pretty much impossible for U-Boot to provide a DT that matches that configuration without adding a lot of code the dynamically build it from some sort of configuration specification provided by QEMU to U-Boot. But at that point QEMU might as well provide the DT itself, don't you agree? Anyway, replying to this thread since for the Apple M1 support that I'm working on we're in a somewhat similar situation. The Asahi Linux project has implemented m1n1 as a bootloader and plans to use U-Boot as a "payload" to implement booting a standard Linux distribution (and other OSes). In this scenario m1n1 actually provides the DT since it parses Apple's version of the DT (which isn't in the standard DT format) and adds/updates certain properties that depend on the actual hardware it is running on. For this code I use CONFIG_OF_BOARD and implement board_fdt_blob_setup() to simple return a pointer to the DT that m1n1 set up. But that seems to be exactly what CONFIG_OF_PRIOR_STAGE does...
Re: Rename of the u-boot-atmel custodian tree
On Thu, Aug 26, 2021 at 07:17:24AM +, eugen.hris...@microchip.com wrote: > Hello everyone, > > I plan to rename the u-boot-atmel tree to u-boot-at91 . > > It's been more than 5 years since Atmel is part of Microchip, and the > name is slowly being changed to reflect the new reality. > > It makes more sense to have the custodian tree named u-boot-at91 , as > this tree is used only for patches related to the at91 boards and > drivers (not any other former Atmel product, nor any other Microchip > product ). I won't be naming it u-boot-microchip since Microchip does > much more than just the former Atmel MPUs (AT91 architecture) . > > This means, however, that all remotes to this tree have to be updated. > I do not see any other side effect at this moment. > > I will wait some more time , few weeks or one month, before making this > change, to allow anyone to comment on this. Sounds like a good plan to me, thanks! -- Tom signature.asc Description: PGP signature
[RFC PATCH v1 2/5] arm64: dts: imx8mm: add common -binman.dtsi
From: Marcel Ziswiler
With the move to using binman to generate SPL aka u-boot-spl-ddr.bin and
U-Boot proper aka u-boot.itb every board now covers such configuration
in its own U-Boot specific device tree include. Introduce a new common
imx8mm-binman.dtsi which covers the common part of that configuration.
Signed-off-by: Marcel Ziswiler
---
arch/arm/dts/imx8mm-binman.dtsi | 136 ++
arch/arm/dts/imx8mm-cl-iot-gate-u-boot.dtsi | 126 ++--
arch/arm/dts/imx8mm-evk-u-boot.dtsi | 124 +---
.../dts/imx8mm-kontron-n801x-s-u-boot.dtsi| 123 +---
arch/arm/dts/imx8mm-venice-u-boot.dtsi| 120 +---
arch/arm/dts/imx8mm-verdin-u-boot.dtsi| 123 +---
6 files changed, 156 insertions(+), 596 deletions(-)
create mode 100644 arch/arm/dts/imx8mm-binman.dtsi
diff --git a/arch/arm/dts/imx8mm-binman.dtsi b/arch/arm/dts/imx8mm-binman.dtsi
new file mode 100644
index 000..2d98c1ef577
--- /dev/null
+++ b/arch/arm/dts/imx8mm-binman.dtsi
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021 Toradex
+ */
+
+/ {
+ binman: binman {
+ multiple-images;
+ };
+};
+
+ {
+ u-boot-spl-ddr {
+ filename = "u-boot-spl-ddr.bin";
+ pad-byte = <0xff>;
+ align-size = <4>;
+ align = <4>;
+
+ u-boot-spl {
+ align-end = <4>;
+ };
+
+ blob_1: blob-ext@1 {
+ filename = "lpddr4_pmu_train_1d_imem.bin";
+ size = <0x8000>;
+ };
+
+ blob_2: blob-ext@2 {
+ filename = "lpddr4_pmu_train_1d_dmem.bin";
+ size = <0x4000>;
+ };
+
+ blob_3: blob-ext@3 {
+ filename = "lpddr4_pmu_train_2d_imem.bin";
+ size = <0x8000>;
+ };
+
+ blob_4: blob-ext@4 {
+ filename = "lpddr4_pmu_train_2d_dmem.bin";
+ size = <0x4000>;
+ };
+ };
+
+ spl {
+ filename = "spl.bin";
+
+ mkimage {
+ args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e
0x7e1000";
+
+ blob {
+ filename = "u-boot-spl-ddr.bin";
+ };
+ };
+ };
+
+ itb {
+ filename = "u-boot.itb";
+
+ fit {
+ description = "Configuration to load ATF before U-Boot";
+ #address-cells = <1>;
+ fit,external-offset = ;
+
+ images {
+ uboot {
+ description = "U-Boot (64-bit)";
+ type = "standalone";
+ arch = "arm64";
+ compression = "none";
+ load = ;
+
+ uboot_blob: blob-ext {
+ filename = "u-boot-nodtb.bin";
+ };
+ };
+
+ atf {
+ description = "ARM Trusted Firmware";
+ type = "firmware";
+ arch = "arm64";
+ compression = "none";
+ load = <0x92>;
+ entry = <0x92>;
+
+ atf_blob: blob-ext {
+ filename = "bl31.bin";
+ };
+ };
+
+ binman_fip: fip {
+ description = "Trusted Firmware FIP";
+ type = "firmware";
+ arch = "arm64";
+ compression = "none";
+ load = <0x4031>;
+ };
+
+ fdt {
+ description = "NAME";
+ type = "flat_dt";
+ compression = "none";
+
+ uboot_fdt_blob: blob-ext {
+ filename = "u-boot.dtb";
+ };
+ };
+ };
+
+ configurations {
+ default = "conf";
+
+
[RFC PATCH v1 5/5] arm64: dts: imx8mm-binman.dtsi: improve odd blob-ext naming
From: Marcel Ziswiler
Rather than using odd implicit blob-ext naming, explicitly specify the
type to be of blob-ext and therefore also simplify the node naming.
Signed-off-by: Marcel Ziswiler
---
Changes in v1:
- This was suggested by Simon on my earlier patch set upon which we
decided to first break this up into a common binman dtsi and then
subsequently do such further improvements.
arch/arm/dts/imx8mm-binman.dtsi | 27 ++-
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/arch/arm/dts/imx8mm-binman.dtsi b/arch/arm/dts/imx8mm-binman.dtsi
index 1d2895d8970..42f5246c30a 100644
--- a/arch/arm/dts/imx8mm-binman.dtsi
+++ b/arch/arm/dts/imx8mm-binman.dtsi
@@ -21,24 +21,28 @@
align-end = <4>;
};
- blob_1: blob-ext@1 {
+ 1d-imem {
filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
+ type = "blob-ext";
};
- blob_2: blob-ext@2 {
+ 1d_dmem {
filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
+ type = "blob-ext";
};
- blob_3: blob-ext@3 {
+ 2d_imem {
filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
+ type = "blob-ext";
};
- blob_4: blob-ext@4 {
+ 2d_dmem {
filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
+ type = "blob-ext";
};
};
@@ -70,8 +74,9 @@
load = ;
type = "standalone";
- uboot_blob: blob-ext {
+ uboot_blob {
filename = "u-boot-nodtb.bin";
+ type = "blob-ext";
};
};
@@ -83,8 +88,9 @@
load = <0x92>;
type = "firmware";
- atf_blob: blob-ext {
+ atf_blob {
filename = "bl31.bin";
+ type = "blob-ext";
};
};
@@ -101,8 +107,9 @@
description = "NAME";
type = "flat_dt";
- uboot_fdt_blob: blob-ext {
+ uboot_fdt_blob {
filename = "u-boot.dtb";
+ type = "blob-ext";
};
};
};
@@ -124,14 +131,16 @@
filename = "flash.bin";
pad-byte = <0x00>;
- spl: blob-ext@1 {
+ spl {
filename = "spl.bin";
offset = <0x0>;
+ type = "blob-ext";
};
- binman_uboot: blob-ext@2 {
+ binman_uboot: uboot {
filename = "u-boot.itb";
offset = <0x57c00>;
+ type = "blob-ext";
};
};
};
--
2.26.2
[RFC PATCH v1 3/5] arm64: dts: imx8mm-binman.dtsi: alphabetically re-order properties
From: Marcel Ziswiler
Alphabetically re-order properties.
Signed-off-by: Marcel Ziswiler
---
arch/arm/dts/imx8mm-binman.dtsi | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/arm/dts/imx8mm-binman.dtsi b/arch/arm/dts/imx8mm-binman.dtsi
index 2d98c1ef577..b7ab8d19934 100644
--- a/arch/arm/dts/imx8mm-binman.dtsi
+++ b/arch/arm/dts/imx8mm-binman.dtsi
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0+
+// SPDX-License-Identifier: GPL-2.0+ OR MIT
/*
* Copyright 2021 Toradex
*/
@@ -11,10 +11,10 @@
{
u-boot-spl-ddr {
+ align = <4>;
+ align-size = <4>;
filename = "u-boot-spl-ddr.bin";
pad-byte = <0xff>;
- align-size = <4>;
- align = <4>;
u-boot-spl {
align-end = <4>;
@@ -58,16 +58,16 @@
fit {
description = "Configuration to load ATF before U-Boot";
- #address-cells = <1>;
fit,external-offset = ;
+ #address-cells = <1>;
images {
uboot {
- description = "U-Boot (64-bit)";
- type = "standalone";
arch = "arm64";
compression = "none";
+ description = "U-Boot (64-bit)";
load = ;
+ type = "standalone";
uboot_blob: blob-ext {
filename = "u-boot-nodtb.bin";
@@ -75,12 +75,12 @@
};
atf {
- description = "ARM Trusted Firmware";
- type = "firmware";
arch = "arm64";
compression = "none";
- load = <0x92>;
+ description = "ARM Trusted Firmware";
entry = <0x92>;
+ load = <0x92>;
+ type = "firmware";
atf_blob: blob-ext {
filename = "bl31.bin";
@@ -88,17 +88,17 @@
};
binman_fip: fip {
- description = "Trusted Firmware FIP";
- type = "firmware";
arch = "arm64";
compression = "none";
+ description = "Trusted Firmware FIP";
load = <0x4031>;
+ type = "firmware";
};
fdt {
+ compression = "none";
description = "NAME";
type = "flat_dt";
- compression = "none";
uboot_fdt_blob: blob-ext {
filename = "u-boot.dtb";
@@ -111,9 +111,9 @@
binman_configuration: conf {
description = "NAME";
+ fdt = "fdt";
firmware = "uboot";
loadables = "atf";
- fdt = "fdt";
};
};
};
@@ -124,13 +124,13 @@
pad-byte = <0x00>;
spl: blob-ext@1 {
- offset = <0x0>;
filename = "spl.bin";
+ offset = <0x0>;
};
binman_uboot: blob-ext@2 {
- offset = <0x57c00>;
filename = "u-boot.itb";
+ offset = <0x57c00>;
};
};
};
--
2.26.2
[RFC PATCH v1 4/5] arm64: dts: imx8mm-binman.dtsi: explicitly add spl filename
From: Marcel Ziswiler
Explicitly add SPL aka u-boot-spl.bin filename.
Signed-off-by: Marcel Ziswiler
---
arch/arm/dts/imx8mm-binman.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/dts/imx8mm-binman.dtsi b/arch/arm/dts/imx8mm-binman.dtsi
index b7ab8d19934..1d2895d8970 100644
--- a/arch/arm/dts/imx8mm-binman.dtsi
+++ b/arch/arm/dts/imx8mm-binman.dtsi
@@ -17,6 +17,7 @@
pad-byte = <0xff>;
u-boot-spl {
+ filename = "u-boot-spl.bin";
align-end = <4>;
};
--
2.26.2
[RFC PATCH v1 0/5] arm64: dts: imx8mm: add common -binman.dtsi and further clean-up
From: Marcel Ziswiler
With the move to using binman to generate SPL aka u-boot-spl-ddr.bin and
U-Boot proper aka u-boot.itb every board now covers such configuration
in its own U-Boot specific device tree include. Introduce a new common
imx8mm-binman.dtsi which covers the common part of that configuration.
The initial patch fixes an issue with intermediate binary naming for the
imx8mm-cl-iot-gate. And subsequent patches further clean up that dtsi.
This series is based on Peng's binman conversion of late [1], my Verdin
iMX8M Mini target refresh [2], Fabio's generating a single bootable
flash.bin again for imx8mm-evk [3], Tim's switching imx8mm_venice to
using binman to pack images [4] and Frieder's support for Kontron
Electronics i.MX6UL/ULL and i.MX8MM SoMs [5].
This series has been run-time tested on Verdin iMX8M Mini. The other
targets were only compile tested.
Please note that for now, I left out the following intricacies of
imx8mm-venice-u-boot.dtsi. Not quite sure what exactly those are used
for.
fit,fdt-list = "of-list";
...
@fdt-SEQ {
...
default = "@config-DEFAULT-SEQ";
...
@config-SEQ {
...
fdt = "fdt-SEQ";
[1] https://marc.info/?l=u-boot=162908367004699
[2] https://marc.info/?l=u-boot=162990457410422
[3] https://marc.info/?l=u-boot=162972343709264
[4] https://marc.info/?l=u-boot=162983187605276
[5] https://marc.info/?l=u-boot=162972880612450
Changes in v1:
- This was suggested by Simon on my earlier patch set upon which we
decided to first break this up into a common binman dtsi and then
subsequently do such further improvements.
Marcel Ziswiler (5):
imx8mm-cl-iot-gate: fix imximage intermediate binary naming
arm64: dts: imx8mm: add common -binman.dtsi
arm64: dts: imx8mm-binman.dtsi: alphabetically re-order properties
arm64: dts: imx8mm-binman.dtsi: explicitly add spl filename
arm64: dts: imx8mm-binman.dtsi: improve odd blob-ext naming
arch/arm/dts/imx8mm-binman.dtsi | 146 ++
arch/arm/dts/imx8mm-cl-iot-gate-u-boot.dtsi | 126 ++-
arch/arm/dts/imx8mm-evk-u-boot.dtsi | 124 +--
.../dts/imx8mm-kontron-n801x-s-u-boot.dtsi| 123 +--
arch/arm/dts/imx8mm-venice-u-boot.dtsi| 120 +-
arch/arm/dts/imx8mm-verdin-u-boot.dtsi| 123 +--
.../imximage-8mm-lpddr4.cfg | 2 +-
7 files changed, 167 insertions(+), 597 deletions(-)
create mode 100644 arch/arm/dts/imx8mm-binman.dtsi
--
2.26.2
[RFC PATCH v1 1/5] imx8mm-cl-iot-gate: fix imximage intermediate binary naming
From: Marcel Ziswiler This fixes the following build time issue: ... BINMAN all binman: Error 1 running 'mkimage -d ./mkimage.spl.mkimage -n spl/u-boot-spl.cfgout -T imx8mimage -e 0x7e1000 ./mkimage-out.spl.mkimage': mkimage.flash.mkimage: Can't open: No such file or directory make: *** [Makefile:1094: all] Error 1 Signed-off-by: Marcel Ziswiler --- board/compulab/imx8mm-cl-iot-gate/imximage-8mm-lpddr4.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/board/compulab/imx8mm-cl-iot-gate/imximage-8mm-lpddr4.cfg b/board/compulab/imx8mm-cl-iot-gate/imximage-8mm-lpddr4.cfg index b89092a5590..4071219fbf4 100644 --- a/board/compulab/imx8mm-cl-iot-gate/imximage-8mm-lpddr4.cfg +++ b/board/compulab/imx8mm-cl-iot-gate/imximage-8mm-lpddr4.cfg @@ -6,4 +6,4 @@ #define __ASSEMBLY__ BOOT_FROM sd -LOADER mkimage.flash.mkimage 0x7E1000 +LOADER u-boot-spl-ddr.bin 0x7e1000 -- 2.26.2
Re: [PATCH 00/28] Initial implementation of bootmethod/bootflow
> Date: Wed, 25 Aug 2021 18:06:05 -0400 > From: Tom Rini > > On Wed, Aug 25, 2021 at 11:54:58PM +0200, Mark Kettenis wrote: > > > Date: Wed, 25 Aug 2021 10:56:35 -0400 > > > From: Tom Rini > > > > > > On Wed, Aug 25, 2021 at 11:42:51PM +0900, AKASHI Takahiro wrote: > > > > Simon, > > > > > > > > On Wed, Aug 25, 2021 at 07:11:44AM -0600, Simon Glass wrote: > > > > > Hi, > > > > > > > > > > On Wed, 25 Aug 2021 at 04:45, Emmanuel Vadot > > > > > wrote: > > > > > > > > > > > > On Tue, 24 Aug 2021 12:22:42 +0200 (CEST) > > > > > > Mark Kettenis wrote: > > > > > > > > > > > > > > Date: Mon, 23 Aug 2021 16:01:46 -0400 > > > > > > > > From: Tom Rini > > > > > > > > > > > > > > > > On Mon, Aug 23, 2021 at 11:25:42AM -0600, Simon Glass wrote: > > > > > > > > > Hi Mark, > > > > > > > > > > > > > > > > > > On Mon, 23 Aug 2021 at 05:54, Mark Kettenis > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > From: Simon Glass > > > > > > > > > > > Date: Wed, 18 Aug 2021 21:45:33 -0600 > > > > > > > > > > > > > > > > > > > > > > Bootmethod and bootflow provide a built-in way for U-Boot > > > > > > > > > > > to automatically boot > > > > > > > > > > > an Operating System without custom scripting and other > > > > > > > > > > > customisation: > > > > > > > > > > > > > > > > > > > > > > - bootmethod - a method to scan a device to find > > > > > > > > > > > bootflows (owned by U-Boot) > > > > > > > > > > > - bootflow - a description of how to boot (owned by the > > > > > > > > > > > distro) > > > > > > > > > > > > > > > > > > > > > > This series provides an initial implementation of these, > > > > > > > > > > > enable to scan > > > > > > > > > > > for bootflows from MMC and Ethernet. The only bootflow > > > > > > > > > > > supported is > > > > > > > > > > > distro boot, i.e. an extlinux.conf file included on a > > > > > > > > > > > filesystem or > > > > > > > > > > > tftp server. It works similiarly to the existing > > > > > > > > > > > script-based approach, > > > > > > > > > > > but is native to U-Boot. > > > > > > > > > > > > > > > > > > > > > > With this we can boot on a Raspberry Pi 3 with just one > > > > > > > > > > > command: > > > > > > > > > > > > > > > > > > > > > >bootflow scan -lb > > > > > > > > > > > > > > > > > > > > > > which means to scan, listing (-l) each bootflow and > > > > > > > > > > > trying to boot each > > > > > > > > > > > one (-b). The final patch shows this. > > > > > > > > > > > > > > > > > > > > > > It is intended that this approach be expanded to support > > > > > > > > > > > mechanisms other > > > > > > > > > > > than distro boot, including EFI-related ones. With a > > > > > > > > > > > standard way to > > > > > > > > > > > identify boot devices, these features become easier. It > > > > > > > > > > > also should > > > > > > > > > > > support U-Boot scripts, for backwards compatibility only. > > > > > > > > > > > > > > > > > > > > > > The first patch of this series moves boot-related code > > > > > > > > > > > out of common/ and > > > > > > > > > > > into a new boot/ directory. This helps to collect these > > > > > > > > > > > related files > > > > > > > > > > > in one place, as common/ is quite large. > > > > > > > > > > > > > > > > > > > > > > Like sysboot, this feature makes use of the existing PXE > > > > > > > > > > > implementation. > > > > > > > > > > > Much of this series consists of cleaning up that code and > > > > > > > > > > > refactoring it > > > > > > > > > > > into something closer to a module that can be called, > > > > > > > > > > > teasing apart its > > > > > > > > > > > reliance on the command-line interpreter to access > > > > > > > > > > > filesystems and the > > > > > > > > > > > like. Also it now uses function arguments and its own > > > > > > > > > > > context struct > > > > > > > > > > > internally rather than environment variables, which is > > > > > > > > > > > very hard to > > > > > > > > > > > follow. No core functional change is included in the > > > > > > > > > > > included PXE patches. > > > > > > > > > > > > > > > > > > > > > > For documentation, see the 'doc' patch. > > > > > > > > > > > > > > > > > > > > > > There is quite a long list of future work included in the > > > > > > > > > > > documentation. > > > > > > > > > > > One question is the choice of naming. Since this is a > > > > > > > > > > > bootloader, should > > > > > > > > > > > we just call this a 'method' and a 'flow' ? The 'boot' > > > > > > > > > > > prefix is already > > > > > > > > > > > shared by other commands like bootm, booti, etc. > > > > > > > > > > > > > > > > > > > > > > The design is described here: > > > > > > > > > > > > > > > > > > > > > > https://drive.google.com/file/d/1ggW0KJpUOR__vBkj3l61L2dav4ZkNC12/view?usp=sharing > > > > > > > > > > > > > > > > > > > > > > The series is available at u-boot-dm/bmea-working > > > > > > > > > > > > > > > > > > > > How does the user control the order in which devices are > > > > > > > > >
Re: [PATCH 00/28] Initial implementation of bootmethod/bootflow
> > By "EFI app", do you mean a way of booting "/efi/boot/bootXX.efi" > > (default file name in case that no image path is specified)? > > > > In fact, this behavior, or removable media support, is defined > > as part of UEFI boot manager in UEFI specification. (See section 3.5) > > What this means is that the boot order, including a removable media > > case and user-provided Boot cases, should be controlled solely > > by "BootOrder" variable. > > So the current combination of distro_bootcmd + UEFI boot manger doesn't > > fully comply with the specification. > > > > Even if those two cases are integrated, I don't know how "BootOrder" > > semantics can be preserved in your approach. > > I think the high level answer is that whereas today part of > distro_bootcmd (and so iterating over boot_targets) "bootefi bootmgr" > gets run, with what Simon is proposing we would have an easier / quicker > way to get over to just running that. Perhaps a clean-up to just use > that, even? Or are we not to the point yet where we could remove the > direct fall-back to /efi/boot/bootXX.efi ? I've not actually got as far as grabbing this series and trying it out but I like a bunch of the concepts. If we can use this to replace a bunch of the distro_boot hush script checks for various devices in which to boot from and make it a more standardised logic I think that's an overall win. Double bonus points if we could have an easy way to specify a boot order and even have the ability to have a "Press F8 to display a boot menu" style option where a user could select the specific device they wish to attempt to boot from without having to interrupt the boot, and often dive into printenv and random commands to try and workout where/where/why to get it to overide the onboarad eMMC so they can boot off a USB stick! Peter
Re: [PATCH v2] board: ea: mx7ulp_com: move setting CONFIG_BOOTCOMMAND to defconfig
Hi Oleksandr, On Thu, Aug 26, 2021 at 8:04 AM Oleksandr Suvorov wrote: > > From: Ricardo Salveti > > Move setting CONFIG_BOOTCOMMAND to the mx7ulp_com_defconfig file. > It also allows replacing the default CONFIG_BOOTCOMMAND without > code modification. > > Signed-off-by: Ricardo Salveti > Signed-off-by: Oleksandr Suvorov > --- > > Changes in v2: > - move setting the command to defconfig Reviewed-by: Fabio Estevam
[PATCH 3/6] efi_loader: don't load signature database from file
The UEFI specification requires that the signature database may only be
stored in tamper-resistant storage. So these variable may not be read
from an unsigned file.
Signed-off-by: Heinrich Schuchardt
---
include/efi_variable.h | 5 +++-
lib/efi_loader/efi_var_common.c | 2 --
lib/efi_loader/efi_var_file.c | 41 -
lib/efi_loader/efi_variable.c | 2 +-
4 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h
index 4623a64142..2d97655e1f 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct
efi_var_file **bufp, loff_t *
/**
* efi_var_restore() - restore EFI variables from buffer
*
+ * Only if @safe is set secure boot related variables will be restored.
+ *
* @buf: buffer
+ * @safe: restoring from tamper-resistant storage
* Return: status code
*/
-efi_status_t efi_var_restore(struct efi_var_file *buf);
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe);
/**
* efi_var_from_file() - read variables from file
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index cf7afecd60..b0c5b672c5 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[] = {
{u"KEK", _global_variable_guid, EFI_AUTH_VAR_KEK},
{u"db", _guid_image_security_database, EFI_AUTH_VAR_DB},
{u"dbx", _guid_image_security_database, EFI_AUTH_VAR_DBX},
- /* not used yet
{u"dbt", _guid_image_security_database, EFI_AUTH_VAR_DBT},
{u"dbr", _guid_image_security_database, EFI_AUTH_VAR_DBR},
- */
};
static bool efi_secure_boot;
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index de076b8cbc..c7c6805ed0 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -148,9 +148,10 @@ error:
#endif
}
-efi_status_t efi_var_restore(struct efi_var_file *buf)
+efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe)
{
struct efi_var_entry *var, *last_var;
+ u16 *data;
efi_status_t ret;
if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC ||
@@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file *buf)
return EFI_INVALID_PARAMETER;
}
- var = buf->var;
last_var = (struct efi_var_entry *)((u8 *)buf + buf->length);
- while (var < last_var) {
- u16 *data = var->name + u16_strlen(var->name) + 1;
-
- if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) {
- ret = efi_var_mem_ins(var->name, >guid, var->attr,
- var->length, data, 0, NULL,
- var->time);
- if (ret != EFI_SUCCESS)
- log_err("Failed to set EFI variable %ls\n",
- var->name);
- }
- var = (struct efi_var_entry *)
- ALIGN((uintptr_t)data + var->length, 8);
+ for (var = buf->var; var < last_var;
+var = (struct efi_var_entry *)
+ ALIGN((uintptr_t)data + var->length, 8)) {
+
+ data = var->name + u16_strlen(var->name) + 1;
+
+ /*
+* Secure boot related and non-volatile variables shall only be
+* restored from U-Boot's preseed.
+*/
+ if (!safe &&
+ (efi_auth_var_get_type(var->name, >guid) !=
+EFI_AUTH_VAR_NONE ||
+!(var->attr & EFI_VARIABLE_NON_VOLATILE)))
+ continue;
+ if (!var->length)
+ continue;
+ ret = efi_var_mem_ins(var->name, >guid, var->attr,
+ var->length, data, 0, NULL,
+ var->time);
+ if (ret != EFI_SUCCESS)
+ log_err("Failed to set EFI variable %ls\n", var->name);
}
return EFI_SUCCESS;
}
@@ -213,7 +222,7 @@ efi_status_t efi_var_from_file(void)
log_err("Failed to load EFI variables\n");
goto error;
}
- if (buf->length != len || efi_var_restore(buf) != EFI_SUCCESS)
+ if (buf->length != len || efi_var_restore(buf, false) != EFI_SUCCESS)
log_err("Invalid EFI variables file\n");
error:
free(buf);
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index ba0874e9e7..a7d305ffbc 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -426,7 +426,7 @@ efi_status_t efi_init_variables(void)
if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
ret =
[PATCH 0/6] efi_loader: fix secure boot mode transitions
The UEFI specification 2.9 defines the different modes that secure boot may be in. The patch series adds support for the "Deployed Mode" and the "Setup Mode". Furthermore the secure boot signature database must only be loaded from tamper-resistant storage. So we must not load it from ubootefi.var on the EFI system partition but only from the preseed variables store or via the OP-TEE driver for the eMMC replay protected memory partition. Heinrich Schuchardt (6): efi_loader: stop recursion in efi_init_secure_state efi_loader: correct determination of secure boot state efi_loader: don't load signature database from file efi_loader: correct secure boot state transition efi_loader: writing AuditMode, DeployedMode efi_loader: always initialize the secure boot state include/efi_variable.h| 6 ++- lib/efi_loader/efi_var_common.c | 66 +++ lib/efi_loader/efi_var_file.c | 41 +++ lib/efi_loader/efi_variable.c | 20 ++ lib/efi_loader/efi_variable_tee.c | 4 +- 5 files changed, 95 insertions(+), 42 deletions(-) -- 2.30.2
[PATCH 4/6] efi_loader: correct secure boot state transition
Variable PK must be deleted when switching either to setup mode or to audit
mode.
Variable AuditMode must be writable in setup mode and user mode.
Variable DeployedMode must only be writable in user mode; simplify the
logic.
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_var_common.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index b0c5b672c5..63ad6fea9e 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -240,7 +240,7 @@ static efi_status_t efi_set_secure_state(u8 secure_boot, u8
setup_mode,
goto err;
ret = efi_set_variable_int(L"AuditMode", _global_variable_guid,
- audit_mode || setup_mode ?
+ audit_mode || deployed_mode ?
attributes_ro : attributes_rw,
sizeof(audit_mode), _mode, false);
if (ret != EFI_SUCCESS)
@@ -248,7 +248,7 @@ static efi_status_t efi_set_secure_state(u8 secure_boot, u8
setup_mode,
ret = efi_set_variable_int(L"DeployedMode",
_global_variable_guid,
- audit_mode || deployed_mode || setup_mode ?
+ deployed_mode || setup_mode ?
attributes_ro : attributes_rw,
sizeof(deployed_mode), _mode,
false);
@@ -273,17 +273,20 @@ static efi_status_t efi_transfer_secure_state(enum
efi_secure_mode mode)
EFI_PRINT("Switching secure state from %d to %d\n", efi_secure_mode,
mode);
- if (mode == EFI_MODE_DEPLOYED) {
- ret = efi_set_secure_state(1, 0, 0, 1);
- if (ret != EFI_SUCCESS)
- goto err;
- } else if (mode == EFI_MODE_AUDIT) {
+ if (mode == EFI_MODE_SETUP || mode == EFI_MODE_AUDIT) {
ret = efi_set_variable_int(L"PK", _global_variable_guid,
EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS,
0, NULL, false);
+ if (ret != EFI_NOT_FOUND && ret != EFI_SUCCESS)
+ goto err;
+ }
+
+ if (mode == EFI_MODE_DEPLOYED) {
+ ret = efi_set_secure_state(1, 0, 0, 1);
if (ret != EFI_SUCCESS)
goto err;
+ } else if (mode == EFI_MODE_AUDIT) {
ret = efi_set_secure_state(0, 1, 1, 0);
if (ret != EFI_SUCCESS)
--
2.30.2
[PATCH 6/6] efi_loader: always initialize the secure boot state
Even if we cannot read the variable store from disk we still need to
initialize the secure boot state.
Don't continue to boot if the variable preseed is invalid as this indicates
that the variable store has been tampered.
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_variable.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 80996d0f47..6d92229e2a 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -427,13 +427,17 @@ efi_status_t efi_init_variables(void)
if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
ret = efi_var_restore((struct efi_var_file *)
__efi_var_file_begin, true);
- if (ret != EFI_SUCCESS)
+ if (ret != EFI_SUCCESS) {
log_err("Invalid EFI variable seed\n");
+ return ret;
+ }
}
-
- ret = efi_var_from_file();
+ ret = efi_init_secure_state();
if (ret != EFI_SUCCESS)
return ret;
- return efi_init_secure_state();
+ /* Don't stop booting if variable store is not available */
+ efi_var_from_file();
+
+ return EFI_SUCCESS;
}
--
2.30.2
[PATCH 5/6] efi_loader: writing AuditMode, DeployedMode
Writing variables AuditMode or Deployed Mode must update the secure boot
state.
Signed-off-by: Heinrich Schuchardt
---
include/efi_variable.h| 1 +
lib/efi_loader/efi_var_common.c | 2 ++
lib/efi_loader/efi_variable.c | 6 +++---
lib/efi_loader/efi_variable_tee.c | 4 +++-
4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h
index 2d97655e1f..0440d356bc 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -12,6 +12,7 @@
enum efi_auth_var_type {
EFI_AUTH_VAR_NONE = 0,
+ EFI_AUTH_MODE,
EFI_AUTH_VAR_PK,
EFI_AUTH_VAR_KEK,
EFI_AUTH_VAR_DB,
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 63ad6fea9e..6fabcfe72c 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -34,6 +34,8 @@ static const struct efi_auth_var_name_type name_type[] = {
{u"dbx", _guid_image_security_database, EFI_AUTH_VAR_DBX},
{u"dbt", _guid_image_security_database, EFI_AUTH_VAR_DBT},
{u"dbr", _guid_image_security_database, EFI_AUTH_VAR_DBR},
+ {u"AuditMode", _global_variable_guid, EFI_AUTH_MODE},
+ {u"DeployedMode", _global_variable_guid, EFI_AUTH_MODE},
};
static bool efi_secure_boot;
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index a7d305ffbc..80996d0f47 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -247,7 +247,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
return EFI_WRITE_PROTECTED;
if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) {
- if (var_type != EFI_AUTH_VAR_NONE)
+ if (var_type >= EFI_AUTH_VAR_PK)
return EFI_WRITE_PROTECTED;
}
@@ -268,7 +268,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
return EFI_NOT_FOUND;
}
- if (var_type != EFI_AUTH_VAR_NONE) {
+ if (var_type >= EFI_AUTH_VAR_PK) {
/* authentication is mandatory */
if (!(attributes &
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) {
@@ -328,7 +328,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
if (ret != EFI_SUCCESS)
return ret;
- if (var_type == EFI_AUTH_VAR_PK)
+ if (var_type == EFI_AUTH_VAR_PK || var_type == EFI_AUTH_MODE)
ret = efi_init_secure_state();
else
ret = EFI_SUCCESS;
diff --git a/lib/efi_loader/efi_variable_tee.c
b/lib/efi_loader/efi_variable_tee.c
index 51920bcb51..e5256c1416 100644
--- a/lib/efi_loader/efi_variable_tee.c
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -512,6 +512,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
efi_uintn_t payload_size;
efi_uintn_t name_size;
u8 *comm_buf = NULL;
+ enum efi_auth_var_type var_type;
bool ro;
if (!variable_name || variable_name[0] == 0 || !vendor) {
@@ -590,7 +591,8 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const
efi_guid_t *vendor,
if (alt_ret != EFI_SUCCESS)
goto out;
- if (!u16_strcmp(variable_name, L"PK"))
+ var_type = efi_auth_var_get_type(variable, vendor);
+ if (var_type == EFI_AUTH_VAR_PK || var_type == EFI_AUTH_MODE)
alt_ret = efi_init_secure_state();
out:
free(comm_buf);
--
2.30.2
[PATCH 1/6] efi_loader: stop recursion in efi_init_secure_state
efi_init_secure_state() calls efi_transfer_secure_state() which may delete
variable "PK" which will result in calling efi_init_secure_state() again.
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_var_common.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 3d92afe2eb..654ce81f9d 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -314,11 +314,15 @@ err:
efi_status_t efi_init_secure_state(void)
{
+ static bool lock;
enum efi_secure_mode mode = EFI_MODE_SETUP;
u8 efi_vendor_keys = 0;
efi_uintn_t size = 0;
efi_status_t ret;
+ if (lock)
+ return EFI_SUCCESS;
+
ret = efi_get_variable_int(L"PK", _global_variable_guid,
NULL, , NULL, NULL);
if (ret == EFI_BUFFER_TOO_SMALL) {
@@ -326,7 +330,9 @@ efi_status_t efi_init_secure_state(void)
mode = EFI_MODE_USER;
}
+ lock = true;
ret = efi_transfer_secure_state(mode);
+ lock = false;
if (ret != EFI_SUCCESS)
return ret;
--
2.30.2
[PATCH 2/6] efi_loader: correct determination of secure boot state
When U-Boot is started we have to use the existing variables to determine
in which secure boot state we are.
* If a platform key PK is present and DeployedMode=1, we are in deployed
mode.
* If no platform key PK is present and AuditMode=1, we are in audit mode.
* Otherwise if a platform key is present, we are in user mode.
* Otherwise if no platform key is present, we are in setup mode.
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_var_common.c | 39 ++---
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 654ce81f9d..cf7afecd60 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -315,20 +315,43 @@ err:
efi_status_t efi_init_secure_state(void)
{
static bool lock;
- enum efi_secure_mode mode = EFI_MODE_SETUP;
+ enum efi_secure_mode mode;
u8 efi_vendor_keys = 0;
- efi_uintn_t size = 0;
+ efi_uintn_t size;
efi_status_t ret;
if (lock)
return EFI_SUCCESS;
-
- ret = efi_get_variable_int(L"PK", _global_variable_guid,
- NULL, , NULL, NULL);
- if (ret == EFI_BUFFER_TOO_SMALL) {
- if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT))
- mode = EFI_MODE_USER;
+ u8 deployed_mode = 0;
+ u8 audit_mode = 0;
+ u8 setup_mode = 1;
+
+ if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) {
+ size = sizeof(deployed_mode);
+ ret = efi_get_variable_int(u"DeployedMode",
_global_variable_guid,
+ NULL, , _mode, NULL);
+ size = sizeof(audit_mode);
+ ret = efi_get_variable_int(u"AuditMode",
_global_variable_guid,
+ NULL, , _mode, NULL);
+ size = 0;
+ ret = efi_get_variable_int(u"PK", _global_variable_guid,
+ NULL, , NULL, NULL);
+ if (ret == EFI_BUFFER_TOO_SMALL) {
+ setup_mode = 0;
+ audit_mode = 0;
+ } else {
+ setup_mode = 1;
+ deployed_mode = 0;
+ }
}
+ if (deployed_mode)
+ mode = EFI_MODE_DEPLOYED;
+ else if (audit_mode)
+ mode = EFI_MODE_AUDIT;
+ else if (setup_mode)
+ mode = EFI_MODE_SETUP;
+ else
+ mode = EFI_MODE_USER;
lock = true;
ret = efi_transfer_secure_state(mode);
--
2.30.2
Re: [PATCH v2 5/6] arm: Use armv8_switch_to_el1 env to switch to EL1
On 8/20/21 6:05 PM, Tom Rini wrote:
On Fri, Aug 20, 2021 at 05:57:51PM +0100, Andre Przywara wrote:
On 8/19/21 4:53 PM, Peter Hoyes wrote:
Hi,
From: Peter Hoyes
Use the environment variable armv8_switch_to_el1 to determine whether
to switch to EL1 at runtime. This is an alternative to the
CONFIG_ARMV8_SWITCH_TO_EL1 compile-time option.
This might be useful outside of the v8-R64 FVP. I cannot find
CONFIG_ARMV8_SWITCH_TO_EL1 being set anywhere, which makes me wonder how
this is used? Are there certain custom builds which define this somehow?
Adding Michal ...
Michal, I see that Xilinx Versal and ZynqMP have a commented "define
CONFIG_ARMV8_SWITCH_TO_EL1" in their include headers. Can you say what
is the use case here?
From an architectural point of view it sounds useful to have this
feature in general and the dynamic switch-ability on top of that for all
ARM64 platforms, but I would first like to understand how this is used
with those Xilink platforms.
asking for a Kconfig migration too :)
That sounds reasonable. I will have a stab once we agree how to address
this feature in general.
Cheers,
Andre
In any case forcing "kernel" entry in either EL1 or EL2, and deciding this
at runtime sounds useful for certain scenarios in general, and be it for
debugging and testing. So shall we get rid of this compile time option at
all, or shall this be retained to avoid extra code?
The environment variable will be ineffective if the ARMV8_MULTIENTRY
config is used.
This is required by the Armv8r64 architecture, which must be able to
boot at S-EL1 for Linux but may need to boot at other ELs for other
systems.
Signed-off-by: Peter Hoyes
---
arch/arm/lib/bootm.c | 40 +---
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index f60ee3a7e6..ea9bfe7570 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -317,7 +317,6 @@ __weak void update_os_arch_secondary_cores(uint8_t os_arch)
{
}
-#ifdef CONFIG_ARMV8_SWITCH_TO_EL1
static void switch_to_el1(void)
{
if ((IH_ARCH_DEFAULT == IH_ARCH_ARM64) &&
@@ -332,7 +331,6 @@ static void switch_to_el1(void)
ES_TO_AARCH64);
}
#endif
-#endif
/* Subcommand: GO */
static void boot_jump_linux(bootm_headers_t *images, int flag)
@@ -359,21 +357,33 @@ static void boot_jump_linux(bootm_headers_t *images, int
flag)
update_os_arch_secondary_cores(images->os.arch);
-#ifdef CONFIG_ARMV8_SWITCH_TO_EL1
- armv8_switch_to_el2((u64)images->ft_addr, 0, 0, 0,
- (u64)switch_to_el1, ES_TO_AARCH64);
+#ifdef CONFIG_ARMV8_MULTIENTRY
+ int armv8_switch_to_el1 = -1;
#else
- if ((IH_ARCH_DEFAULT == IH_ARCH_ARM64) &&
- (images->os.arch == IH_ARCH_ARM))
- armv8_switch_to_el2(0, (u64)gd->bd->bi_arch_number,
- (u64)images->ft_addr, 0,
- (u64)images->ep,
- ES_TO_AARCH32);
- else
- armv8_switch_to_el2((u64)images->ft_addr, 0, 0, 0,
- images->ep,
- ES_TO_AARCH64);
+ int armv8_switch_to_el1 = env_get_yesno("armv8_switch_to_el1");
#endif
+#ifdef CONFIG_ARMV8_SWITCH_TO_EL1
+ if (armv8_switch_to_el1 == -1) {
+ armv8_switch_to_el1 = 1;
+ }
+#endif
+ if (armv8_switch_to_el1 == 1) {
This looks confusing. Can't we use CONFIG_IS_ENABLED() and override
armv8_switch_to_el1, then use this one variable to trigger the action?
Cheers,
Andre
+ armv8_switch_to_el2((u64)images->ft_addr, 0, 0, 0,
+ (u64)switch_to_el1, ES_TO_AARCH64);
+ } else {
+ if ((IH_ARCH_DEFAULT == IH_ARCH_ARM64) &&
+ (images->os.arch == IH_ARCH_ARM))
+ armv8_switch_to_el2(0,
+ (u64)gd->bd->bi_arch_number,
+ (u64)images->ft_addr, 0,
+ (u64)images->ep,
+ ES_TO_AARCH32);
+ else
+ armv8_switch_to_el2((u64)images->ft_addr,
+ 0, 0, 0,
+ images->ep,
+ ES_TO_AARCH64);
+ }
}
#else
unsigned long machid = gd->bd->bi_arch_number;
Rename of the u-boot-atmel custodian tree
Hello everyone, I plan to rename the u-boot-atmel tree to u-boot-at91 . It's been more than 5 years since Atmel is part of Microchip, and the name is slowly being changed to reflect the new reality. It makes more sense to have the custodian tree named u-boot-at91 , as this tree is used only for patches related to the at91 boards and drivers (not any other former Atmel product, nor any other Microchip product ). I won't be naming it u-boot-microchip since Microchip does much more than just the former Atmel MPUs (AT91 architecture) . This means, however, that all remotes to this tree have to be updated. I do not see any other side effect at this moment. I will wait some more time , few weeks or one month, before making this change, to allow anyone to comment on this. Thanks, Eugen
[PATCH v2] board: ea: mx7ulp_com: move setting CONFIG_BOOTCOMMAND to defconfig
From: Ricardo Salveti
Move setting CONFIG_BOOTCOMMAND to the mx7ulp_com_defconfig file.
It also allows replacing the default CONFIG_BOOTCOMMAND without
code modification.
Signed-off-by: Ricardo Salveti
Signed-off-by: Oleksandr Suvorov
---
Changes in v2:
- move setting the command to defconfig
configs/mx7ulp_com_defconfig | 2 ++
include/configs/mx7ulp_com.h | 5 -
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/configs/mx7ulp_com_defconfig b/configs/mx7ulp_com_defconfig
index 62992fecdf..5fcf25a7fb 100644
--- a/configs/mx7ulp_com_defconfig
+++ b/configs/mx7ulp_com_defconfig
@@ -9,6 +9,8 @@ CONFIG_DEFAULT_DEVICE_TREE="imx7ulp-com"
CONFIG_LDO_ENABLED_MODE=y
CONFIG_TARGET_MX7ULP_COM=y
CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/ea/mx7ulp_com/imximage.cfg"
+CONFIG_USE_BOOTCOMMAND=y
+CONFIG_BOOTCOMMAND="if run loadimage; then run mmcboot; fi"
CONFIG_DEFAULT_FDT_FILE="imx7ulp-com"
CONFIG_BOARD_EARLY_INIT_F=y
CONFIG_HUSH_PARSER=y
diff --git a/include/configs/mx7ulp_com.h b/include/configs/mx7ulp_com.h
index 28672c4f94..3ef41e565a 100644
--- a/include/configs/mx7ulp_com.h
+++ b/include/configs/mx7ulp_com.h
@@ -69,11 +69,6 @@
"bootz ${loadaddr} - ${fdt_addr}; " \
"fi;\0" \
-#define CONFIG_BOOTCOMMAND \
- "if run loadimage; then " \
- "run mmcboot; " \
- "fi; " \
-
#define CONFIG_SYS_LOAD_ADDR CONFIG_LOADADDR
#define CONFIG_SYS_INIT_RAM_ADDR IRAM_BASE_ADDR
--
2.31.1
[PATCH] lx2162a : Rename emmc boot command variable
From: Meenakshi Aggarwal
Rename emmc_bootcmd environment variable to sd2_bootcmd
to fix emmc boot on lx2162aqds board.
Signed-off-by: Meenakshi Aggarwal
---
include/configs/lx2162aqds.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/configs/lx2162aqds.h b/include/configs/lx2162aqds.h
index 847534c550..d2145db6f1 100644
--- a/include/configs/lx2162aqds.h
+++ b/include/configs/lx2162aqds.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0+ */
/*
- * Copyright 2020 NXP
+ * Copyright 2020-2021 NXP
*/
#ifndef __LX2162_QDS_H
@@ -65,7 +65,7 @@ u8 qixis_esdhc_detect_quirk(void);
"$kernelhdr_addr_sd $kernelhdr_size_sd "\
" && esbc_validate ${kernelheader_addr_r};" \
"bootm $load_addr#$BOARD\0" \
- "emmc_bootcmd=echo Trying load from emmc card..;" \
+ "sd2_bootcmd=echo Trying load from emmc card..;"\
"mmc dev 1; mmcinfo; mmc read $load_addr " \
"$kernel_addr_sd $kernel_size_sd ;" \
"env exists secureboot && mmc read $kernelheader_addr_r "\
--
2.25.1
Fwd: Re: [PATCH v2 15/16] clk: Detect failure to set defaults
Hi Stefano and Peng,
There is an issue that prevents the imx8mn to boot in 2021.10-rc2. See
the conversation below. Could you help with this?
-- Harm
Forwarded Message
From: Simon Glass
To: Harm Berntsen
Cc: u-boot@lists.denx.de , tr...@konsulko.com
Subject: Re: [PATCH v2 15/16] clk: Detect failure to set defaults
Date: Fri, 20 Aug 2021 12:18:07 -0600
> Hi Harm,
>
> On Wed, 18 Aug 2021 at 08:09, Harm Berntsen
> wrote:
> >
> > On Thu, 2021-05-13 at 19:39 -0600, Simon Glass wrote:
> > > When the default clocks cannot be set, the clock is silently
> > > probed and
> > > the error is ignored. This is incorrect, since having the clocks
> > > at the
> > > correct speed may be important for operation of the system.
> > >
> > > Fix it by checking the return code.
> > >
> > > Signed-off-by: Simon Glass
> > > ---
> > >
> > > (no changes since v1)
> > >
> > > drivers/clk/clk-uclass.c | 6 +-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> > > index 4ab3c402ed8..2a2e1cfbd61 100644
> > > --- a/drivers/clk/clk-uclass.c
> > > +++ b/drivers/clk/clk-uclass.c
> > > @@ -796,13 +796,17 @@ void devm_clk_put(struct udevice *dev,
> > > struct clk
> > > *clk)
> > >
> > > int clk_uclass_post_probe(struct udevice *dev)
> > > {
> > > + int ret;
> > > +
> > > /*
> > > * when a clock provider is probed. Call
> > > clk_set_defaults()
> > > * also after the device is probed. This takes care of
> > > cases
> > > * where the DT is used to setup default parents and
> > > rates
> > > * using assigned-clocks
> > > */
> > > - clk_set_defaults(dev, 1);
> > > + ret = clk_set_defaults(dev, 1);
> > > + if (ret)
> > > + return log_ret(ret);
> > >
> > > return 0;
> > > }
> >
> > Note that this patch broke booting my imx8mn based board on U-Boot
> > v2021.10-rc2. The failure is due to the clock-controller@3038
> > configuration in the imx8mn.dtsi file. I had to remove the
> > following
> > clocks from the device tree to get my device to boot again (all
> > from
> > the assigned-clocks of clock-controller@3038):
> >
> > < IMX8MN_CLK_A53_CORE>,
> > < IMX8MN_CLK_NOC>,
> > < IMX8MN_CLK_AUDIO_AHB>,
> > < IMX8MN_CLK_IPG_AUDIO_ROOT>,
> > < IMX8MN_SYS_PLL3>,
> > < IMX8MN_AUDIO_PLL1>,
> > < IMX8MN_AUDIO_PLL2>;
> >
> > I looked into the clk-imx8mn.c code and I see that we indeed miss
> > clocks there. Unfortunately I could not port code from the Linux
> > kernel: we are missing the imx_clk_hw_mux2 function for the
> > IMX8MN_CLK_A53_CORE clock. I did not look into the other clocks.
>
>
> Perhaps the iMX maintainer could help with this? It does sound like a
> bug.
>
> Regards,
> SImon
>
> >
> >
> > -- Harm
[PATCH v2] arm: dts: imx8mp: Generate single bootable binary
binman conversion made flashing flash.bin
and u-boot.itb necessary. Update binman config
to create a single flash.bin image again.
This updates imx8mp_evk and phyCORE-i.MX8MP as they share the
same binman config.
Updated also imx8mp_evk documentation.
Tested on phyCORE-i.MX8MP.
Signed-off-by: Teresa Remmet
Reviewed-by: Fabio Estevam
Reviewed-by: Heiko Schocher
---
Changes in v2:
- Added filename to spl node
- Use u-boot-spl-ddr.bin in the LOADER section
arch/arm/dts/imx8mp-u-boot.dtsi | 19 ++-
.../imx8mp_evk/imximage-8mp-lpddr4.cfg| 2 +-
.../phytec/phycore_imx8mp/imximage-8mp-sd.cfg | 2 +-
doc/board/nxp/imx8mp_evk.rst | 1 -
4 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi b/arch/arm/dts/imx8mp-u-boot.dtsi
index a6ede2ed6e30..120c4c4dbb13 100644
--- a/arch/arm/dts/imx8mp-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-u-boot.dtsi
@@ -82,7 +82,9 @@
};
};
- flash {
+ spl {
+ filename = "spl.bin";
+
mkimage {
args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e
0x92";
@@ -149,4 +151,19 @@
};
};
};
+
+ imx-boot {
+ filename = "flash.bin";
+ pad-byte = <0x00>;
+
+ spl: blob-ext@1 {
+ filename = "spl.bin";
+ offset = <0x0>;
+ };
+
+ uboot: blob-ext@2 {
+ filename = "u-boot.itb";
+ offset = <0x58000>;
+ };
+ };
};
diff --git a/board/freescale/imx8mp_evk/imximage-8mp-lpddr4.cfg
b/board/freescale/imx8mp_evk/imximage-8mp-lpddr4.cfg
index b2920b4908ed..4c3ecf5a71ad 100644
--- a/board/freescale/imx8mp_evk/imximage-8mp-lpddr4.cfg
+++ b/board/freescale/imx8mp_evk/imximage-8mp-lpddr4.cfg
@@ -7,4 +7,4 @@
ROM_VERSIONv2
BOOT_FROM sd
-LOADER mkimage.flash.mkimage 0x92
+LOADER u-boot-spl-ddr.bin 0x92
diff --git a/board/phytec/phycore_imx8mp/imximage-8mp-sd.cfg
b/board/phytec/phycore_imx8mp/imximage-8mp-sd.cfg
index b2920b4908ed..4c3ecf5a71ad 100644
--- a/board/phytec/phycore_imx8mp/imximage-8mp-sd.cfg
+++ b/board/phytec/phycore_imx8mp/imximage-8mp-sd.cfg
@@ -7,4 +7,4 @@
ROM_VERSIONv2
BOOT_FROM sd
-LOADER mkimage.flash.mkimage 0x92
+LOADER u-boot-spl-ddr.bin 0x92
diff --git a/doc/board/nxp/imx8mp_evk.rst b/doc/board/nxp/imx8mp_evk.rst
index 609a29f3ebc3..b996ae055e69 100644
--- a/doc/board/nxp/imx8mp_evk.rst
+++ b/doc/board/nxp/imx8mp_evk.rst
@@ -52,7 +52,6 @@ Burn the flash.bin to the MicroSD card at offset 32KB:
.. code-block:: bash
$sudo dd if=build/flash.bin of=/dev/sd[x] bs=1K seek=32 conv=notrunc; sync
- $sudo dd if=build/u-boot.itb of=/dev/sd[x] bs=1K seek=384 conv=notrunc; sync
Boot
--
2.25.1
