[PATCH] bootstd: Replicate the dtb-filename quirks of distroboot
For EFI, the distro boot scripts search in three different directories for the .dtb file. The SOC-based filename fallback is supported only for 32-bit ARM.
Adjust the code to mirror this behaviour.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Mark Kettenis kettenis@openbsd.org ---
boot/bootmeth_efi.c | 63 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 9 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 67c972e3fe4..f9544846e68 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -147,25 +147,57 @@ static int distro_efi_check(struct udevice *dev, struct bootflow_iter *iter) return 0; }
-static void distro_efi_get_fdt_name(char *fname, int size) +/** + * distro_efi_get_fdt_name() - Get the filename for reading the .dtb file + * + * @fname: Place to put filename + * @size: Max size of filename + * @seq: Sequence number, to cycle through options (0=first) + * Returns: 0 on success, -ENOENT if the "fdtfile" env var does not exist, + * -EINVAL if there are no more options + */ +static int distro_efi_get_fdt_name(char *fname, int size, int seq) { const char *fdt_fname; + const char *prefix; + + /* select the prefix */ + switch (seq) { + case 0: + /* this is the default */ + prefix = "/dtb"; + break; + case 1: + prefix = ""; + break; + case 2: + prefix = "/dtb/current"; + break; + default: + return log_msg_ret("pref", -EINVAL); + }
fdt_fname = env_get("fdtfile"); if (fdt_fname) { - snprintf(fname, size, "dtb/%s", fdt_fname); + snprintf(fname, size, "%s/%s", prefix, fdt_fname); log_debug("Using device tree: %s\n", fname); - } else { + + /* Use this fallback only for 32-bit ARM */ + } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) { const char *soc = env_get("soc"); const char *board = env_get("board"); const char *boardver = env_get("boardver");
/* cf the code in label_boot() which seems very complex */ - snprintf(fname, size, "dtb/%s%s%s%s.dtb", + snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix, soc ? soc : "", soc ? "-" : "", board ? board : "", boardver ? boardver : ""); log_debug("Using default device tree: %s\n", fname); + } else { + return log_msg_ret("env", -ENOENT); } + + return 0; }
static int distro_efi_read_bootflow_file(struct udevice *dev, @@ -174,7 +206,7 @@ static int distro_efi_read_bootflow_file(struct udevice *dev, struct blk_desc *desc = NULL; ulong fdt_addr, size; char fname[256]; - int ret; + int ret, seq;
/* We require a partition table */ if (!bflow->part) @@ -196,13 +228,22 @@ static int distro_efi_read_bootflow_file(struct udevice *dev, if (ret) return log_msg_ret("read", -EINVAL);
- distro_efi_get_fdt_name(fname, sizeof(fname)); + fdt_addr = env_get_hex("fdt_addr_r", 0); + + /* try the various available names */ + ret = -ENOENT; + for (seq = 0; ret; seq++) { + ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq); + if (ret) + return log_msg_ret("nam", ret); + ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr, + &size); + } + bflow->fdt_fname = strdup(fname); if (!bflow->fdt_fname) return log_msg_ret("fil", -ENOMEM);
- fdt_addr = env_get_hex("fdt_addr_r", 0); - ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr, &size); if (!ret) { bflow->fdt_size = size; bflow->fdt_addr = fdt_addr; @@ -277,7 +318,11 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow) fdt_addr = hextoul(fdt_addr_str, NULL); sprintf(file_addr, "%lx", fdt_addr);
- distro_efi_get_fdt_name(fname, sizeof(fname)); + /* We only allow the first prefix with PXE */ + ret = distro_efi_get_fdt_name(fname, sizeof(fname), 0); + if (ret) + return log_msg_ret("nam", ret); + bflow->fdt_fname = strdup(fname); if (!bflow->fdt_fname) return log_msg_ret("fil", -ENOMEM);
On 1/25/23 04:11, Simon Glass wrote:
For EFI, the distro boot scripts search in three different directories for the .dtb file. The SOC-based filename fallback is supported only for 32-bit ARM.
Adjust the code to mirror this behaviour.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Mark Kettenis kettenis@openbsd.org
boot/bootmeth_efi.c | 63 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 9 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 67c972e3fe4..f9544846e68 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -147,25 +147,57 @@ static int distro_efi_check(struct udevice *dev, struct bootflow_iter *iter) return 0; }
-static void distro_efi_get_fdt_name(char *fname, int size) +/**
- distro_efi_get_fdt_name() - Get the filename for reading the .dtb file
- @fname: Place to put filename
- @size: Max size of filename
- @seq: Sequence number, to cycle through options (0=first)
- Returns: 0 on success, -ENOENT if the "fdtfile" env var does not exist,
- -EINVAL if there are no more options
- */
+static int distro_efi_get_fdt_name(char *fname, int size, int seq) { const char *fdt_fname;
const char *prefix;
/* select the prefix */
switch (seq) {
case 0:
/* this is the default */prefix = "/dtb";break;case 1:
prefix = "";break;case 2:
prefix = "/dtb/current";break;default:
return log_msg_ret("pref", -EINVAL);}
fdt_fname = env_get("fdtfile"); if (fdt_fname) {
snprintf(fname, size, "dtb/%s", fdt_fname);
snprintf(fname, size, "%s/%s", prefix, fdt_fname);
- } else {
/* Use this fallback only for 32-bit ARM */
} else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) { const char *soc = env_get("soc"); const char *board = env_get("board"); const char *boardver = env_get("boardver");
/* cf the code in label_boot() which seems very complex */
snprintf(fname, size, "dtb/%s%s%s%s.dtb",
snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix, soc ? soc : "", soc ? "-" : "", board ? board : "", boardver ? boardver : "");log_debug("Using default device tree: %s\n", fname);
} else {
return log_msg_ret("env", -ENOENT);}
return 0; }
static int distro_efi_read_bootflow_file(struct udevice *dev,
@@ -174,7 +206,7 @@ static int distro_efi_read_bootflow_file(struct udevice *dev, struct blk_desc *desc = NULL; ulong fdt_addr, size; char fname[256];
- int ret;
int ret, seq;
/* We require a partition table */ if (!bflow->part)
@@ -196,13 +228,22 @@ static int distro_efi_read_bootflow_file(struct udevice *dev, if (ret) return log_msg_ret("read", -EINVAL);
- distro_efi_get_fdt_name(fname, sizeof(fname));
- fdt_addr = env_get_hex("fdt_addr_r", 0);
- /* try the various available names */
- ret = -ENOENT;
- for (seq = 0; ret; seq++)
There are boards that don't have a dtb file available and that is ok. Don't loop past seq = 2.
{
ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);if (ret)return log_msg_ret("nam", ret);ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr,&size);- }
- bflow->fdt_fname = strdup(fname); if (!bflow->fdt_fname) return log_msg_ret("fil", -ENOMEM);
This should not be an error. The Distroboot fallback is the device-tree at $fdtcontroladdr.
Best regards
Heinrich
- fdt_addr = env_get_hex("fdt_addr_r", 0);
- ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr, &size); if (!ret) { bflow->fdt_size = size; bflow->fdt_addr = fdt_addr;
@@ -277,7 +318,11 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow) fdt_addr = hextoul(fdt_addr_str, NULL); sprintf(file_addr, "%lx", fdt_addr);
- distro_efi_get_fdt_name(fname, sizeof(fname));
- /* We only allow the first prefix with PXE */
- ret = distro_efi_get_fdt_name(fname, sizeof(fname), 0);
- if (ret)
return log_msg_ret("nam", ret);- bflow->fdt_fname = strdup(fname); if (!bflow->fdt_fname) return log_msg_ret("fil", -ENOMEM);
On Wed, Jan 25, 2023 at 04:38:53PM +0100, Heinrich Schuchardt wrote:
On 1/25/23 04:11, Simon Glass wrote:
For EFI, the distro boot scripts search in three different directories for the .dtb file. The SOC-based filename fallback is supported only for 32-bit ARM.
Adjust the code to mirror this behaviour.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Mark Kettenis kettenis@openbsd.org
boot/bootmeth_efi.c | 63 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 9 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 67c972e3fe4..f9544846e68 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -147,25 +147,57 @@ static int distro_efi_check(struct udevice *dev, struct bootflow_iter *iter) return 0; } -static void distro_efi_get_fdt_name(char *fname, int size) +/**
- distro_efi_get_fdt_name() - Get the filename for reading the .dtb file
- @fname: Place to put filename
- @size: Max size of filename
- @seq: Sequence number, to cycle through options (0=first)
- Returns: 0 on success, -ENOENT if the "fdtfile" env var does not exist,
- -EINVAL if there are no more options
- */
+static int distro_efi_get_fdt_name(char *fname, int size, int seq) { const char *fdt_fname;
- const char *prefix;
- /* select the prefix */
- switch (seq) {
- case 0:
/* this is the default */prefix = "/dtb";break;- case 1:
prefix = "";break;- case 2:
prefix = "/dtb/current";break;- default:
return log_msg_ret("pref", -EINVAL);- } fdt_fname = env_get("fdtfile"); if (fdt_fname) {
snprintf(fname, size, "dtb/%s", fdt_fname);
snprintf(fname, size, "%s/%s", prefix, fdt_fname);
- } else {
- /* Use this fallback only for 32-bit ARM */
- } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) { const char *soc = env_get("soc"); const char *board = env_get("board"); const char *boardver = env_get("boardver"); /* cf the code in label_boot() which seems very complex */
snprintf(fname, size, "dtb/%s%s%s%s.dtb",
snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix, soc ? soc : "", soc ? "-" : "", board ? board : "", boardver ? boardver : "");- } else {
return log_msg_ret("env", -ENOENT);- return 0; } static int distro_efi_read_bootflow_file(struct udevice *dev,
@@ -174,7 +206,7 @@ static int distro_efi_read_bootflow_file(struct udevice *dev, struct blk_desc *desc = NULL; ulong fdt_addr, size; char fname[256];
- int ret;
- int ret, seq; /* We require a partition table */ if (!bflow->part)
@@ -196,13 +228,22 @@ static int distro_efi_read_bootflow_file(struct udevice *dev, if (ret) return log_msg_ret("read", -EINVAL);
- distro_efi_get_fdt_name(fname, sizeof(fname));
- fdt_addr = env_get_hex("fdt_addr_r", 0);
- /* try the various available names */
- ret = -ENOENT;
- for (seq = 0; ret; seq++)
There are boards that don't have a dtb file available and that is ok. Don't loop past seq = 2.
{
ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);if (ret)return log_msg_ret("nam", ret);ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr,&size);- }
- bflow->fdt_fname = strdup(fname); if (!bflow->fdt_fname) return log_msg_ret("fil", -ENOMEM);
This should not be an error. The Distroboot fallback is the device-tree at $fdtcontroladdr.
But it should note that it's using that because that's still quite often an unexpected feature to people.
On 1/25/23 17:15, Tom Rini wrote:
On Wed, Jan 25, 2023 at 04:38:53PM +0100, Heinrich Schuchardt wrote:
On 1/25/23 04:11, Simon Glass wrote:
For EFI, the distro boot scripts search in three different directories for the .dtb file. The SOC-based filename fallback is supported only for 32-bit ARM.
Adjust the code to mirror this behaviour.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Mark Kettenis kettenis@openbsd.org
boot/bootmeth_efi.c | 63 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 9 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 67c972e3fe4..f9544846e68 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -147,25 +147,57 @@ static int distro_efi_check(struct udevice *dev, struct bootflow_iter *iter) return 0; } -static void distro_efi_get_fdt_name(char *fname, int size) +/**
- distro_efi_get_fdt_name() - Get the filename for reading the .dtb file
- @fname: Place to put filename
- @size: Max size of filename
- @seq: Sequence number, to cycle through options (0=first)
- Returns: 0 on success, -ENOENT if the "fdtfile" env var does not exist,
- -EINVAL if there are no more options
- */
+static int distro_efi_get_fdt_name(char *fname, int size, int seq) { const char *fdt_fname;
- const char *prefix;
- /* select the prefix */
- switch (seq) {
- case 0:
/* this is the default */prefix = "/dtb";break;- case 1:
prefix = "";break;- case 2:
prefix = "/dtb/current";break;- default:
return log_msg_ret("pref", -EINVAL);- } fdt_fname = env_get("fdtfile"); if (fdt_fname) {
snprintf(fname, size, "dtb/%s", fdt_fname);
snprintf(fname, size, "%s/%s", prefix, fdt_fname); log_debug("Using device tree: %s\n", fname);
- } else {
- /* Use this fallback only for 32-bit ARM */
- } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) { const char *soc = env_get("soc"); const char *board = env_get("board"); const char *boardver = env_get("boardver"); /* cf the code in label_boot() which seems very complex */
snprintf(fname, size, "dtb/%s%s%s%s.dtb",
snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix, soc ? soc : "", soc ? "-" : "", board ? board : "", boardver ? boardver : ""); log_debug("Using default device tree: %s\n", fname);- } else {
return log_msg_ret("env", -ENOENT);- return 0; } static int distro_efi_read_bootflow_file(struct udevice *dev,
@@ -174,7 +206,7 @@ static int distro_efi_read_bootflow_file(struct udevice *dev, struct blk_desc *desc = NULL; ulong fdt_addr, size; char fname[256];
- int ret;
- int ret, seq; /* We require a partition table */ if (!bflow->part)
@@ -196,13 +228,22 @@ static int distro_efi_read_bootflow_file(struct udevice *dev, if (ret) return log_msg_ret("read", -EINVAL);
- distro_efi_get_fdt_name(fname, sizeof(fname));
- fdt_addr = env_get_hex("fdt_addr_r", 0);
- /* try the various available names */
- ret = -ENOENT;
- for (seq = 0; ret; seq++)
There are boards that don't have a dtb file available and that is ok. Don't loop past seq = 2.
{
ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);if (ret)return log_msg_ret("nam", ret);ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr,&size);- }
- bflow->fdt_fname = strdup(fname); if (!bflow->fdt_fname) return log_msg_ret("fil", -ENOMEM);
This should not be an error. The Distroboot fallback is the device-tree at $fdtcontroladdr.
But it should note that it's using that because that's still quite often an unexpected feature to people.
On QEMU it is just what is needed. Other boards supply the device-tree via TF-A or OpenSBI. We should not start breaking boards.
A message "fil: returning err= -12" signals not caring about users.
Best regards
Heinrich
On Wed, Jan 25, 2023 at 06:04:56PM +0100, Heinrich Schuchardt wrote:
On 1/25/23 17:15, Tom Rini wrote:
On Wed, Jan 25, 2023 at 04:38:53PM +0100, Heinrich Schuchardt wrote:
On 1/25/23 04:11, Simon Glass wrote:
For EFI, the distro boot scripts search in three different directories for the .dtb file. The SOC-based filename fallback is supported only for 32-bit ARM.
Adjust the code to mirror this behaviour.
Signed-off-by: Simon Glass sjg@chromium.org Suggested-by: Mark Kettenis kettenis@openbsd.org
boot/bootmeth_efi.c | 63 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 9 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 67c972e3fe4..f9544846e68 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -147,25 +147,57 @@ static int distro_efi_check(struct udevice *dev, struct bootflow_iter *iter) return 0; } -static void distro_efi_get_fdt_name(char *fname, int size) +/**
- distro_efi_get_fdt_name() - Get the filename for reading the .dtb file
- @fname: Place to put filename
- @size: Max size of filename
- @seq: Sequence number, to cycle through options (0=first)
- Returns: 0 on success, -ENOENT if the "fdtfile" env var does not exist,
- -EINVAL if there are no more options
- */
+static int distro_efi_get_fdt_name(char *fname, int size, int seq) { const char *fdt_fname;
- const char *prefix;
- /* select the prefix */
- switch (seq) {
- case 0:
/* this is the default */prefix = "/dtb";break;- case 1:
prefix = "";break;- case 2:
prefix = "/dtb/current";break;- default:
return log_msg_ret("pref", -EINVAL);- } fdt_fname = env_get("fdtfile"); if (fdt_fname) {
snprintf(fname, size, "dtb/%s", fdt_fname);
snprintf(fname, size, "%s/%s", prefix, fdt_fname); log_debug("Using device tree: %s\n", fname);
- } else {
- /* Use this fallback only for 32-bit ARM */
- } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) { const char *soc = env_get("soc"); const char *board = env_get("board"); const char *boardver = env_get("boardver"); /* cf the code in label_boot() which seems very complex */
snprintf(fname, size, "dtb/%s%s%s%s.dtb",
snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix, soc ? soc : "", soc ? "-" : "", board ? board : "", boardver ? boardver : ""); log_debug("Using default device tree: %s\n", fname);- } else {
return log_msg_ret("env", -ENOENT);- return 0; } static int distro_efi_read_bootflow_file(struct udevice *dev,
@@ -174,7 +206,7 @@ static int distro_efi_read_bootflow_file(struct udevice *dev, struct blk_desc *desc = NULL; ulong fdt_addr, size; char fname[256];
- int ret;
- int ret, seq; /* We require a partition table */ if (!bflow->part)
@@ -196,13 +228,22 @@ static int distro_efi_read_bootflow_file(struct udevice *dev, if (ret) return log_msg_ret("read", -EINVAL);
- distro_efi_get_fdt_name(fname, sizeof(fname));
- fdt_addr = env_get_hex("fdt_addr_r", 0);
- /* try the various available names */
- ret = -ENOENT;
- for (seq = 0; ret; seq++)
There are boards that don't have a dtb file available and that is ok. Don't loop past seq = 2.
{
ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);if (ret)return log_msg_ret("nam", ret);ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr,&size);- }
- bflow->fdt_fname = strdup(fname); if (!bflow->fdt_fname) return log_msg_ret("fil", -ENOMEM);
This should not be an error. The Distroboot fallback is the device-tree at $fdtcontroladdr.
But it should note that it's using that because that's still quite often an unexpected feature to people.
On QEMU it is just what is needed. Other boards supply the device-tree via TF-A or OpenSBI. We should not start breaking boards.
A message "fil: returning err= -12" signals not caring about users.
Yes, I'm saying we need a positive message when using the in memory device tree.
participants (3)
-
Heinrich Schuchardt -
Simon Glass -
Tom Rini