All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH RESEND] imx/genimage: Place the rootfs at a proper offset when BR2_LINUX_KERNEL_INSTALL_TARGET=y
@ 2021-11-13 14:38 Fabio Estevam
  2021-11-13 17:06 ` Thomas Petazzoni
  0 siblings, 1 reply; 5+ messages in thread
From: Fabio Estevam @ 2021-11-13 14:38 UTC (permalink / raw)
  To: buildroot; +Cc: Fabio Estevam

Currently, when BR2_LINUX_KERNEL_INSTALL_TARGET=y is selected, issuing
a "saveenv" command in the U-Boot prompt may lead to rootfs corruption.

Avoid this problem by placing the rootfs at an 8MB offset, just like
it is done in genimage.cfg.template.

Tested on a imx6qp-wandboard and also on a custom imx6ull based board.

"saveenv" does not corrupt the rootfs anymore after this change.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 board/freescale/common/imx/genimage.cfg.template_no_boot_part    | 1 +
 .../freescale/common/imx/genimage.cfg.template_no_boot_part_spl  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/board/freescale/common/imx/genimage.cfg.template_no_boot_part b/board/freescale/common/imx/genimage.cfg.template_no_boot_part
index edc34d0fcd12..b8fa43210b91 100644
--- a/board/freescale/common/imx/genimage.cfg.template_no_boot_part
+++ b/board/freescale/common/imx/genimage.cfg.template_no_boot_part
@@ -11,5 +11,6 @@ image sdcard.img {
   partition rootfs {
     partition-type = 0x83
     image = "rootfs.ext2"
+    offset = 8M
   }
 }
diff --git a/board/freescale/common/imx/genimage.cfg.template_no_boot_part_spl b/board/freescale/common/imx/genimage.cfg.template_no_boot_part_spl
index c29032572aef..ef015918a5b2 100644
--- a/board/freescale/common/imx/genimage.cfg.template_no_boot_part_spl
+++ b/board/freescale/common/imx/genimage.cfg.template_no_boot_part_spl
@@ -26,5 +26,6 @@ image sdcard.img {
   partition rootfs {
     partition-type = 0x83
     image = "rootfs.ext2"
+    offset = 8M
   }
 }
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

* Re: [Buildroot] [PATCH RESEND] imx/genimage: Place the rootfs at a proper offset when BR2_LINUX_KERNEL_INSTALL_TARGET=y
  2021-11-13 14:38 [Buildroot] [PATCH RESEND] imx/genimage: Place the rootfs at a proper offset when BR2_LINUX_KERNEL_INSTALL_TARGET=y Fabio Estevam
@ 2021-11-13 17:06 ` Thomas Petazzoni
  2021-11-13 18:11   ` Fabio Estevam
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Petazzoni @ 2021-11-13 17:06 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: buildroot

On Sat, 13 Nov 2021 11:38:57 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> Currently, when BR2_LINUX_KERNEL_INSTALL_TARGET=y is selected, issuing
> a "saveenv" command in the U-Boot prompt may lead to rootfs corruption.

I don't see the relationship between BR2_LINUX_KERNEL_INSTALL_TARGET=y
and changing the offset of the rootfs to avoid rootfs corruption.

All what BR2_LINUX_KERNEL_INSTALL_TARGET=y does it add the kernel image
into the /boot directory of the rootfs. It makes the rootfs larger for
sure, but it would still start at the same place.

Are you sure there's any relationship with
BR2_LINUX_KERNEL_INSTALL_TARGET=y here?

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

* Re: [Buildroot] [PATCH RESEND] imx/genimage: Place the rootfs at a proper offset when BR2_LINUX_KERNEL_INSTALL_TARGET=y
  2021-11-13 17:06 ` Thomas Petazzoni
@ 2021-11-13 18:11   ` Fabio Estevam
  2021-11-13 21:02     ` Yann E. MORIN
  0 siblings, 1 reply; 5+ messages in thread
From: Fabio Estevam @ 2021-11-13 18:11 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: buildroot

Hi Thomas,

On Sat, Nov 13, 2021 at 2:06 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:

> I don't see the relationship between BR2_LINUX_KERNEL_INSTALL_TARGET=y
> and changing the offset of the rootfs to avoid rootfs corruption.
>
> All what BR2_LINUX_KERNEL_INSTALL_TARGET=y does it add the kernel image
> into the /boot directory of the rootfs. It makes the rootfs larger for
> sure, but it would still start at the same place.
>
> Are you sure there's any relationship with
> BR2_LINUX_KERNEL_INSTALL_TARGET=y here?

Yes, this problem only happens with BR2_LINUX_KERNEL_INSTALL_TARGET=y.

When BR2_LINUX_KERNEL_INSTALL_TARGET is not selected, then
board/freescale/common/imx/genimage.cfg.template is used as per the logic
inside board/freescale/common/imx/post-image.sh.

board/freescale/common/imx/genimage.cfg.template correctly puts the
rootfs at a safe offset.

With BR2_LINUX_KERNEL_INSTALL_TARGET=y, then
board/freescale/common/imx/genimage.cfg.template_no_boot_part is used and no
offset to the rootfs is given, which may cause U-Boot environment
area to write into the rootfs
area, causing the rootfs corruption.

Please let me know if you need more information as this problem is
easy to reproduce.

Regards,

Fabio Estevam
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

* Re: [Buildroot] [PATCH RESEND] imx/genimage: Place the rootfs at a proper offset when BR2_LINUX_KERNEL_INSTALL_TARGET=y
  2021-11-13 18:11   ` Fabio Estevam
@ 2021-11-13 21:02     ` Yann E. MORIN
  2021-11-13 21:09       ` Yann E. MORIN
  0 siblings, 1 reply; 5+ messages in thread
From: Yann E. MORIN @ 2021-11-13 21:02 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: Thomas Petazzoni, buildroot

Fabio, All,

On 2021-11-13 15:11 -0300, Fabio Estevam spake thusly:
> Hi Thomas,
> 
> On Sat, Nov 13, 2021 at 2:06 PM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
> 
> > I don't see the relationship between BR2_LINUX_KERNEL_INSTALL_TARGET=y
> > and changing the offset of the rootfs to avoid rootfs corruption.
> >
> > All what BR2_LINUX_KERNEL_INSTALL_TARGET=y does it add the kernel image
> > into the /boot directory of the rootfs. It makes the rootfs larger for
> > sure, but it would still start at the same place.
> >
> > Are you sure there's any relationship with
> > BR2_LINUX_KERNEL_INSTALL_TARGET=y here?
> 
> Yes, this problem only happens with BR2_LINUX_KERNEL_INSTALL_TARGET=y.
> 
> When BR2_LINUX_KERNEL_INSTALL_TARGET is not selected, then
> board/freescale/common/imx/genimage.cfg.template is used as per the logic
> inside board/freescale/common/imx/post-image.sh.
> 
> board/freescale/common/imx/genimage.cfg.template correctly puts the
> rootfs at a safe offset.
> 
> With BR2_LINUX_KERNEL_INSTALL_TARGET=y, then
> board/freescale/common/imx/genimage.cfg.template_no_boot_part is used and no
> offset to the rootfs is given, which may cause U-Boot environment
> area to write into the rootfs
> area, causing the rootfs corruption.

Ah, that indeed explains the case. This type of exp[lanations definitely
should be in the commit log.

However, please not that there are two cases for
BR2_LINUX_KERNEL_INSTALL_TARGET=y:

    if grep -Eq "^BR2_TARGET_UBOOT_SPL=y$" ${BR2_CONFIG}; then
        echo "genimage.cfg.template_no_boot_part_spl"
    else
        echo "genimage.cfg.template_no_boot_part"
    fi

And this patch only addresses the second one.

Could you respin, fixing the second template, and including these
extended explanations in the commit log, please?

Regards,
Yann E. MORIN.

> Please let me know if you need more information as this problem is
> easy to reproduce.
> 
> Regards,
> 
> Fabio Estevam
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

* Re: [Buildroot] [PATCH RESEND] imx/genimage: Place the rootfs at a proper offset when BR2_LINUX_KERNEL_INSTALL_TARGET=y
  2021-11-13 21:02     ` Yann E. MORIN
@ 2021-11-13 21:09       ` Yann E. MORIN
  0 siblings, 0 replies; 5+ messages in thread
From: Yann E. MORIN @ 2021-11-13 21:09 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: Thomas Petazzoni, buildroot

Fabio, All,

On 2021-11-13 22:02 +0100, Yann E. MORIN spake thusly:
> On 2021-11-13 15:11 -0300, Fabio Estevam spake thusly:
> > On Sat, Nov 13, 2021 at 2:06 PM Thomas Petazzoni
> > <thomas.petazzoni@bootlin.com> wrote:
> > > I don't see the relationship between BR2_LINUX_KERNEL_INSTALL_TARGET=y
> > > and changing the offset of the rootfs to avoid rootfs corruption.
> > When BR2_LINUX_KERNEL_INSTALL_TARGET is not selected, then
> > board/freescale/common/imx/genimage.cfg.template is used as per the logic
> > inside board/freescale/common/imx/post-image.sh.
> > 
> > board/freescale/common/imx/genimage.cfg.template correctly puts the
> > rootfs at a safe offset.
> > 
> > With BR2_LINUX_KERNEL_INSTALL_TARGET=y, then
> > board/freescale/common/imx/genimage.cfg.template_no_boot_part is used and no
> > offset to the rootfs is given, which may cause U-Boot environment
> > area to write into the rootfs
> > area, causing the rootfs corruption.
> 
> Ah, that indeed explains the case. This type of exp[lanations definitely
> should be in the commit log.
> 
> However, please not that there are two cases for

s/not/note/

> BR2_LINUX_KERNEL_INSTALL_TARGET=y:
> 
>     if grep -Eq "^BR2_TARGET_UBOOT_SPL=y$" ${BR2_CONFIG}; then
>         echo "genimage.cfg.template_no_boot_part_spl"
>     else
>         echo "genimage.cfg.template_no_boot_part"
>     fi
> 
> And this patch only addresses the second one.

Of course, I checked the case where we do not put the kernel in target/,
and both the SPL and non-SPL cases do have an 8MiB offset for the 'boot'
partition...

Regards,
Yann E. MORIN.

> Could you respin, fixing the second template, and including these
> extended explanations in the commit log, please?
> 
> Regards,
> Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

end of thread, other threads:[~2021-11-13 21:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-13 14:38 [Buildroot] [PATCH RESEND] imx/genimage: Place the rootfs at a proper offset when BR2_LINUX_KERNEL_INSTALL_TARGET=y Fabio Estevam
2021-11-13 17:06 ` Thomas Petazzoni
2021-11-13 18:11   ` Fabio Estevam
2021-11-13 21:02     ` Yann E. MORIN
2021-11-13 21:09       ` Yann E. MORIN

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.