Re: [PATCH] bootm: dont use internal oftree fallback by default
On Mon, 5 Dec 2016, Uwe Kleine-König wrote: > On Fri, Dec 02, 2016 at 10:46:38AM +0100, Lucas Stach wrote: > > Am Donnerstag, den 01.12.2016, 19:55 +0100 schrieb Alexander Kurz: > > > Booting via bootm offers several methods to load oftree data. When no > > > dedicated oftree image is provided, barebox checks for the presence of > > > its own internal oftree, assuming it as a good choice for boot. > > > > > > This fallback method breaks the usecase when a modern OF-based barebox > > > is used to boot a legacy ATAG dependent non OF based vendor provided > > > kernel > > > (e.g. ATAGs will be switched off). > > > Unfortunately those kernels are being still actively shipped today. > > > > > > Change barebox according to the principle of least surprise: when no > > > oftree data is proactively configured, then perform a non-oftree boot. > > > Make the fallback-use of the barebox internal oftree an opt-in feature. > > > > > > Note: this will break boards where the boot process relied on this > > > feature, > > > e.g.: oftree based barebox plus oftree based kernel without an explicit > > > given dts. For those boards global.bootm.internal_oftree_fallback=1 should > > > be set. > > > > > The least surprise on a modern oftree based kernel is that the firmware > > (Barebox) provides the DT, if there isn't an explicit override. > > > > So NACK on the patch in it's current form. If you have a board where you > > know that the kernel doesn't play along, you may reverse this patch so > > that you can set global.bootm.no_internal_oftree=1 in your board code. > > Ack for the NAck. And AFAIK you can already today just do > > oftree -f > > before calling bootm to discard the internal dtb which should make > barebox fall back to ATAG. After flushing the DT, ATAGs get passed during boot, this makes my patch obsolete now. Thank you very much, Alexander > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König| > Industrial Linux Solutions | http://www.pengutronix.de/ | > ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH] bootm: dont use internal oftree fallback by default
On Fri, 2 Dec 2016, Lucas Stach wrote:
> Am Donnerstag, den 01.12.2016, 19:55 +0100 schrieb Alexander Kurz:
> > Booting via bootm offers several methods to load oftree data. When no
> > dedicated oftree image is provided, barebox checks for the presence of
> > its own internal oftree, assuming it as a good choice for boot.
> >
> > This fallback method breaks the usecase when a modern OF-based barebox
> > is used to boot a legacy ATAG dependent non OF based vendor provided kernel
> > (e.g. ATAGs will be switched off).
> > Unfortunately those kernels are being still actively shipped today.
> >
> > Change barebox according to the principle of least surprise: when no
> > oftree data is proactively configured, then perform a non-oftree boot.
> > Make the fallback-use of the barebox internal oftree an opt-in feature.
> >
> > Note: this will break boards where the boot process relied on this feature,
> > e.g.: oftree based barebox plus oftree based kernel without an explicit
> > given dts. For those boards global.bootm.internal_oftree_fallback=1 should
> > be set.
> >
> The least surprise on a modern oftree based kernel is that the firmware
> (Barebox) provides the DT, if there isn't an explicit override.
I'm just wondering about the consequences in terms of possible backward
and forward DT compatibility issues. DT can get quite complicated and
often contain parts from the SoC and SoM manufacturer and probably also
the device manufacturer. I am not sure if everyone holds the dicipline
maintaining DT stable like the kernel team does.
Explicitly shipping the kernel-DT with the kernel will prevent unwanted
surprises at this point.
> So NACK on the patch in it's current form. If you have a board where you
> know that the kernel doesn't play along, you may reverse this patch so
> that you can set global.bootm.no_internal_oftree=1 in your board code.
I have already considered it as a proposal-for-discussion.
I'll post the patch with inverted logic later on and let the decision to
you.
Regards, Alexander
>
> Regards,
> Lucas
>
> > Signed-off-by: Alexander Kurz
> > ---
> > common/bootm.c | 8 ++--
> > include/bootm.h | 2 ++
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/common/bootm.c b/common/bootm.c
> > index 5984319..f5303fa 100644
> > --- a/common/bootm.c
> > +++ b/common/bootm.c
> > @@ -58,6 +58,7 @@ void bootm_data_init_defaults(struct bootm_data *data)
> > data->initrd_address = UIMAGE_INVALID_ADDRESS;
> > data->os_address = UIMAGE_SOME_ADDRESS;
> > data->oftree_file = getenv_nonempty("global.bootm.oftree");
> > + data->internal_oftree_fallback =
> > getenv_nonempty("global.bootm.internal_oftree_fallback");
> > data->os_file = getenv_nonempty("global.bootm.image");
> > getenv_ul("global.bootm.image.loadaddr", &data->os_address);
> > getenv_ul("global.bootm.initrd.loadaddr", &data->initrd_address);
> > @@ -375,14 +376,15 @@ int bootm_load_devicetree(struct image_data *data,
> > unsigned long load_address)
> > pr_err("unable to unflatten devicetree\n");
> > return -EINVAL;
> > }
> > -
> > - } else {
> > + } else if (data->internal_oftree_fallback) {
> > data->of_root_node = of_get_root_node();
> > if (!data->of_root_node)
> > return 0;
> >
> > if (bootm_verbose(data) > 1 && data->of_root_node)
> > printf("using internal devicetree\n");
> > + } else {
> > + return 0;
> > }
> >
> > if (data->initrd_res) {
> > @@ -530,6 +532,7 @@ int bootm_boot(struct bootm_data *bootm_data)
> > data->verify = bootm_data->verify;
> > data->force = bootm_data->force;
> > data->dryrun = bootm_data->dryrun;
> > + data->internal_oftree_fallback = bootm_data->internal_oftree_fallback;
> > data->initrd_address = bootm_data->initrd_address;
> > data->os_address = bootm_data->os_address;
> > data->os_entry = bootm_data->os_entry;
> > @@ -683,6 +686,7 @@ BAREBOX_MAGICVAR_NAMED(global_bootm_image_loadaddr,
> > global.bootm.image.loadaddr,
> > BAREBOX_MAGICVAR_NAMED(global_bootm_initrd, global.bootm.initrd, "bootm
> > default initrd");
> > BAREBOX_MAGICVAR_NAMED(global_bootm_initrd_loadaddr,
> > global.bootm.initrd.loadaddr, "bootm default initrd loadaddr");
> > BAREBOX_MAGICVAR_NAMED(global_bootm_oftree, global.bootm.oftree, "bootm
> > default oftree");
> > +BAREBOX_MAGICVAR_NAMED(global_bootm_internal_oftree_fallback,
> > global.bootm.internal_oftree_fallback, "use barebox oftree as fallback");
> > BAREBOX_MAGICVAR_NAMED(global_bootm_verify, global.bootm.verify, "bootm
> > default verify level");
> > BAREBOX_MAGICVAR_NAMED(global_bootm_verbose, global.bootm.verify, "bootm
> > default verbosity level (0=quiet)");
> > BAREBOX_MAGICVAR_NAMED(global_bootm_appendroot, global.bootm.appendroot,
> > "Add root= option to Kernel to mount rootfs from the device the Kernel
> > comes fr
Re: [PATCH] bootm: dont use internal oftree fallback by default
On Fri, Dec 02, 2016 at 10:46:38AM +0100, Lucas Stach wrote: > Am Donnerstag, den 01.12.2016, 19:55 +0100 schrieb Alexander Kurz: > > Booting via bootm offers several methods to load oftree data. When no > > dedicated oftree image is provided, barebox checks for the presence of > > its own internal oftree, assuming it as a good choice for boot. > > > > This fallback method breaks the usecase when a modern OF-based barebox > > is used to boot a legacy ATAG dependent non OF based vendor provided kernel > > (e.g. ATAGs will be switched off). > > Unfortunately those kernels are being still actively shipped today. > > > > Change barebox according to the principle of least surprise: when no > > oftree data is proactively configured, then perform a non-oftree boot. > > Make the fallback-use of the barebox internal oftree an opt-in feature. > > > > Note: this will break boards where the boot process relied on this feature, > > e.g.: oftree based barebox plus oftree based kernel without an explicit > > given dts. For those boards global.bootm.internal_oftree_fallback=1 should > > be set. > > > The least surprise on a modern oftree based kernel is that the firmware > (Barebox) provides the DT, if there isn't an explicit override. > > So NACK on the patch in it's current form. If you have a board where you > know that the kernel doesn't play along, you may reverse this patch so > that you can set global.bootm.no_internal_oftree=1 in your board code. Ack for the NAck. And AFAIK you can already today just do oftree -f before calling bootm to discard the internal dtb which should make barebox fall back to ATAG. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH] bootm: dont use internal oftree fallback by default
Am Donnerstag, den 01.12.2016, 19:55 +0100 schrieb Alexander Kurz:
> Booting via bootm offers several methods to load oftree data. When no
> dedicated oftree image is provided, barebox checks for the presence of
> its own internal oftree, assuming it as a good choice for boot.
>
> This fallback method breaks the usecase when a modern OF-based barebox
> is used to boot a legacy ATAG dependent non OF based vendor provided kernel
> (e.g. ATAGs will be switched off).
> Unfortunately those kernels are being still actively shipped today.
>
> Change barebox according to the principle of least surprise: when no
> oftree data is proactively configured, then perform a non-oftree boot.
> Make the fallback-use of the barebox internal oftree an opt-in feature.
>
> Note: this will break boards where the boot process relied on this feature,
> e.g.: oftree based barebox plus oftree based kernel without an explicit
> given dts. For those boards global.bootm.internal_oftree_fallback=1 should
> be set.
>
The least surprise on a modern oftree based kernel is that the firmware
(Barebox) provides the DT, if there isn't an explicit override.
So NACK on the patch in it's current form. If you have a board where you
know that the kernel doesn't play along, you may reverse this patch so
that you can set global.bootm.no_internal_oftree=1 in your board code.
Regards,
Lucas
> Signed-off-by: Alexander Kurz
> ---
> common/bootm.c | 8 ++--
> include/bootm.h | 2 ++
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/common/bootm.c b/common/bootm.c
> index 5984319..f5303fa 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -58,6 +58,7 @@ void bootm_data_init_defaults(struct bootm_data *data)
> data->initrd_address = UIMAGE_INVALID_ADDRESS;
> data->os_address = UIMAGE_SOME_ADDRESS;
> data->oftree_file = getenv_nonempty("global.bootm.oftree");
> + data->internal_oftree_fallback =
> getenv_nonempty("global.bootm.internal_oftree_fallback");
> data->os_file = getenv_nonempty("global.bootm.image");
> getenv_ul("global.bootm.image.loadaddr", &data->os_address);
> getenv_ul("global.bootm.initrd.loadaddr", &data->initrd_address);
> @@ -375,14 +376,15 @@ int bootm_load_devicetree(struct image_data *data,
> unsigned long load_address)
> pr_err("unable to unflatten devicetree\n");
> return -EINVAL;
> }
> -
> - } else {
> + } else if (data->internal_oftree_fallback) {
> data->of_root_node = of_get_root_node();
> if (!data->of_root_node)
> return 0;
>
> if (bootm_verbose(data) > 1 && data->of_root_node)
> printf("using internal devicetree\n");
> + } else {
> + return 0;
> }
>
> if (data->initrd_res) {
> @@ -530,6 +532,7 @@ int bootm_boot(struct bootm_data *bootm_data)
> data->verify = bootm_data->verify;
> data->force = bootm_data->force;
> data->dryrun = bootm_data->dryrun;
> + data->internal_oftree_fallback = bootm_data->internal_oftree_fallback;
> data->initrd_address = bootm_data->initrd_address;
> data->os_address = bootm_data->os_address;
> data->os_entry = bootm_data->os_entry;
> @@ -683,6 +686,7 @@ BAREBOX_MAGICVAR_NAMED(global_bootm_image_loadaddr,
> global.bootm.image.loadaddr,
> BAREBOX_MAGICVAR_NAMED(global_bootm_initrd, global.bootm.initrd, "bootm
> default initrd");
> BAREBOX_MAGICVAR_NAMED(global_bootm_initrd_loadaddr,
> global.bootm.initrd.loadaddr, "bootm default initrd loadaddr");
> BAREBOX_MAGICVAR_NAMED(global_bootm_oftree, global.bootm.oftree, "bootm
> default oftree");
> +BAREBOX_MAGICVAR_NAMED(global_bootm_internal_oftree_fallback,
> global.bootm.internal_oftree_fallback, "use barebox oftree as fallback");
> BAREBOX_MAGICVAR_NAMED(global_bootm_verify, global.bootm.verify, "bootm
> default verify level");
> BAREBOX_MAGICVAR_NAMED(global_bootm_verbose, global.bootm.verify, "bootm
> default verbosity level (0=quiet)");
> BAREBOX_MAGICVAR_NAMED(global_bootm_appendroot, global.bootm.appendroot,
> "Add root= option to Kernel to mount rootfs from the device the Kernel comes
> from");
> diff --git a/include/bootm.h b/include/bootm.h
> index 6e9777a..c945c99 100644
> --- a/include/bootm.h
> +++ b/include/bootm.h
> @@ -20,6 +20,7 @@ struct bootm_data {
> enum bootm_verify verify;
> bool force;
> bool dryrun;
> + bool internal_oftree_fallback;
> /*
>* appendroot - if true, try to add a suitable root= Kernel option to
>* mount the rootfs from the same device as the Kernel comes from.
> @@ -81,6 +82,7 @@ struct image_data {
> int verbose;
> int force;
> int dryrun;
> + int internal_oftree_fallback;
> };
>
> struct image_handler {
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailma
[PATCH] bootm: dont use internal oftree fallback by default
Booting via bootm offers several methods to load oftree data. When no
dedicated oftree image is provided, barebox checks for the presence of
its own internal oftree, assuming it as a good choice for boot.
This fallback method breaks the usecase when a modern OF-based barebox
is used to boot a legacy ATAG dependent non OF based vendor provided kernel
(e.g. ATAGs will be switched off).
Unfortunately those kernels are being still actively shipped today.
Change barebox according to the principle of least surprise: when no
oftree data is proactively configured, then perform a non-oftree boot.
Make the fallback-use of the barebox internal oftree an opt-in feature.
Note: this will break boards where the boot process relied on this feature,
e.g.: oftree based barebox plus oftree based kernel without an explicit
given dts. For those boards global.bootm.internal_oftree_fallback=1 should
be set.
Signed-off-by: Alexander Kurz
---
common/bootm.c | 8 ++--
include/bootm.h | 2 ++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c
index 5984319..f5303fa 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -58,6 +58,7 @@ void bootm_data_init_defaults(struct bootm_data *data)
data->initrd_address = UIMAGE_INVALID_ADDRESS;
data->os_address = UIMAGE_SOME_ADDRESS;
data->oftree_file = getenv_nonempty("global.bootm.oftree");
+ data->internal_oftree_fallback =
getenv_nonempty("global.bootm.internal_oftree_fallback");
data->os_file = getenv_nonempty("global.bootm.image");
getenv_ul("global.bootm.image.loadaddr", &data->os_address);
getenv_ul("global.bootm.initrd.loadaddr", &data->initrd_address);
@@ -375,14 +376,15 @@ int bootm_load_devicetree(struct image_data *data,
unsigned long load_address)
pr_err("unable to unflatten devicetree\n");
return -EINVAL;
}
-
- } else {
+ } else if (data->internal_oftree_fallback) {
data->of_root_node = of_get_root_node();
if (!data->of_root_node)
return 0;
if (bootm_verbose(data) > 1 && data->of_root_node)
printf("using internal devicetree\n");
+ } else {
+ return 0;
}
if (data->initrd_res) {
@@ -530,6 +532,7 @@ int bootm_boot(struct bootm_data *bootm_data)
data->verify = bootm_data->verify;
data->force = bootm_data->force;
data->dryrun = bootm_data->dryrun;
+ data->internal_oftree_fallback = bootm_data->internal_oftree_fallback;
data->initrd_address = bootm_data->initrd_address;
data->os_address = bootm_data->os_address;
data->os_entry = bootm_data->os_entry;
@@ -683,6 +686,7 @@ BAREBOX_MAGICVAR_NAMED(global_bootm_image_loadaddr,
global.bootm.image.loadaddr,
BAREBOX_MAGICVAR_NAMED(global_bootm_initrd, global.bootm.initrd, "bootm
default initrd");
BAREBOX_MAGICVAR_NAMED(global_bootm_initrd_loadaddr,
global.bootm.initrd.loadaddr, "bootm default initrd loadaddr");
BAREBOX_MAGICVAR_NAMED(global_bootm_oftree, global.bootm.oftree, "bootm
default oftree");
+BAREBOX_MAGICVAR_NAMED(global_bootm_internal_oftree_fallback,
global.bootm.internal_oftree_fallback, "use barebox oftree as fallback");
BAREBOX_MAGICVAR_NAMED(global_bootm_verify, global.bootm.verify, "bootm
default verify level");
BAREBOX_MAGICVAR_NAMED(global_bootm_verbose, global.bootm.verify, "bootm
default verbosity level (0=quiet)");
BAREBOX_MAGICVAR_NAMED(global_bootm_appendroot, global.bootm.appendroot, "Add
root= option to Kernel to mount rootfs from the device the Kernel comes from");
diff --git a/include/bootm.h b/include/bootm.h
index 6e9777a..c945c99 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -20,6 +20,7 @@ struct bootm_data {
enum bootm_verify verify;
bool force;
bool dryrun;
+ bool internal_oftree_fallback;
/*
* appendroot - if true, try to add a suitable root= Kernel option to
* mount the rootfs from the same device as the Kernel comes from.
@@ -81,6 +82,7 @@ struct image_data {
int verbose;
int force;
int dryrun;
+ int internal_oftree_fallback;
};
struct image_handler {
--
2.1.4
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
