[bug#54615] [PATCH 0/2] Add riscv-pk and improve spike

[bug#54615] [PATCH 0/2] Add riscv-pk and improve spike

[Arun Isaac]

Patch 1 adds riscv-pk---a companion package for spike and something that is
meant to be used with it. riscv-pk is not meant to build on x86_64. So, a
regular `guix build riscv-pk' on a x86_64 machine will fail. Only `guix build
--target=riscv64-linux-gnu riscv-pk' will succeed.

Patch 2 improves our spike package by substituting the absolute path to dtc
instead of wrapping the spike executable.

Arun Isaac (2):
  gnu: spike: Substitute path to dtc instead of wrapping executables.
  gnu: Add riscv-pk.

 gnu/packages/virtualization.scm | 64 ++++++++++++++++++++++++++++-----
 1 file changed, 55 insertions(+), 9 deletions(-)

-- 
2.34.0

[bug#54615] [PATCH 1/2] gnu: spike: Substitute path to dtc instead of wrapping executables.

[Arun Isaac]

* gnu/packages/virtualization.scm (spike)[arguments]: Delete the wrap-binary
phase. Add a configure-dtc-path phase that substitutes the absolute path to
dtc.
---
 gnu/packages/virtualization.scm | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/gnu/packages/virtualization.scm b/gnu/packages/virtualization.scm
index bd297977df..9c86670ce7 100644
--- a/gnu/packages/virtualization.scm
+++ b/gnu/packages/virtualization.scm
@@ -26,6 +26,7 @@
 ;;; Copyright © 2021 Raghav Gururajan <rg@raghavgururajan.name>
 ;;; Copyright © 2022 Oleg Pykhalov <go.wigust@gmail.com>
 ;;; Copyright © 2022 Ekaitz Zarraga <ekaitz@elenq.tech>
+;;; Copyright © 2022 Arun Isaac <arunisaac@systemreboot.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1037,15 +1038,12 @@ (define-public spike
      (list
        #:phases
        #~(modify-phases %standard-phases
-           (add-after 'install 'wrap-binary
-             (lambda* (#:key inputs outputs #:allow-other-keys)
-               (let ((out (assoc-ref outputs "out")))
-                 (for-each
-                   (lambda (file)
-                     (wrap-program file
-                       `("PATH" ":" prefix
-                         (,(dirname (search-input-file inputs "/bin/dtc"))))))
-                   (find-files (string-append out "/bin")))))))))
+           (add-before 'configure 'configure-dtc-path
+             (lambda* (#:key inputs #:allow-other-keys)
+               ;; Reference dtc by its absolute store path.
+               (substitute* "riscv/dts.cc"
+                 (("DTC")
+                  (string-append "\"" (assoc-ref inputs "dtc") "/bin/dtc\""))))))))
     (inputs
      (list bash-minimal dtc))
     (native-inputs
-- 
2.34.0

[bug#54615] [PATCH 2/2] gnu: Add riscv-pk.

[Arun Isaac]

* gnu/packages/virtualization.scm (riscv-pk): New variable.
---
 gnu/packages/virtualization.scm | 48 +++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/gnu/packages/virtualization.scm b/gnu/packages/virtualization.scm
index 9c86670ce7..1f83addb1b 100644
--- a/gnu/packages/virtualization.scm
+++ b/gnu/packages/virtualization.scm
@@ -2365,3 +2365,51 @@ (define-public python-transient
      "@code{transient} is a wrapper for QEMU allowing the creation of virtual
 machines with shared folder, ssh, and disk creation support.")
     (license license:expat)))
+
+(define-public riscv-pk
+  (package
+    (name "riscv-pk")
+    (version "1.0.0")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/riscv-software-src/riscv-pk")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32
+         "1cc0rz4q3a1zw8756b8yysw8lb5g4xbjajh5lvqbjix41hbdx6xz"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:make-flags (list (string-append "INSTALLDIR="
+                                         (assoc-ref %outputs "out")))
+       ;; Add flags to keep symbols fromhost and tohost. These symbols are
+       ;; required for the correct functioning of pk.
+       #:strip-flags (list "--strip-unneeded"
+                           "--keep-symbol=fromhost"
+                           "--keep-symbol=tohost"
+                           "--enable-deterministic-archives")
+       #:phases
+       (modify-phases %standard-phases
+         (add-before 'configure 'pre-configure
+           (lambda _
+             ;; Building directly in the source tree is not supported. So,
+             ;; create a build directory and make configure accessible from
+             ;; there.
+             (mkdir-p "build")
+             (chdir "build")
+             (symlink "../configure" "configure")))
+         (delete 'strip))))
+    (home-page "https://github.com/riscv-software-src/riscv-pk")
+    (synopsis "RISC-V Proxy Kernel")
+    (description "The RISC-V Proxy Kernel, @command{pk}, is a lightweight
+application execution environment that can host statically-linked RISC-V ELF
+binaries.  It is designed to support tethered RISC-V implementations with
+limited I/O capability and thus handles I/O-related system calls by proxying
+them to a host computer.
+
+This package also contains the Berkeley Boot Loader, @command{bbl}, which is a
+supervisor execution environment for tethered RISC-V systems.  It is designed
+to host the RISC-V Linux port.")
+    (license license:bsd-3)))
-- 
2.34.0

[bug#54615] [PATCH v2 0/2] Add riscv-pk and improve spike

[Arun Isaac]

Hi Efraim,

I didn't realize you had already packaged riscv-pk in the guix-bioinformatics
channel. I copied the idea of using #:out-of-source? from your version of the
package. Here is an improved patchset.

In your version of the package, I didn't understand why you had #:target and
native-inputs. Could you enlighten me?

Thanks,
Arun

Arun Isaac (2):
  gnu: spike: Substitute path to dtc instead of wrapping executables.
  gnu: Add riscv-pk.

 gnu/packages/virtualization.scm | 54 +++++++++++++++++++++++++++------
 1 file changed, 45 insertions(+), 9 deletions(-)

-- 
2.34.0

[bug#54615] [PATCH v2 1/2] gnu: spike: Substitute path to dtc instead of wrapping executables.

[Arun Isaac]

* gnu/packages/virtualization.scm (spike)[arguments]: Delete the wrap-binary
phase. Add a configure-dtc-path phase that substitutes the absolute path to
dtc.
---
 gnu/packages/virtualization.scm | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/gnu/packages/virtualization.scm b/gnu/packages/virtualization.scm
index bd297977df..9c86670ce7 100644
--- a/gnu/packages/virtualization.scm
+++ b/gnu/packages/virtualization.scm
@@ -26,6 +26,7 @@
 ;;; Copyright © 2021 Raghav Gururajan <rg@raghavgururajan.name>
 ;;; Copyright © 2022 Oleg Pykhalov <go.wigust@gmail.com>
 ;;; Copyright © 2022 Ekaitz Zarraga <ekaitz@elenq.tech>
+;;; Copyright © 2022 Arun Isaac <arunisaac@systemreboot.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1037,15 +1038,12 @@ (define-public spike
      (list
        #:phases
        #~(modify-phases %standard-phases
-           (add-after 'install 'wrap-binary
-             (lambda* (#:key inputs outputs #:allow-other-keys)
-               (let ((out (assoc-ref outputs "out")))
-                 (for-each
-                   (lambda (file)
-                     (wrap-program file
-                       `("PATH" ":" prefix
-                         (,(dirname (search-input-file inputs "/bin/dtc"))))))
-                   (find-files (string-append out "/bin")))))))))
+           (add-before 'configure 'configure-dtc-path
+             (lambda* (#:key inputs #:allow-other-keys)
+               ;; Reference dtc by its absolute store path.
+               (substitute* "riscv/dts.cc"
+                 (("DTC")
+                  (string-append "\"" (assoc-ref inputs "dtc") "/bin/dtc\""))))))))
     (inputs
      (list bash-minimal dtc))
     (native-inputs
-- 
2.34.0

[bug#54615] [PATCH v2 1/2] gnu: spike: Substitute path to dtc instead of wrapping executables.

[Maxime Devos]

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

Arun Isaac schreef op di 29-03-2022 om 00:25 [+0530]:
> -                         (,(dirname (search-input-file inputs "/bin/dtc"))))))
> -                   (find-files (string-append out "/bin")))))))))
> +           (add-before 'configure 'configure-dtc-path
> +             (lambda* (#:key inputs #:allow-other-keys)
> +               ;; Reference dtc by its absolute store path.
> +               (substitute* "riscv/dts.cc"
> +                 (("DTC")
> +                  (string-append "\"" (assoc-ref inputs "dtc") "/bin/dtc\""))))))))

Why is search-input-file being replaced by assoc-ref?

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#54615] [PATCH v2 1/2] gnu: spike: Substitute path to dtc instead of wrapping executables.

[Arun Isaac]

Hi Maxime,

> %outputs is almost undocumented

> Why is search-input-file being replaced by assoc-ref?

Thanks for catching these! It looks like my knowledge of Guix packaging
is a bit behind the times. I didn't know about %outputs being
deprecated, and didn't even know about search-input-file! I've fixed
both issues. A v3 of the patchset follows.

Regards,
Arun

[bug#54615] [PATCH v3 1/2] gnu: spike: Substitute path to dtc instead of wrapping executables.

[Arun Isaac]

* gnu/packages/virtualization.scm (spike)[arguments]: Delete the wrap-binary
phase. Add a configure-dtc-path phase that substitutes the absolute path to
dtc.
---
 gnu/packages/virtualization.scm | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/gnu/packages/virtualization.scm b/gnu/packages/virtualization.scm
index bd297977df..4c0f02154c 100644
--- a/gnu/packages/virtualization.scm
+++ b/gnu/packages/virtualization.scm
@@ -26,6 +26,7 @@
 ;;; Copyright © 2021 Raghav Gururajan <rg@raghavgururajan.name>
 ;;; Copyright © 2022 Oleg Pykhalov <go.wigust@gmail.com>
 ;;; Copyright © 2022 Ekaitz Zarraga <ekaitz@elenq.tech>
+;;; Copyright © 2022 Arun Isaac <arunisaac@systemreboot.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1037,15 +1038,12 @@ (define-public spike
      (list
        #:phases
        #~(modify-phases %standard-phases
-           (add-after 'install 'wrap-binary
-             (lambda* (#:key inputs outputs #:allow-other-keys)
-               (let ((out (assoc-ref outputs "out")))
-                 (for-each
-                   (lambda (file)
-                     (wrap-program file
-                       `("PATH" ":" prefix
-                         (,(dirname (search-input-file inputs "/bin/dtc"))))))
-                   (find-files (string-append out "/bin")))))))))
+           (add-before 'configure 'configure-dtc-path
+             (lambda* (#:key inputs #:allow-other-keys)
+               ;; Reference dtc by its absolute store path.
+               (substitute* "riscv/dts.cc"
+                 (("DTC")
+                  (string-append "\"" (search-input-file inputs "/bin/dtc") "\""))))))))
     (inputs
      (list bash-minimal dtc))
     (native-inputs
-- 
2.34.0

[bug#54615] [PATCH v3 2/2] gnu: Add riscv-pk.

[Arun Isaac]

* gnu/packages/virtualization.scm (riscv-pk): New variable.
---
 gnu/packages/virtualization.scm | 39 +++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/gnu/packages/virtualization.scm b/gnu/packages/virtualization.scm
index 4c0f02154c..a058e59f6b 100644
--- a/gnu/packages/virtualization.scm
+++ b/gnu/packages/virtualization.scm
@@ -2365,3 +2365,42 @@ (define-public python-transient
      "@code{transient} is a wrapper for QEMU allowing the creation of virtual
 machines with shared folder, ssh, and disk creation support.")
     (license license:expat)))
+
+(define-public riscv-pk
+  (package
+    (name "riscv-pk")
+    (version "1.0.0")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/riscv-software-src/riscv-pk")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32
+         "1cc0rz4q3a1zw8756b8yysw8lb5g4xbjajh5lvqbjix41hbdx6xz"))))
+    (build-system gnu-build-system)
+    (arguments
+     (list #:out-of-source? #t
+           ;; riscv-pk can only be built for riscv64.
+           #:target "riscv64-linux-gnu"
+           #:make-flags #~(list (string-append "INSTALLDIR=" #$output))
+           ;; Add flags to keep symbols fromhost and tohost. These symbols are
+           ;; required for the correct functioning of pk.
+           #:strip-flags #~(list "--strip-unneeded"
+                                 "--keep-symbol=fromhost"
+                                 "--keep-symbol=tohost"
+                                 "--enable-deterministic-archives")))
+    (home-page "https://github.com/riscv-software-src/riscv-pk")
+    (synopsis "RISC-V Proxy Kernel")
+    (description "The RISC-V Proxy Kernel, @command{pk}, is a lightweight
+application execution environment that can host statically-linked RISC-V ELF
+binaries.  It is designed to support tethered RISC-V implementations with
+limited I/O capability and thus handles I/O-related system calls by proxying
+them to a host computer.
+
+This package also contains the Berkeley Boot Loader, @command{bbl}, which is a
+supervisor execution environment for tethered RISC-V systems.  It is designed
+to host the RISC-V Linux port.")
+    (license license:bsd-3)))
-- 
2.34.0

bug#54615: [PATCH v3 2/2] gnu: Add riscv-pk.

[Efraim Flashner]

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

Thanks for the patches. Does it make sense to wrap spike with riscv-pk?
Right now I'm running it with:
guix shell spike riscv-pk -- sh -c 'spike $GUIX_ENVIRONMENT/bin/pk /gnu/store/hzd70l3dbgf66m4ibf34dzl7sif06f3k-hello-static-2.12/bin/hello'

I'm not sure how much spike is used without pk.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

[bug#54615] [PATCH v3 2/2] gnu: Add riscv-pk.

[Arun Isaac]

> Thanks for the patches. Does it make sense to wrap spike with
> riscv-pk?
>
> I'm not sure how much spike is used without pk.

I see and feel your pain. :-P But, maybe it's best to not make that
assumption. I don't know enough about riscv-pk to say for sure.

The better solution may be for spike to look in $PATH when trying to run
a program. I have asked for this upstream:
https://github.com/riscv-software-src/riscv-isa-sim/issues/961

Thank you both for reviewing and merging these patches!

[bug#54615] [PATCH v2 2/2] gnu: Add riscv-pk.

[Arun Isaac]

* gnu/packages/virtualization.scm (riscv-pk): New variable.
---
 gnu/packages/virtualization.scm | 38 +++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gnu/packages/virtualization.scm b/gnu/packages/virtualization.scm
index 9c86670ce7..5904f274f5 100644
--- a/gnu/packages/virtualization.scm
+++ b/gnu/packages/virtualization.scm
@@ -2365,3 +2365,41 @@ (define-public python-transient
      "@code{transient} is a wrapper for QEMU allowing the creation of virtual
 machines with shared folder, ssh, and disk creation support.")
     (license license:expat)))
+
+(define-public riscv-pk
+  (package
+    (name "riscv-pk")
+    (version "1.0.0")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/riscv-software-src/riscv-pk")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32
+         "1cc0rz4q3a1zw8756b8yysw8lb5g4xbjajh5lvqbjix41hbdx6xz"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:out-of-source? #t
+       #:make-flags (list (string-append "INSTALLDIR="
+                                         (assoc-ref %outputs "out")))
+       ;; Add flags to keep symbols fromhost and tohost. These symbols are
+       ;; required for the correct functioning of pk.
+       #:strip-flags (list "--strip-unneeded"
+                           "--keep-symbol=fromhost"
+                           "--keep-symbol=tohost"
+                           "--enable-deterministic-archives")))
+    (home-page "https://github.com/riscv-software-src/riscv-pk")
+    (synopsis "RISC-V Proxy Kernel")
+    (description "The RISC-V Proxy Kernel, @command{pk}, is a lightweight
+application execution environment that can host statically-linked RISC-V ELF
+binaries.  It is designed to support tethered RISC-V implementations with
+limited I/O capability and thus handles I/O-related system calls by proxying
+them to a host computer.
+
+This package also contains the Berkeley Boot Loader, @command{bbl}, which is a
+supervisor execution environment for tethered RISC-V systems.  It is designed
+to host the RISC-V Linux port.")
+    (license license:bsd-3)))
-- 
2.34.0

[bug#54615] [PATCH v2 2/2] gnu: Add riscv-pk.

[Maxime Devos]

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

Arun Isaac schreef op di 29-03-2022 om 00:25 [+0530]:
> +       #:make-flags (list (string-append "INSTALLDIR="
> +                                         (assoc-ref %outputs "out")))

%outputs is almost undocumented (and for some build systems even
undefined), I recommend G-exps instead:

  (arguments
    (list #:make-flags
          #~(list (string-append "INSTALLDIR=" #$output))
          ...))

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#54615] [PATCH v2 0/2] Add riscv-pk and improve spike

[Efraim Flashner]

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

On Tue, Mar 29, 2022 at 12:25:55AM +0530, Arun Isaac wrote:
> Hi Efraim,
> 
> I didn't realize you had already packaged riscv-pk in the guix-bioinformatics
> channel. I copied the idea of using #:out-of-source? from your version of the
> package. Here is an improved patchset.
> 
> In your version of the package, I didn't understand why you had #:target and
> native-inputs. Could you enlighten me?
> 
> Thanks,
> Arun

The native-inputs I got rid of because they weren't needed in the end, I
just hadn't committed the change yet. For target I was under the
impression that it had to be compiled targeting
riscv64-(unknown-)?linux-gnu so I had that there. The instructions
upstream involved using a configure-flag instead, which is why I started
with the native-inputs.

> Arun Isaac (2):
>   gnu: spike: Substitute path to dtc instead of wrapping executables.
>   gnu: Add riscv-pk.
> 
>  gnu/packages/virtualization.scm | 54 +++++++++++++++++++++++++++------
>  1 file changed, 45 insertions(+), 9 deletions(-)
> 
> -- 
> 2.34.0
> 

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

[bug#54615] [PATCH v2 0/2] Add riscv-pk and improve spike

[Arun Isaac]

> The native-inputs I got rid of because they weren't needed in the end, I
> just hadn't committed the change yet. For target I was under the
> impression that it had to be compiled targeting
> riscv64-(unknown-)?linux-gnu so I had that there. The instructions
> upstream involved using a configure-flag instead, which is why I started
> with the native-inputs.

Thanks, that clears it up! I've added #:target in v3 of my patchset, and
will send it shortly.