All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] distro_boot: Fix bootfile env after calling boot_extlinux
@ 2021-10-12  8:55 Artem Lapkin
  2021-10-12 19:44 ` Tom Rini
  0 siblings, 1 reply; 8+ messages in thread
From: Artem Lapkin @ 2021-10-12  8:55 UTC (permalink / raw)
  To: sjg
  Cc: trini, narmstrong, twarren, andre.przywara, u-boot,
	u-boot-amlogic, art, nick, gouwa

Problem

PXE cannot boot normally after Sysboot changed the bootfile env (called
from boot_extlinux) from the default "boot.scr.uimg" to
"/boot/extlinux/extlinux.conf".

In addition, an unbootable extlinux configuration will also make the PXE
boot unbootable, because it will use the incorrect path "/boot/extlinux/"
from the bootfile env.

Solution

Save and restore default bootfile env value when boot_extlinux is used.

Example
================================================================
Hit SPACE in 2 seconds to stop autoboot
... is now current device
Found /boot/extlinux/extlinux.conf
Retrieving file: /boot/extlinux/extlinux.conf
413 bytes read in 2 ms (201.2 KiB/s)
Skipping Krescue for failure retrieving kernel
SCRIPT FAILED: continuing...
...
Speed: 1000, full duplex
BOOTP broadcast 1
DHCP client bound to address 192.168.11.151 (8 ms)
Using ethernet@ff3f0000 device
TFTP from server 192.168.11.1; our IP address is 192.168.11.151
Filename '/boot/extlinux/pxelinux.cfg/default'.
Not retrying...
================================================================

Signed-off-by: Artem Lapkin <art@khadas.com>
---
 include/config_distro_bootcmd.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
index 3f724aa10f..db3d1b2362 100644
--- a/include/config_distro_bootcmd.h
+++ b/include/config_distro_bootcmd.h
@@ -445,7 +445,9 @@
 				"${devnum}:${distro_bootpart} "           \
 				"${prefix}${boot_syslinux_conf}; then "   \
 			"echo Found ${prefix}${boot_syslinux_conf}; "     \
+			"bootfile_bak=${bootfile}; "                      \
 			"run boot_extlinux; "                             \
+			"setenv bootfile ${bootfile_bak}; "               \
 			"echo SCRIPT FAILED: continuing...; "             \
 		"fi\0"                                                    \
 	\
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] distro_boot: Fix bootfile env after calling boot_extlinux
  2021-10-12  8:55 [PATCH] distro_boot: Fix bootfile env after calling boot_extlinux Artem Lapkin
@ 2021-10-12 19:44 ` Tom Rini
  2021-10-12 20:31   ` Simon Glass
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Rini @ 2021-10-12 19:44 UTC (permalink / raw)
  To: Artem Lapkin
  Cc: sjg, narmstrong, twarren, andre.przywara, u-boot, u-boot-amlogic,
	art, nick, gouwa

[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]

On Tue, Oct 12, 2021 at 04:55:44PM +0800, Artem Lapkin wrote:

> Problem
> 
> PXE cannot boot normally after Sysboot changed the bootfile env (called
> from boot_extlinux) from the default "boot.scr.uimg" to
> "/boot/extlinux/extlinux.conf".
> 
> In addition, an unbootable extlinux configuration will also make the PXE
> boot unbootable, because it will use the incorrect path "/boot/extlinux/"
> from the bootfile env.
> 
> Solution
> 
> Save and restore default bootfile env value when boot_extlinux is used.
> 
> Example
> ================================================================
> Hit SPACE in 2 seconds to stop autoboot
> ... is now current device
> Found /boot/extlinux/extlinux.conf
> Retrieving file: /boot/extlinux/extlinux.conf
> 413 bytes read in 2 ms (201.2 KiB/s)
> Skipping Krescue for failure retrieving kernel
> SCRIPT FAILED: continuing...
> ...
> Speed: 1000, full duplex
> BOOTP broadcast 1
> DHCP client bound to address 192.168.11.151 (8 ms)
> Using ethernet@ff3f0000 device
> TFTP from server 192.168.11.1; our IP address is 192.168.11.151
> Filename '/boot/extlinux/pxelinux.cfg/default'.
> Not retrying...
> ================================================================
> 
> Signed-off-by: Artem Lapkin <art@khadas.com>
> ---
>  include/config_distro_bootcmd.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
> index 3f724aa10f..db3d1b2362 100644
> --- a/include/config_distro_bootcmd.h
> +++ b/include/config_distro_bootcmd.h
> @@ -445,7 +445,9 @@
>  				"${devnum}:${distro_bootpart} "           \
>  				"${prefix}${boot_syslinux_conf}; then "   \
>  			"echo Found ${prefix}${boot_syslinux_conf}; "     \
> +			"bootfile_bak=${bootfile}; "                      \
>  			"run boot_extlinux; "                             \
> +			"setenv bootfile ${bootfile_bak}; "               \
>  			"echo SCRIPT FAILED: continuing...; "             \
>  		"fi\0"                                                    \
>  	\

We've had this kind of problem before, and the answer is that variables
should be local, not global in scope.  In this case, I see that the way
the pxe/sysboot code works is that we have to env_set("..") in one place
to env_get("..") in another, so I don't see a way around this.

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] distro_boot: Fix bootfile env after calling boot_extlinux
  2021-10-12 19:44 ` Tom Rini
@ 2021-10-12 20:31   ` Simon Glass
  2021-10-12 21:07     ` Tom Rini
  0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2021-10-12 20:31 UTC (permalink / raw)
  To: Tom Rini
  Cc: Artem Lapkin, Neil Armstrong, Tom Warren, Andre Przywara,
	U-Boot Mailing List, u-boot-amlogic, art, nick, gouwa

Hi Tom,

On Tue, 12 Oct 2021 at 13:44, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Oct 12, 2021 at 04:55:44PM +0800, Artem Lapkin wrote:
>
> > Problem
> >
> > PXE cannot boot normally after Sysboot changed the bootfile env (called
> > from boot_extlinux) from the default "boot.scr.uimg" to
> > "/boot/extlinux/extlinux.conf".
> >
> > In addition, an unbootable extlinux configuration will also make the PXE
> > boot unbootable, because it will use the incorrect path "/boot/extlinux/"
> > from the bootfile env.
> >
> > Solution
> >
> > Save and restore default bootfile env value when boot_extlinux is used.
> >
> > Example
> > ================================================================
> > Hit SPACE in 2 seconds to stop autoboot
> > ... is now current device
> > Found /boot/extlinux/extlinux.conf
> > Retrieving file: /boot/extlinux/extlinux.conf
> > 413 bytes read in 2 ms (201.2 KiB/s)
> > Skipping Krescue for failure retrieving kernel
> > SCRIPT FAILED: continuing...
> > ...
> > Speed: 1000, full duplex
> > BOOTP broadcast 1
> > DHCP client bound to address 192.168.11.151 (8 ms)
> > Using ethernet@ff3f0000 device
> > TFTP from server 192.168.11.1; our IP address is 192.168.11.151
> > Filename '/boot/extlinux/pxelinux.cfg/default'.
> > Not retrying...
> > ================================================================
> >
> > Signed-off-by: Artem Lapkin <art@khadas.com>
> > ---
> >  include/config_distro_bootcmd.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
> > index 3f724aa10f..db3d1b2362 100644
> > --- a/include/config_distro_bootcmd.h
> > +++ b/include/config_distro_bootcmd.h
> > @@ -445,7 +445,9 @@
> >                               "${devnum}:${distro_bootpart} "           \
> >                               "${prefix}${boot_syslinux_conf}; then "   \
> >                       "echo Found ${prefix}${boot_syslinux_conf}; "     \
> > +                     "bootfile_bak=${bootfile}; "                      \
> >                       "run boot_extlinux; "                             \
> > +                     "setenv bootfile ${bootfile_bak}; "               \
> >                       "echo SCRIPT FAILED: continuing...; "             \
> >               "fi\0"                                                    \
> >       \
>
> We've had this kind of problem before, and the answer is that variables
> should be local, not global in scope.  In this case, I see that the way
> the pxe/sysboot code works is that we have to env_set("..") in one place
> to env_get("..") in another, so I don't see a way around this.
>
> Reviewed-by: Tom Rini <trini@konsulko.com>

IMO a better approach will be the bootflow implementation. I hope to
get v2 out early next week.

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] distro_boot: Fix bootfile env after calling boot_extlinux
  2021-10-12 20:31   ` Simon Glass
@ 2021-10-12 21:07     ` Tom Rini
  2021-10-13  3:44       ` Art Nikpal
  2021-10-13 16:58       ` Simon Glass
  0 siblings, 2 replies; 8+ messages in thread
From: Tom Rini @ 2021-10-12 21:07 UTC (permalink / raw)
  To: Simon Glass
  Cc: Artem Lapkin, Neil Armstrong, Tom Warren, Andre Przywara,
	U-Boot Mailing List, u-boot-amlogic, art, nick, gouwa

[-- Attachment #1: Type: text/plain, Size: 3422 bytes --]

On Tue, Oct 12, 2021 at 02:31:18PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 12 Oct 2021 at 13:44, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Oct 12, 2021 at 04:55:44PM +0800, Artem Lapkin wrote:
> >
> > > Problem
> > >
> > > PXE cannot boot normally after Sysboot changed the bootfile env (called
> > > from boot_extlinux) from the default "boot.scr.uimg" to
> > > "/boot/extlinux/extlinux.conf".
> > >
> > > In addition, an unbootable extlinux configuration will also make the PXE
> > > boot unbootable, because it will use the incorrect path "/boot/extlinux/"
> > > from the bootfile env.
> > >
> > > Solution
> > >
> > > Save and restore default bootfile env value when boot_extlinux is used.
> > >
> > > Example
> > > ================================================================
> > > Hit SPACE in 2 seconds to stop autoboot
> > > ... is now current device
> > > Found /boot/extlinux/extlinux.conf
> > > Retrieving file: /boot/extlinux/extlinux.conf
> > > 413 bytes read in 2 ms (201.2 KiB/s)
> > > Skipping Krescue for failure retrieving kernel
> > > SCRIPT FAILED: continuing...
> > > ...
> > > Speed: 1000, full duplex
> > > BOOTP broadcast 1
> > > DHCP client bound to address 192.168.11.151 (8 ms)
> > > Using ethernet@ff3f0000 device
> > > TFTP from server 192.168.11.1; our IP address is 192.168.11.151
> > > Filename '/boot/extlinux/pxelinux.cfg/default'.
> > > Not retrying...
> > > ================================================================
> > >
> > > Signed-off-by: Artem Lapkin <art@khadas.com>
> > > ---
> > >  include/config_distro_bootcmd.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
> > > index 3f724aa10f..db3d1b2362 100644
> > > --- a/include/config_distro_bootcmd.h
> > > +++ b/include/config_distro_bootcmd.h
> > > @@ -445,7 +445,9 @@
> > >                               "${devnum}:${distro_bootpart} "           \
> > >                               "${prefix}${boot_syslinux_conf}; then "   \
> > >                       "echo Found ${prefix}${boot_syslinux_conf}; "     \
> > > +                     "bootfile_bak=${bootfile}; "                      \
> > >                       "run boot_extlinux; "                             \
> > > +                     "setenv bootfile ${bootfile_bak}; "               \
> > >                       "echo SCRIPT FAILED: continuing...; "             \
> > >               "fi\0"                                                    \
> > >       \
> >
> > We've had this kind of problem before, and the answer is that variables
> > should be local, not global in scope.  In this case, I see that the way
> > the pxe/sysboot code works is that we have to env_set("..") in one place
> > to env_get("..") in another, so I don't see a way around this.
> >
> > Reviewed-by: Tom Rini <trini@konsulko.com>
> 
> IMO a better approach will be the bootflow implementation. I hope to
> get v2 out early next week.

I'm not sure if the bootflow way of going here would or would not have
the same problem, or perhaps a slightly different problem.  At heart
here, "sysboot" calls env_set(...) and then calls in to the pxe code
which does env_get(...).  So now I wonder how this would be fixed in
bootflow, since we aren't dealing with the environment directly.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] distro_boot: Fix bootfile env after calling boot_extlinux
  2021-10-12 21:07     ` Tom Rini
@ 2021-10-13  3:44       ` Art Nikpal
  2021-10-13 16:58         ` Simon Glass
  2021-10-13 16:58       ` Simon Glass
  1 sibling, 1 reply; 8+ messages in thread
From: Art Nikpal @ 2021-10-13  3:44 UTC (permalink / raw)
  To: Tom Rini
  Cc: Simon Glass, Neil Armstrong, Tom Warren, Andre Przywara,
	U-Boot Mailing List, u-boot-amlogic, Artem Lapkin, Nick Xie,
	Gouwa Wang

Yes changes inside include/config_distro_bootcmd.h not the best solution
for this issue.
I think it is better to change sysboot cmd and i have prepared another
solution already!
https://patchwork.ozlabs.org/project/uboot/patch/20211013033912.3297227-1-art@khadas.com/
how about this ?

On Wed, Oct 13, 2021 at 5:07 AM Tom Rini <trini@konsulko.com> wrote:

> On Tue, Oct 12, 2021 at 02:31:18PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 12 Oct 2021 at 13:44, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Tue, Oct 12, 2021 at 04:55:44PM +0800, Artem Lapkin wrote:
> > >
> > > > Problem
> > > >
> > > > PXE cannot boot normally after Sysboot changed the bootfile env
> (called
> > > > from boot_extlinux) from the default "boot.scr.uimg" to
> > > > "/boot/extlinux/extlinux.conf".
> > > >
> > > > In addition, an unbootable extlinux configuration will also make the
> PXE
> > > > boot unbootable, because it will use the incorrect path
> "/boot/extlinux/"
> > > > from the bootfile env.
> > > >
> > > > Solution
> > > >
> > > > Save and restore default bootfile env value when boot_extlinux is
> used.
> > > >
> > > > Example
> > > > ================================================================
> > > > Hit SPACE in 2 seconds to stop autoboot
> > > > ... is now current device
> > > > Found /boot/extlinux/extlinux.conf
> > > > Retrieving file: /boot/extlinux/extlinux.conf
> > > > 413 bytes read in 2 ms (201.2 KiB/s)
> > > > Skipping Krescue for failure retrieving kernel
> > > > SCRIPT FAILED: continuing...
> > > > ...
> > > > Speed: 1000, full duplex
> > > > BOOTP broadcast 1
> > > > DHCP client bound to address 192.168.11.151 (8 ms)
> > > > Using ethernet@ff3f0000 device
> > > > TFTP from server 192.168.11.1; our IP address is 192.168.11.151
> > > > Filename '/boot/extlinux/pxelinux.cfg/default'.
> > > > Not retrying...
> > > > ================================================================
> > > >
> > > > Signed-off-by: Artem Lapkin <art@khadas.com>
> > > > ---
> > > >  include/config_distro_bootcmd.h | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/include/config_distro_bootcmd.h
> b/include/config_distro_bootcmd.h
> > > > index 3f724aa10f..db3d1b2362 100644
> > > > --- a/include/config_distro_bootcmd.h
> > > > +++ b/include/config_distro_bootcmd.h
> > > > @@ -445,7 +445,9 @@
> > > >                               "${devnum}:${distro_bootpart} "
>    \
> > > >                               "${prefix}${boot_syslinux_conf}; then
> "   \
> > > >                       "echo Found ${prefix}${boot_syslinux_conf}; "
>    \
> > > > +                     "bootfile_bak=${bootfile}; "
>     \
> > > >                       "run boot_extlinux; "
>    \
> > > > +                     "setenv bootfile ${bootfile_bak}; "
>    \
> > > >                       "echo SCRIPT FAILED: continuing...; "
>    \
> > > >               "fi\0"
>     \
> > > >       \
> > >
> > > We've had this kind of problem before, and the answer is that variables
> > > should be local, not global in scope.  In this case, I see that the way
> > > the pxe/sysboot code works is that we have to env_set("..") in one
> place
> > > to env_get("..") in another, so I don't see a way around this.
> > >
> > > Reviewed-by: Tom Rini <trini@konsulko.com>
> >
> > IMO a better approach will be the bootflow implementation. I hope to
> > get v2 out early next week.
>
> I'm not sure if the bootflow way of going here would or would not have
> the same problem, or perhaps a slightly different problem.  At heart
> here, "sysboot" calls env_set(...) and then calls in to the pxe code
> which does env_get(...).  So now I wonder how this would be fixed in
> bootflow, since we aren't dealing with the environment directly.
>
> --
> Tom
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] distro_boot: Fix bootfile env after calling boot_extlinux
  2021-10-13  3:44       ` Art Nikpal
@ 2021-10-13 16:58         ` Simon Glass
  2021-10-14  4:32           ` Art Nikpal
  0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2021-10-13 16:58 UTC (permalink / raw)
  To: Art Nikpal
  Cc: Tom Rini, Neil Armstrong, Tom Warren, Andre Przywara,
	U-Boot Mailing List, u-boot-amlogic, Artem Lapkin, Nick Xie,
	Gouwa Wang

Hi Art,

On Tue, 12 Oct 2021 at 21:45, Art Nikpal <email2tema@gmail.com> wrote:
>
> Yes changes inside include/config_distro_bootcmd.h not the best solution for this issue.
> I think it is better to change sysboot cmd and i have prepared another solution already!
> https://patchwork.ozlabs.org/project/uboot/patch/20211013033912.3297227-1-art@khadas.com/
> how about this ?

Seem reasonable to me, but could you check out my syslinux refactor
series? It could use some review.

http://patchwork.ozlabs.org/project/uboot/list/?series=264265


- Simon

>
> On Wed, Oct 13, 2021 at 5:07 AM Tom Rini <trini@konsulko.com> wrote:
>>
>> On Tue, Oct 12, 2021 at 02:31:18PM -0600, Simon Glass wrote:
>> > Hi Tom,
>> >
>> > On Tue, 12 Oct 2021 at 13:44, Tom Rini <trini@konsulko.com> wrote:
>> > >
>> > > On Tue, Oct 12, 2021 at 04:55:44PM +0800, Artem Lapkin wrote:
>> > >
>> > > > Problem
>> > > >
>> > > > PXE cannot boot normally after Sysboot changed the bootfile env (called
>> > > > from boot_extlinux) from the default "boot.scr.uimg" to
>> > > > "/boot/extlinux/extlinux.conf".
>> > > >
>> > > > In addition, an unbootable extlinux configuration will also make the PXE
>> > > > boot unbootable, because it will use the incorrect path "/boot/extlinux/"
>> > > > from the bootfile env.
>> > > >
>> > > > Solution
>> > > >
>> > > > Save and restore default bootfile env value when boot_extlinux is used.
>> > > >
>> > > > Example
>> > > > ================================================================
>> > > > Hit SPACE in 2 seconds to stop autoboot
>> > > > ... is now current device
>> > > > Found /boot/extlinux/extlinux.conf
>> > > > Retrieving file: /boot/extlinux/extlinux.conf
>> > > > 413 bytes read in 2 ms (201.2 KiB/s)
>> > > > Skipping Krescue for failure retrieving kernel
>> > > > SCRIPT FAILED: continuing...
>> > > > ...
>> > > > Speed: 1000, full duplex
>> > > > BOOTP broadcast 1
>> > > > DHCP client bound to address 192.168.11.151 (8 ms)
>> > > > Using ethernet@ff3f0000 device
>> > > > TFTP from server 192.168.11.1; our IP address is 192.168.11.151
>> > > > Filename '/boot/extlinux/pxelinux.cfg/default'.
>> > > > Not retrying...
>> > > > ================================================================
>> > > >
>> > > > Signed-off-by: Artem Lapkin <art@khadas.com>
>> > > > ---
>> > > >  include/config_distro_bootcmd.h | 2 ++
>> > > >  1 file changed, 2 insertions(+)
>> > > >
>> > > > diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
>> > > > index 3f724aa10f..db3d1b2362 100644
>> > > > --- a/include/config_distro_bootcmd.h
>> > > > +++ b/include/config_distro_bootcmd.h
>> > > > @@ -445,7 +445,9 @@
>> > > >                               "${devnum}:${distro_bootpart} "           \
>> > > >                               "${prefix}${boot_syslinux_conf}; then "   \
>> > > >                       "echo Found ${prefix}${boot_syslinux_conf}; "     \
>> > > > +                     "bootfile_bak=${bootfile}; "                      \
>> > > >                       "run boot_extlinux; "                             \
>> > > > +                     "setenv bootfile ${bootfile_bak}; "               \
>> > > >                       "echo SCRIPT FAILED: continuing...; "             \
>> > > >               "fi\0"                                                    \
>> > > >       \
>> > >
>> > > We've had this kind of problem before, and the answer is that variables
>> > > should be local, not global in scope.  In this case, I see that the way
>> > > the pxe/sysboot code works is that we have to env_set("..") in one place
>> > > to env_get("..") in another, so I don't see a way around this.
>> > >
>> > > Reviewed-by: Tom Rini <trini@konsulko.com>
>> >
>> > IMO a better approach will be the bootflow implementation. I hope to
>> > get v2 out early next week.
>>
>> I'm not sure if the bootflow way of going here would or would not have
>> the same problem, or perhaps a slightly different problem.  At heart
>> here, "sysboot" calls env_set(...) and then calls in to the pxe code
>> which does env_get(...).  So now I wonder how this would be fixed in
>> bootflow, since we aren't dealing with the environment directly.
>>
>> --
>> Tom

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] distro_boot: Fix bootfile env after calling boot_extlinux
  2021-10-12 21:07     ` Tom Rini
  2021-10-13  3:44       ` Art Nikpal
@ 2021-10-13 16:58       ` Simon Glass
  1 sibling, 0 replies; 8+ messages in thread
From: Simon Glass @ 2021-10-13 16:58 UTC (permalink / raw)
  To: Tom Rini
  Cc: Artem Lapkin, Neil Armstrong, Tom Warren, Andre Przywara,
	U-Boot Mailing List, u-boot-amlogic, art, nick, gouwa

Hi Tom,

On Tue, 12 Oct 2021 at 15:07, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Oct 12, 2021 at 02:31:18PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 12 Oct 2021 at 13:44, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Tue, Oct 12, 2021 at 04:55:44PM +0800, Artem Lapkin wrote:
> > >
> > > > Problem
> > > >
> > > > PXE cannot boot normally after Sysboot changed the bootfile env (called
> > > > from boot_extlinux) from the default "boot.scr.uimg" to
> > > > "/boot/extlinux/extlinux.conf".
> > > >
> > > > In addition, an unbootable extlinux configuration will also make the PXE
> > > > boot unbootable, because it will use the incorrect path "/boot/extlinux/"
> > > > from the bootfile env.
> > > >
> > > > Solution
> > > >
> > > > Save and restore default bootfile env value when boot_extlinux is used.
> > > >
> > > > Example
> > > > ================================================================
> > > > Hit SPACE in 2 seconds to stop autoboot
> > > > ... is now current device
> > > > Found /boot/extlinux/extlinux.conf
> > > > Retrieving file: /boot/extlinux/extlinux.conf
> > > > 413 bytes read in 2 ms (201.2 KiB/s)
> > > > Skipping Krescue for failure retrieving kernel
> > > > SCRIPT FAILED: continuing...
> > > > ...
> > > > Speed: 1000, full duplex
> > > > BOOTP broadcast 1
> > > > DHCP client bound to address 192.168.11.151 (8 ms)
> > > > Using ethernet@ff3f0000 device
> > > > TFTP from server 192.168.11.1; our IP address is 192.168.11.151
> > > > Filename '/boot/extlinux/pxelinux.cfg/default'.
> > > > Not retrying...
> > > > ================================================================
> > > >
> > > > Signed-off-by: Artem Lapkin <art@khadas.com>
> > > > ---
> > > >  include/config_distro_bootcmd.h | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
> > > > index 3f724aa10f..db3d1b2362 100644
> > > > --- a/include/config_distro_bootcmd.h
> > > > +++ b/include/config_distro_bootcmd.h
> > > > @@ -445,7 +445,9 @@
> > > >                               "${devnum}:${distro_bootpart} "           \
> > > >                               "${prefix}${boot_syslinux_conf}; then "   \
> > > >                       "echo Found ${prefix}${boot_syslinux_conf}; "     \
> > > > +                     "bootfile_bak=${bootfile}; "                      \
> > > >                       "run boot_extlinux; "                             \
> > > > +                     "setenv bootfile ${bootfile_bak}; "               \
> > > >                       "echo SCRIPT FAILED: continuing...; "             \
> > > >               "fi\0"                                                    \
> > > >       \
> > >
> > > We've had this kind of problem before, and the answer is that variables
> > > should be local, not global in scope.  In this case, I see that the way
> > > the pxe/sysboot code works is that we have to env_set("..") in one place
> > > to env_get("..") in another, so I don't see a way around this.
> > >
> > > Reviewed-by: Tom Rini <trini@konsulko.com>
> >
> > IMO a better approach will be the bootflow implementation. I hope to
> > get v2 out early next week.
>
> I'm not sure if the bootflow way of going here would or would not have
> the same problem, or perhaps a slightly different problem.  At heart
> here, "sysboot" calls env_set(...) and then calls in to the pxe code
> which does env_get(...).  So now I wonder how this would be fixed in
> bootflow, since we aren't dealing with the environment directly.

Here's the patch where (I believe) this problem was addressed:

http://patchwork.ozlabs.org/project/uboot/patch/20210927092350.v2.11.I6a01e73ef114448e39cb6899c21f6c169e4da216@changeid/

Regards,
Simon

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] distro_boot: Fix bootfile env after calling boot_extlinux
  2021-10-13 16:58         ` Simon Glass
@ 2021-10-14  4:32           ` Art Nikpal
  0 siblings, 0 replies; 8+ messages in thread
From: Art Nikpal @ 2021-10-14  4:32 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, Neil Armstrong, Tom Warren, Andre Przywara,
	U-Boot Mailing List, u-boot-amlogic, Artem Lapkin, Nick Xie,
	Gouwa Wang

> could you check out my syslinux refactor
> series? It could use some review.
>
> http://patchwork.ozlabs.org/project/uboot/list/?series=264265

Yes ! But I already can't apply this series for actual uboot code
(need to change)
maybe u can make v3 patches for actual uboot state.

I checked it quickly. This series has many useful improvements!

> Here's the patch where (I believe) this problem was addressed:
> http://patchwork.ozlabs.org/project/uboot/patch/20210927092350.v2.11.I6a01e73ef114448e39cb6899c21f6c169e4da216@changeid/

look like yes and  if your series will be applied my patch already no need.

On Thu, Oct 14, 2021 at 12:58 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Art,
>
> On Tue, 12 Oct 2021 at 21:45, Art Nikpal <email2tema@gmail.com> wrote:
> >
> > Yes changes inside include/config_distro_bootcmd.h not the best solution for this issue.
> > I think it is better to change sysboot cmd and i have prepared another solution already!
> > https://patchwork.ozlabs.org/project/uboot/patch/20211013033912.3297227-1-art@khadas.com/
> > how about this ?
>
> Seem reasonable to me, but could you check out my syslinux refactor
> series? It could use some review.
>
> http://patchwork.ozlabs.org/project/uboot/list/?series=264265
>
>
> - Simon
>
> >
> > On Wed, Oct 13, 2021 at 5:07 AM Tom Rini <trini@konsulko.com> wrote:
> >>
> >> On Tue, Oct 12, 2021 at 02:31:18PM -0600, Simon Glass wrote:
> >> > Hi Tom,
> >> >
> >> > On Tue, 12 Oct 2021 at 13:44, Tom Rini <trini@konsulko.com> wrote:
> >> > >
> >> > > On Tue, Oct 12, 2021 at 04:55:44PM +0800, Artem Lapkin wrote:
> >> > >
> >> > > > Problem
> >> > > >
> >> > > > PXE cannot boot normally after Sysboot changed the bootfile env (called
> >> > > > from boot_extlinux) from the default "boot.scr.uimg" to
> >> > > > "/boot/extlinux/extlinux.conf".
> >> > > >
> >> > > > In addition, an unbootable extlinux configuration will also make the PXE
> >> > > > boot unbootable, because it will use the incorrect path "/boot/extlinux/"
> >> > > > from the bootfile env.
> >> > > >
> >> > > > Solution
> >> > > >
> >> > > > Save and restore default bootfile env value when boot_extlinux is used.
> >> > > >
> >> > > > Example
> >> > > > ================================================================
> >> > > > Hit SPACE in 2 seconds to stop autoboot
> >> > > > ... is now current device
> >> > > > Found /boot/extlinux/extlinux.conf
> >> > > > Retrieving file: /boot/extlinux/extlinux.conf
> >> > > > 413 bytes read in 2 ms (201.2 KiB/s)
> >> > > > Skipping Krescue for failure retrieving kernel
> >> > > > SCRIPT FAILED: continuing...
> >> > > > ...
> >> > > > Speed: 1000, full duplex
> >> > > > BOOTP broadcast 1
> >> > > > DHCP client bound to address 192.168.11.151 (8 ms)
> >> > > > Using ethernet@ff3f0000 device
> >> > > > TFTP from server 192.168.11.1; our IP address is 192.168.11.151
> >> > > > Filename '/boot/extlinux/pxelinux.cfg/default'.
> >> > > > Not retrying...
> >> > > > ================================================================
> >> > > >
> >> > > > Signed-off-by: Artem Lapkin <art@khadas.com>
> >> > > > ---
> >> > > >  include/config_distro_bootcmd.h | 2 ++
> >> > > >  1 file changed, 2 insertions(+)
> >> > > >
> >> > > > diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
> >> > > > index 3f724aa10f..db3d1b2362 100644
> >> > > > --- a/include/config_distro_bootcmd.h
> >> > > > +++ b/include/config_distro_bootcmd.h
> >> > > > @@ -445,7 +445,9 @@
> >> > > >                               "${devnum}:${distro_bootpart} "           \
> >> > > >                               "${prefix}${boot_syslinux_conf}; then "   \
> >> > > >                       "echo Found ${prefix}${boot_syslinux_conf}; "     \
> >> > > > +                     "bootfile_bak=${bootfile}; "                      \
> >> > > >                       "run boot_extlinux; "                             \
> >> > > > +                     "setenv bootfile ${bootfile_bak}; "               \
> >> > > >                       "echo SCRIPT FAILED: continuing...; "             \
> >> > > >               "fi\0"                                                    \
> >> > > >       \
> >> > >
> >> > > We've had this kind of problem before, and the answer is that variables
> >> > > should be local, not global in scope.  In this case, I see that the way
> >> > > the pxe/sysboot code works is that we have to env_set("..") in one place
> >> > > to env_get("..") in another, so I don't see a way around this.
> >> > >
> >> > > Reviewed-by: Tom Rini <trini@konsulko.com>
> >> >
> >> > IMO a better approach will be the bootflow implementation. I hope to
> >> > get v2 out early next week.
> >>
> >> I'm not sure if the bootflow way of going here would or would not have
> >> the same problem, or perhaps a slightly different problem.  At heart
> >> here, "sysboot" calls env_set(...) and then calls in to the pxe code
> >> which does env_get(...).  So now I wonder how this would be fixed in
> >> bootflow, since we aren't dealing with the environment directly.
> >>
> >> --
> >> Tom

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-10-14  4:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-12  8:55 [PATCH] distro_boot: Fix bootfile env after calling boot_extlinux Artem Lapkin
2021-10-12 19:44 ` Tom Rini
2021-10-12 20:31   ` Simon Glass
2021-10-12 21:07     ` Tom Rini
2021-10-13  3:44       ` Art Nikpal
2021-10-13 16:58         ` Simon Glass
2021-10-14  4:32           ` Art Nikpal
2021-10-13 16:58       ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.