[PATCH 0/2] system: image: Fix root fs corruption from certain u-boot.

  • Done
  • quality assurance status badge
Details
2 participants
  • Caliph Nomble
  • Mathieu Othacehe
Owner
unassigned
Submitted by
Caliph Nomble
Severity
normal
C
C
Caliph Nomble wrote on 1 Jan 2021 00:34
(name . guix-patches@gnu.org)(address . guix-patches@gnu.org)
AJu4ls8-KuZO9eCkkVO_MApWUbxUt-RLNY1iy-xT-w-5FZV5_9QXIdGfPDReJhPqLExElN6z9rb5vD1pDJ22QZW_sn2HwKlAGAAohucNMuw=@protonmail.com
Hi,

u-boot for pretty much every rockchip system supported installs to an offset of
8MB, which is currently in the middle of generated disk images' root
filesystems. This patchset adds another disk-image option for devices that
require a larger root offset, and configures the pinebook-pro image to use it.

I chose a 9MB offset by adding the previous root offset of 1MB to the
additional 8MB offset required. I know at least u-boot-rockpro64-rk3399 doesn't
build to over 1MB in size, so it should be fine? Feel free to change the
specific offset used.

Thanks!
C
C
Caliph Nomble wrote on 1 Jan 2021 02:11
[PATCH 1/2] system: image: Add support for rockchip bootloader offsets.
(address . 45584@debbugs.gnu.org)(name . Caliph Nomble)(address . calnomble@protonmail.com)
CPrSnKXmKTMtnTOqOCUH3Mu8UdaGrGnlYu7QEGzYqw@cp3-web-020.plabs.ch
* gnu/system/image.scm (root-largeboot-offset, arm32-largeboot-disk-image,
arm32-largeboot-image-type): New variables, used primarily to define...
(arm64-largeboot-disk-image,
arm64-largeboot-image-type): ...these new variables.
---
gnu/system/image.scm | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

Toggle diff (92 lines)
diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index 67930750d5..58d92cf4b6 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -59,6 +59,7 @@
#:use-module (ice-9 format)
#:use-module (ice-9 match)
#:export (root-offset
+ root-largeboot-offset
root-label
esp-partition
@@ -67,7 +68,9 @@
efi-disk-image
iso9660-image
arm32-disk-image
+ arm32-largeboot-disk-image
arm64-disk-image
+ arm64-largeboot-disk-image
image-with-os
raw-image-type
@@ -75,7 +78,9 @@
iso-image-type
uncompressed-iso-image-type
arm32-image-type
+ arm32-largeboot-image-type
arm64-image-type
+ arm64-largeboot-image-type
image-with-label
system-image
@@ -92,6 +97,10 @@
;; this post-MBR gap.
(define root-offset (* 512 2048))
+;; Same as above, except 9MB big. Necessary for eg. rk3399 and rk3328
+;; systems, which install the bootloader at an 8MB offset.
+(define root-largeboot-offset (+ root-offset (* 8 (expt 2 20))))
+
;; Generic root partition label.
(define root-label "Guix_image")
@@ -140,11 +149,24 @@
;; fails.
(volatile-root? #f)))
+(define arm32-largeboot-disk-image
+ (image
+ (inherit arm32-disk-image)
+ (partitions
+ (list (partition
+ (inherit root-partition)
+ (offset root-largeboot-offset))))))
+
(define arm64-disk-image
(image
(inherit arm32-disk-image)
(target "aarch64-linux-gnu")))
+(define arm64-largeboot-disk-image
+ (image
+ (inherit arm32-largeboot-disk-image)
+ (target "aarch64-linux-gnu")))
+
;;;
;;; Images types.
@@ -191,11 +213,21 @@ set to the given OS."
(name 'arm32-raw)
(constructor (cut image-with-os arm32-disk-image <>))))
+(define arm32-largeboot-image-type
+ (image-type
+ (name 'arm32-largeboot-raw)
+ (constructor (cut image-with-os arm32-largeboot-disk-image <>))))
+
(define arm64-image-type
(image-type
(name 'arm64-raw)
(constructor (cut image-with-os arm64-disk-image <>))))
+(define arm64-largeboot-image-type
+ (image-type
+ (name 'arm64-largeboot-raw)
+ (constructor (cut image-with-os arm64-largeboot-disk-image <>))))
+
;;
;; Helpers.
--
2.26.2
C
C
Caliph Nomble wrote on 1 Jan 2021 02:12
[PATCH 2/2] image: pinebook-pro: Fix pinebook-pro root filesystem.
(address . 45584@debbugs.gnu.org)(name . Caliph Nomble)(address . calnomble@protonmail.com)
VvRVKj5g1rzAMpqASaPvAoZiiPnpeEfblPsvmHeRE@cp4-web-032.plabs.ch
* gnu/system/images/pinebook-pro.scm (pinebook-pro-image-type): Use
arm64-largeboot-disk-image instead of arm64-disk-image.
---
gnu/system/images/pinebook-pro.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Toggle diff (15 lines)
diff --git a/gnu/system/images/pinebook-pro.scm b/gnu/system/images/pinebook-pro.scm
index b038e262cb..e4bb808e3c 100644
--- a/gnu/system/images/pinebook-pro.scm
+++ b/gnu/system/images/pinebook-pro.scm
@@ -57,7 +57,7 @@
(define pinebook-pro-image-type
(image-type
(name 'pinebook-pro-raw)
- (constructor (cut image-with-os arm64-disk-image <>))))
+ (constructor (cut image-with-os arm64-largeboot-disk-image <>))))
(define pinebook-pro-barebones-raw-image
(image
--
2.26.2
M
M
Mathieu Othacehe wrote on 1 Jan 2021 17:10
Re: [bug#45584] [PATCH 0/2] system: image: Fix root fs corruption from certain u-boot.
(name . Caliph Nomble via Guix-patches via)(address . guix-patches@gnu.org)
87zh1swsch.fsf@gnu.org
Hello,

Toggle quote (5 lines)
> I chose a 9MB offset by adding the previous root offset of 1MB to the
> additional 8MB offset required. I know at least u-boot-rockpro64-rk3399 doesn't
> build to over 1MB in size, so it should be fine? Feel free to change the
> specific offset used.

This seems reasonable to me. We could also turn "arm32-disk-image" and
"arm64-disk-image" into procedures and pass the root offset as argument
to avoid multiplying image types. Danny, Vagrant, any opinion?

Were you able to actually use the generated image on a pinebook-pro? I
added support for this machine without being able to test it.

Thanks,

Mathieu
C
C
Caliph Nomble wrote on 2 Jan 2021 00:32
(name . 45584@debbugs.gnu.org)(address . 45584@debbugs.gnu.org)(name . othacehe@gnu.org)(address . othacehe@gnu.org)
LCsju6Xhd7Uqz2cPgtoYT3txjfm-JZIiNIkXMpzTtlQ0rXk7wfZFhT_UrWbEUDymoyVATd9KKG_IcT8QQG8tFnz-mA5NRYC8WBOXbkF5pCo=@protonmail.com
Hi,

Toggle quote (4 lines)
> This seems reasonable to me. We could also turn "arm32-disk-image" and
> "arm64-disk-image" into procedures and pass the root offset as argument
> to avoid multiplying image types. Danny, Vagrant, any opinion?

I was originally considering doing something like that, but I was unsure how
that'd interact with --image-type (unless there's another way to specify an
image that I don't know of)? Would there still just be multiple image-types but
with procedural disk-images?

Toggle quote (3 lines)
> Were you able to actually use the generated image on a pinebook-pro? I
> added support for this machine without being able to test it.

I just tried to use it on one, but I was unable to get it to boot. I'm not sure
why, exactly, as I didn't have a chance to open it up to enable serial. Could
just be the kernel, seeing as wip-pinebook-pro has its own patched linux-libre
(which I was unable to get working with an inferior on the main branch, but
I don't really know how to use them anyway).

I did, however, test these changes on a rockpro64 (both pinebook-pro and
rockpro64 use the rk3399 SoC), and it did fix fs corruption preventing proper
boot.

Thanks!
M
M
Mathieu Othacehe wrote on 2 Jan 2021 18:03
(name . Caliph Nomble)(address . calnomble@protonmail.com)(name . 45584@debbugs.gnu.org)(address . 45584@debbugs.gnu.org)
87zh1rp8yx.fsf@gnu.org
Hey,

Toggle quote (5 lines)
> I was originally considering doing something like that, but I was unsure how
> that'd interact with --image-type (unless there's another way to specify an
> image that I don't know of)? Would there still just be multiple image-types but
> with procedural disk-images?

Here's an attached patch that should do that, but I'm not sure it brings
a real improvement, unless there are a lot of different offset out
there.

Toggle quote (9 lines)
>> Were you able to actually use the generated image on a pinebook-pro? I
>> added support for this machine without being able to test it.
>
> I just tried to use it on one, but I was unable to get it to boot. I'm not sure
> why, exactly, as I didn't have a chance to open it up to enable serial. Could
> just be the kernel, seeing as wip-pinebook-pro has its own patched linux-libre
> (which I was unable to get working with an inferior on the main branch, but
> I don't really know how to use them anyway).

It looks like the wip-pinebook-pro is adding some kernel patches, maybe
you could try to apply them directly on master?

Toggle quote (4 lines)
> I did, however, test these changes on a rockpro64 (both pinebook-pro and
> rockpro64 use the rk3399 SoC), and it did fix fs corruption preventing proper
> boot.

That's nice, thanks for your work!

Mathieu
From 2c0806c28ae5ca07cba136ce2e32a7de0702693d Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Sat, 2 Jan 2021 17:56:25 +0100
Subject: [PATCH] offset

---
gnu/system/image.scm | 12 ++++++------
gnu/system/images/novena.scm | 2 +-
gnu/system/images/pine64.scm | 2 +-
gnu/system/images/pinebook-pro.scm | 4 +++-
4 files changed, 11 insertions(+), 9 deletions(-)

Toggle diff (88 lines)
diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index 67930750d5..90b9209988 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -128,21 +128,21 @@
(label "GUIX_IMAGE")
(flags '(boot)))))))
-(define arm32-disk-image
+(define* (arm32-disk-image #:optional (offset root-offset))
(image
(format 'disk-image)
(target "arm-linux-gnueabihf")
(partitions
(list (partition
(inherit root-partition)
- (offset root-offset))))
+ (offset offset))))
;; FIXME: Deleting and creating "/var/run" and "/tmp" on the overlayfs
;; fails.
(volatile-root? #f)))
-(define arm64-disk-image
+(define* (arm64-disk-image #:optional (offset root-offset))
(image
- (inherit arm32-disk-image)
+ (inherit (arm32-disk-image offset))
(target "aarch64-linux-gnu")))
@@ -189,12 +189,12 @@ set to the given OS."
(define arm32-image-type
(image-type
(name 'arm32-raw)
- (constructor (cut image-with-os arm32-disk-image <>))))
+ (constructor (cut image-with-os (arm32-disk-image) <>))))
(define arm64-image-type
(image-type
(name 'arm64-raw)
- (constructor (cut image-with-os arm64-disk-image <>))))
+ (constructor (cut image-with-os (arm64-disk-image) <>))))
;;
diff --git a/gnu/system/images/novena.scm b/gnu/system/images/novena.scm
index c4d25e850e..dfaf2c60ee 100644
--- a/gnu/system/images/novena.scm
+++ b/gnu/system/images/novena.scm
@@ -52,7 +52,7 @@
(define novena-image-type
(image-type
(name 'novena-raw)
- (constructor (cut image-with-os arm32-disk-image <>))))
+ (constructor (cut image-with-os (arm32-disk-image) <>))))
(define novena-barebones-raw-image
(image
diff --git a/gnu/system/images/pine64.scm b/gnu/system/images/pine64.scm
index f0b0c3f50d..63b31399a5 100644
--- a/gnu/system/images/pine64.scm
+++ b/gnu/system/images/pine64.scm
@@ -57,7 +57,7 @@
(define pine64-image-type
(image-type
(name 'pine64-raw)
- (constructor (cut image-with-os arm64-disk-image <>))))
+ (constructor (cut image-with-os (arm64-disk-image) <>))))
(define pine64-barebones-raw-image
(image
diff --git a/gnu/system/images/pinebook-pro.scm b/gnu/system/images/pinebook-pro.scm
index b038e262cb..02a0b8132d 100644
--- a/gnu/system/images/pinebook-pro.scm
+++ b/gnu/system/images/pinebook-pro.scm
@@ -57,7 +57,9 @@
(define pinebook-pro-image-type
(image-type
(name 'pinebook-pro-raw)
- (constructor (cut image-with-os arm64-disk-image <>))))
+ (constructor (cut image-with-os
+ (arm64-disk-image (* 9 (expt 2 20))) ;9MiB
+ <>))))
(define pinebook-pro-barebones-raw-image
(image
--
2.29.2
C
C
Caliph Nomble wrote on 10 Jan 2021 01:48
(name . 45584@debbugs.gnu.org)(address . 45584@debbugs.gnu.org)(name . Mathieu Othacehe)(address . othacehe@gnu.org)
XaDnfECdXA7BsEEFYRTYm2DkOtRe2-BlVF-k9jz-X30JFOMxNdCihvx8BN7WBzEKqJ-QLz4HMHC74I5i6-Ao5c2mE4B63NOYy38SpIdBCuk=@protonmail.com
Hi,

Sorry for the late response. I just got a chance to test on a pinebook-pro, and
was able to use a generated image on it. Screen wouldn't turn on but I got a
functioning shell on serial; I was still just using mainline
linux-libre-arm64-generic so I don't think that's a bootloader problem.

Toggle quote (4 lines)
> Here's an attached patch that should do that, but I'm not sure it brings
> a real improvement, unless there are a lot of different offset out
> there.

I'm all for extensability, so that sounds good to me.

Over the past few days I've been playing with u-boot, and I think that the
install offset may be configurable, with the current offset just being used for
compatability with Rockchip's proprietary miniloader. I could make a patchset
to move u-boot up and test it out on the pinebook-pro and rockpro64, if that
would be a better solution?

Thanks!
From 2c0806c28ae5ca07cba136ce2e32a7de0702693d Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Sat, 2 Jan 2021 17:56:25 +0100
Subject: [PATCH] offset

---
gnu/system/image.scm | 12 ++++++------
gnu/system/images/novena.scm | 2 +-
gnu/system/images/pine64.scm | 2 +-
gnu/system/images/pinebook-pro.scm | 4 +++-
4 files changed, 11 insertions(+), 9 deletions(-)

Toggle diff (88 lines)
diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index 67930750d5..90b9209988 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -128,21 +128,21 @@
(label "GUIX_IMAGE")
(flags '(boot)))))))
-(define arm32-disk-image
+(define* (arm32-disk-image #:optional (offset root-offset))
(image
(format 'disk-image)
(target "arm-linux-gnueabihf")
(partitions
(list (partition
(inherit root-partition)
- (offset root-offset))))
+ (offset offset))))
;; FIXME: Deleting and creating "/var/run" and "/tmp" on the overlayfs
;; fails.
(volatile-root? #f)))
-(define arm64-disk-image
+(define* (arm64-disk-image #:optional (offset root-offset))
(image
- (inherit arm32-disk-image)
+ (inherit (arm32-disk-image offset))
(target "aarch64-linux-gnu")))
@@ -189,12 +189,12 @@ set to the given OS."
(define arm32-image-type
(image-type
(name 'arm32-raw)
- (constructor (cut image-with-os arm32-disk-image <>))))
+ (constructor (cut image-with-os (arm32-disk-image) <>))))
(define arm64-image-type
(image-type
(name 'arm64-raw)
- (constructor (cut image-with-os arm64-disk-image <>))))
+ (constructor (cut image-with-os (arm64-disk-image) <>))))
;;
diff --git a/gnu/system/images/novena.scm b/gnu/system/images/novena.scm
index c4d25e850e..dfaf2c60ee 100644
--- a/gnu/system/images/novena.scm
+++ b/gnu/system/images/novena.scm
@@ -52,7 +52,7 @@
(define novena-image-type
(image-type
(name 'novena-raw)
- (constructor (cut image-with-os arm32-disk-image <>))))
+ (constructor (cut image-with-os (arm32-disk-image) <>))))
(define novena-barebones-raw-image
(image
diff --git a/gnu/system/images/pine64.scm b/gnu/system/images/pine64.scm
index f0b0c3f50d..63b31399a5 100644
--- a/gnu/system/images/pine64.scm
+++ b/gnu/system/images/pine64.scm
@@ -57,7 +57,7 @@
(define pine64-image-type
(image-type
(name 'pine64-raw)
- (constructor (cut image-with-os arm64-disk-image <>))))
+ (constructor (cut image-with-os (arm64-disk-image) <>))))
(define pine64-barebones-raw-image
(image
diff --git a/gnu/system/images/pinebook-pro.scm b/gnu/system/images/pinebook-pro.scm
index b038e262cb..02a0b8132d 100644
--- a/gnu/system/images/pinebook-pro.scm
+++ b/gnu/system/images/pinebook-pro.scm
@@ -57,7 +57,9 @@
(define pinebook-pro-image-type
(image-type
(name 'pinebook-pro-raw)
- (constructor (cut image-with-os arm64-disk-image <>))))
+ (constructor (cut image-with-os
+ (arm64-disk-image (* 9 (expt 2 20))) ;9MiB
+ <>))))
(define pinebook-pro-barebones-raw-image
(image
--
2.29.2
C
C
Caliph Nomble wrote on 10 Jan 2021 01:53
(name . 45584@debbugs.gnu.org)(address . 45584@debbugs.gnu.org)(name . Mathieu Othacehe)(address . othacehe@gnu.org)
nl5tRAZ2X9wzTKvdTRnQNuYUKRbTA_UCvrTcDSJZXYy-b7i7eed36xlmYJ93nDfhYaf9qFRp93YpSJfVdcEAThLSM14hFG0Irf0IuPyqlII=@protonmail.com
Sorry, I accidentally attached your patch to my last message. Disregard it.
M
M
Mathieu Othacehe wrote on 10 Jan 2021 16:16
(name . Caliph Nomble)(address . calnomble@protonmail.com)
871resg6uj.fsf@gnu.org
Hello,

Toggle quote (5 lines)
> Sorry for the late response. I just got a chance to test on a pinebook-pro, and
> was able to use a generated image on it. Screen wouldn't turn on but I got a
> functioning shell on serial; I was still just using mainline
> linux-libre-arm64-generic so I don't think that's a bootloader problem.

Many thanks for testing. I pushed the patch I proposed.

Toggle quote (6 lines)
> Over the past few days I've been playing with u-boot, and I think that the
> install offset may be configurable, with the current offset just being used for
> compatability with Rockchip's proprietary miniloader. I could make a patchset
> to move u-boot up and test it out on the pinebook-pro and rockpro64, if that
> would be a better solution?

Sure that could be nice and in that case we could revert the 9MiB offset
change in gnu/system/image/pinebook-pro.scm.

I also found that janneke used some kernel-arguments that could maybe
help you in this blog post:

Closing this one,

Thanks,

Mathieu
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 45584@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 45584
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch