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

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

[Caliph Nomble via Guix-patches via]

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!

[bug#45584] [PATCH 1/2] system: image: Add support for rockchip bootloader offsets.

[Caliph Nomble via Guix-patches via]

* 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(+)

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")))
+
 \f
 ;;;
 ;;; 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 <>))))
+
 \f
 ;;
 ;; Helpers.
-- 
2.26.2

[bug#45584] [PATCH 2/2] image: pinebook-pro: Fix pinebook-pro root filesystem.

[Caliph Nomble via Guix-patches via]

* 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(-)

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

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

[Mathieu Othacehe]

Hello,

> 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

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

[Caliph Nomble via Guix-patches via]

Hi,

> 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?

> 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!

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

[Mathieu Othacehe]

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


Hey,

> 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.

>> 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?

> 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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-offset.patch --]
[-- Type: text/x-diff, Size: 3123 bytes --]

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(-)

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")))
 
 \f
@@ -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) <>))))
 
 \f
 ;;
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

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

[Caliph Nomble via Guix-patches via]

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

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.

> 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!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-offset.patch --]
[-- Type: text/x-diff; name=0001-offset.patch, Size: 3123 bytes --]

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(-)

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")))
 
 \f
@@ -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) <>))))
 
 \f
 ;;
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

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

[Caliph Nomble via Guix-patches via]

Sorry, I accidentally attached your patch to my last message. Disregard it.

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

[Mathieu Othacehe]

Hello,

> 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.

> 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:
https://joyofsource.com/guix-system-on-the-pinebook-pro.html.

Closing this one,

Thanks,

Mathieu