[PATCH v3 1/2] fastboot: always try barebox_update handler for bbu- partitions

[PATCH v3 1/2] fastboot: always try barebox_update handler for bbu- partitions

[Ahmad Fatoum]

So far, fastboot barebox_update code was only entered when
filetype_is_barebox_image() returned true, a function exclusively used
for fastboot. Align this with normal barebox_update from the command
line by leaving the decision on whether the image is correctly to the
barebox_update handler if the user explicitly targets a bbu- fastboot
partition.

We keep the old check OR-ed to maintain backwards-compatibility for
invoking barebox_update for non bbu- fastboot partitions.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v2 -> v3:
  - no changes
v1 -> v2:
  - new patch (Sascha)
---
 common/fastboot.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index f8ed40c86e00..330a06f5a32f 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -673,7 +673,8 @@ static void cb_flash(struct fastboot *fb, const char *cmd)
 		goto out;
 	}
 
-	if (IS_ENABLED(CONFIG_BAREBOX_UPDATE) && filetype_is_barebox_image(filetype)) {
+	if (IS_ENABLED(CONFIG_BAREBOX_UPDATE) &&
+	    (filetype_is_barebox_image(filetype) || strstarts(fentry->name, "bbu-"))) {
 		void *buf;
 		struct bbu_handler *handler;
 		struct bbu_data data = {
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v3 2/2] fastboot: bail if update handler couldn't be found for bbu-partition

[Ahmad Fatoum]

Fastboot would fall back to a raw copy even for bbu- partitions if
no barebox_update handler was found. Prevent this by bailing out
with an error code.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v2 -> v3:
  - bail out instead of only printing message and falling back
    to raw copy (Sascha)
v1 -> v2:
  - print message in case barebox_update handler is not found
---
 common/fastboot.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index 330a06f5a32f..a5cf04b39ecd 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -683,8 +683,13 @@ static void cb_flash(struct fastboot *fb, const char *cmd)
 		};
 
 		handler = bbu_find_handler_by_device(data.devicefile);
-		if (!handler)
-			goto copy;
+		if (!handler) {
+			fastboot_tx_print(fb, FASTBOOT_MSG_FAIL,
+					  "No barebox update handler registered for %s",
+					  data.devicefile);
+			ret = -ENOENT;
+			goto out;
+		}
 
 		fastboot_tx_print(fb, FASTBOOT_MSG_INFO,
 				  "This is a barebox image...");
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH] fixup! fastboot: bail if update handler couldn't be found for bbu-partition

[Ahmad Fatoum]

No longer used.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/fastboot.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/common/fastboot.c b/common/fastboot.c
index a5cf04b39ecd..baa1851e7fb2 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -716,7 +716,6 @@ static void cb_flash(struct fastboot *fb, const char *cmd)
 		goto out;
 	}
 
-copy:
 	ret = copy_file(fb->tempname, filename, 1);
 	if (ret)
 		fastboot_tx_print(fb, FASTBOOT_MSG_FAIL,
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 2/2] fastboot: bail if update handler couldn't be found for bbu-partition

[Sascha Hauer]

On Thu, Jun 09, 2022 at 03:09:36PM +0200, Ahmad Fatoum wrote:
> Fastboot would fall back to a raw copy even for bbu- partitions if
> no barebox_update handler was found. Prevent this by bailing out
> with an error code.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> v2 -> v3:
>   - bail out instead of only printing message and falling back
>     to raw copy (Sascha)
> v1 -> v2:
>   - print message in case barebox_update handler is not found
> ---
>  common/fastboot.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/common/fastboot.c b/common/fastboot.c
> index 330a06f5a32f..a5cf04b39ecd 100644
> --- a/common/fastboot.c
> +++ b/common/fastboot.c
> @@ -683,8 +683,13 @@ static void cb_flash(struct fastboot *fb, const char *cmd)
>  		};
>  
>  		handler = bbu_find_handler_by_device(data.devicefile);
> -		if (!handler)
> -			goto copy;
> +		if (!handler) {
> +			fastboot_tx_print(fb, FASTBOOT_MSG_FAIL,
> +					  "No barebox update handler registered for %s",
> +					  data.devicefile);
> +			ret = -ENOENT;
> +			goto out;
> +		}

I didn't verify that, but I believe a full raw bootable disk image
generated for i.MX will be detected as barebox image. With this patch we
wouldn't be able to flash that anymore.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 2/2] fastboot: bail if update handler couldn't be found for bbu-partition

[Ahmad Fatoum]

On 09.06.22 16:10, Sascha Hauer wrote:
> On Thu, Jun 09, 2022 at 03:09:36PM +0200, Ahmad Fatoum wrote:
>> Fastboot would fall back to a raw copy even for bbu- partitions if
>> no barebox_update handler was found. Prevent this by bailing out
>> with an error code.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> v2 -> v3:
>>   - bail out instead of only printing message and falling back
>>     to raw copy (Sascha)
>> v1 -> v2:
>>   - print message in case barebox_update handler is not found
>> ---
>>  common/fastboot.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/fastboot.c b/common/fastboot.c
>> index 330a06f5a32f..a5cf04b39ecd 100644
>> --- a/common/fastboot.c
>> +++ b/common/fastboot.c
>> @@ -683,8 +683,13 @@ static void cb_flash(struct fastboot *fb, const char *cmd)
>>  		};
>>  
>>  		handler = bbu_find_handler_by_device(data.devicefile);
>> -		if (!handler)
>> -			goto copy;
>> +		if (!handler) {
>> +			fastboot_tx_print(fb, FASTBOOT_MSG_FAIL,
>> +					  "No barebox update handler registered for %s",
>> +					  data.devicefile);
>> +			ret = -ENOENT;
>> +			goto out;
>> +		}
> 
> I didn't verify that, but I believe a full raw bootable disk image
> generated for i.MX will be detected as barebox image. With this patch we
> wouldn't be able to flash that anymore.

Can we just apply v2 and see how often we see the message?

> 
> Sascha
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 2/2] fastboot: bail if update handler couldn't be found for bbu-partition

[Michael Olbrich]

On Thu, Jun 09, 2022 at 04:12:39PM +0200, Ahmad Fatoum wrote:
> On 09.06.22 16:10, Sascha Hauer wrote:
> > On Thu, Jun 09, 2022 at 03:09:36PM +0200, Ahmad Fatoum wrote:
> >> Fastboot would fall back to a raw copy even for bbu- partitions if
> >> no barebox_update handler was found. Prevent this by bailing out
> >> with an error code.
> >>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >> v2 -> v3:
> >>   - bail out instead of only printing message and falling back
> >>     to raw copy (Sascha)
> >> v1 -> v2:
> >>   - print message in case barebox_update handler is not found
> >> ---
> >>  common/fastboot.c | 9 +++++++--
> >>  1 file changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/common/fastboot.c b/common/fastboot.c
> >> index 330a06f5a32f..a5cf04b39ecd 100644
> >> --- a/common/fastboot.c
> >> +++ b/common/fastboot.c
> >> @@ -683,8 +683,13 @@ static void cb_flash(struct fastboot *fb, const char *cmd)
> >>  		};
> >>  
> >>  		handler = bbu_find_handler_by_device(data.devicefile);
> >> -		if (!handler)
> >> -			goto copy;
> >> +		if (!handler) {
> >> +			fastboot_tx_print(fb, FASTBOOT_MSG_FAIL,
> >> +					  "No barebox update handler registered for %s",
> >> +					  data.devicefile);
> >> +			ret = -ENOENT;
> >> +			goto out;
> >> +		}
> > 
> > I didn't verify that, but I believe a full raw bootable disk image
> > generated for i.MX will be detected as barebox image. With this patch we
> > wouldn't be able to flash that anymore.
> 
> Can we just apply v2 and see how often we see the message?

Or maybe the 'is a barebox image' detection is not strict enough? We know
the image size, right? Is there a way to determine the size of the actual
barebox image? If the two differ then it's not a barebox image after all.

Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 2/2] fastboot: bail if update handler couldn't be found for bbu-partition

[Sascha Hauer]

On Fri, Jun 10, 2022 at 08:36:35AM +0200, Michael Olbrich wrote:
> On Thu, Jun 09, 2022 at 04:12:39PM +0200, Ahmad Fatoum wrote:
> > On 09.06.22 16:10, Sascha Hauer wrote:
> > > On Thu, Jun 09, 2022 at 03:09:36PM +0200, Ahmad Fatoum wrote:
> > >> Fastboot would fall back to a raw copy even for bbu- partitions if
> > >> no barebox_update handler was found. Prevent this by bailing out
> > >> with an error code.
> > >>
> > >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > >> ---
> > >> v2 -> v3:
> > >>   - bail out instead of only printing message and falling back
> > >>     to raw copy (Sascha)
> > >> v1 -> v2:
> > >>   - print message in case barebox_update handler is not found
> > >> ---
> > >>  common/fastboot.c | 9 +++++++--
> > >>  1 file changed, 7 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/common/fastboot.c b/common/fastboot.c
> > >> index 330a06f5a32f..a5cf04b39ecd 100644
> > >> --- a/common/fastboot.c
> > >> +++ b/common/fastboot.c
> > >> @@ -683,8 +683,13 @@ static void cb_flash(struct fastboot *fb, const char *cmd)
> > >>  		};
> > >>  
> > >>  		handler = bbu_find_handler_by_device(data.devicefile);
> > >> -		if (!handler)
> > >> -			goto copy;
> > >> +		if (!handler) {
> > >> +			fastboot_tx_print(fb, FASTBOOT_MSG_FAIL,
> > >> +					  "No barebox update handler registered for %s",
> > >> +					  data.devicefile);
> > >> +			ret = -ENOENT;
> > >> +			goto out;
> > >> +		}
> > > 
> > > I didn't verify that, but I believe a full raw bootable disk image
> > > generated for i.MX will be detected as barebox image. With this patch we
> > > wouldn't be able to flash that anymore.
> > 
> > Can we just apply v2 and see how often we see the message?
> 
> Or maybe the 'is a barebox image' detection is not strict enough? We know
> the image size, right? Is there a way to determine the size of the actual
> barebox image? If the two differ then it's not a barebox image after all.

filetype_is_barebox_image() looks like this:

bool filetype_is_barebox_image(enum filetype ft)
{
        switch (ft) {
        case filetype_arm_barebox:
        case filetype_mips_barebox:
        case filetype_ch_image:
        case filetype_ch_image_be:
        case filetype_layerscape_image:
        case filetype_layerscape_qspi_image:
        case filetype_stm32_image_fsbl_v1:
        case filetype_fip:
                return true;
        default:
                return false;
        }
}

There's likely a way to determine the size for some of the images, but
probably not for all.

We could check the size of the image. If it's too big for a barebox
image then it is none. I don't have a very good feeling about such a
heuristic though.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 1/2] fastboot: always try barebox_update handler for bbu- partitions

[Sascha Hauer]

On Thu, Jun 09, 2022 at 03:09:35PM +0200, Ahmad Fatoum wrote:
> So far, fastboot barebox_update code was only entered when
> filetype_is_barebox_image() returned true, a function exclusively used
> for fastboot. Align this with normal barebox_update from the command
> line by leaving the decision on whether the image is correctly to the
> barebox_update handler if the user explicitly targets a bbu- fastboot
> partition.
> 
> We keep the old check OR-ed to maintain backwards-compatibility for
> invoking barebox_update for non bbu- fastboot partitions.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> v2 -> v3:
>   - no changes
> v1 -> v2:
>   - new patch (Sascha)
> ---
>  common/fastboot.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied (this patch), thanks

Sascha

> 
> diff --git a/common/fastboot.c b/common/fastboot.c
> index f8ed40c86e00..330a06f5a32f 100644
> --- a/common/fastboot.c
> +++ b/common/fastboot.c
> @@ -673,7 +673,8 @@ static void cb_flash(struct fastboot *fb, const char *cmd)
>  		goto out;
>  	}
>  
> -	if (IS_ENABLED(CONFIG_BAREBOX_UPDATE) && filetype_is_barebox_image(filetype)) {
> +	if (IS_ENABLED(CONFIG_BAREBOX_UPDATE) &&
> +	    (filetype_is_barebox_image(filetype) || strstarts(fentry->name, "bbu-"))) {
>  		void *buf;
>  		struct bbu_handler *handler;
>  		struct bbu_data data = {
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox