diff mbox

[bug#69899,0/7] support to use `guix system vm' with riscv64.

Message ID 87zfub3h8l.fsf@iscas.ac.cn
State New
Headers show

Commit Message

Z572 April 1, 2024, 5:20 p.m. UTC 
Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Zheng Junjie <zhengjunjie@iscas.ac.cn> skribis:
>
>> Leo Famulari <leo@famulari.name> writes:
>>
>>> Hi, thanks for these patches!
>>>
>>> On Tue, Mar 19, 2024 at 11:15:53PM +0800, Zheng Junjie wrote:
>>>> hello!. this patchset make `guix system vm --target=riscv64-linux-gnu' work.
>>>> and `guix system vm --system=riscv64-linux` also work, But there is a problem
>>>> that the qemu running with --system=riscv64-linux is also riscv architecture,
>>>> now you have to manually modify to use native qemu or the guix of qemu-system
>>>> on qemu-user is too slow. and must enable qemu-binfmt.
>>>
>>> So, what should we do about the problem? What's the ideal solution for
>>> Guix? I don't have any experience with this subject matter.
>>
>> I've come up with two ideas:
>>
>> 1. Force the current system's qemu to be used, on x86_64 and -s
>> riscv64-linux, use x86_64's qemu.
>> 2. add an environment variable, GUIX_QEMU, to allow the internal qemu
>> execution to be replaced
>>
>> There may be other solutions, but I can't think of them yet.
>
> Thing is, ‘--system=X’ is supposed to be giving the exact same result as
> if you were building natively on X.  Thus, it’s not surprising that
> ‘guix system vm --system=X’ gives on a QEMU binary built for X.
>
> Now, it’s admittedly not very useful in this case.  I believe the
> attached patch implements #1 (I wasn’t able to test it yet because too
> many things had to be built).  How does it sound?

it works, and the following patch is required.
And maybe shebang interpreter also need override %CURRENT-SYSTEM?[1]
Even though when the Shebang interpreter cannot execute, the file will
be interpreted and executed by the current shell.

>
> diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
> index 7d9d07ebb7..a2743453e7 100644
> --- a/gnu/system/vm.scm
> +++ b/gnu/system/vm.scm
> @@ -287,8 +287,11 @@ (define* (system-qemu-image/shared-store-script os
>        #~(format #f "/tmp/guix-image-~a" (basename #$base-image)))
>  
>      (define qemu-exec
> -      #~(list #+(file-append qemu "/bin/"
> -                             (qemu-command (or target system)))
> +      #~(list #+(with-parameters ((%current-system %system)
> +                                  (%current-target-system #f))
> +                  ;; Override %CURRENT-SYSTEM to always use a native emulator.
> +                  (file-append qemu "/bin/"
> +                               (qemu-command (or target system))))
>                ;; Tells qemu to use the terminal it was started in for IO.
>                #$@(if graphic? '() #~("-nographic"))
>                #$@(if full-boot?
>
>
> Thanks,
> Ludo’.

[1] https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/system/vm.scm?h=9e9ec741d0dc5ce58f8d21d31800ff2cafce128f#n328

Comments

Leo Famulari April 3, 2024, 10:42 p.m. UTC | #1 
On Tue, Apr 02, 2024 at 01:20:12AM +0800, Zheng Junjie wrote:
> it works, and the following patch is required.

Great! Does this mean you can send a ready-to-build v2 patch series
including these additions?
Ludovic Courtès April 16, 2024, 5:04 p.m. UTC | #2 
Zheng Junjie <zhengjunjie@iscas.ac.cn> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> Thing is, ‘--system=X’ is supposed to be giving the exact same result as
>> if you were building natively on X.  Thus, it’s not surprising that
>> ‘guix system vm --system=X’ gives on a QEMU binary built for X.
>>
>> Now, it’s admittedly not very useful in this case.  I believe the
>> attached patch implements #1 (I wasn’t able to test it yet because too
>> many things had to be built).  How does it sound?
>
> it works,

Great, I’ll push it shortly.

> and the following patch is required.
>
> From e178de931859399abdd4bd3300b7ce7c5593a6e3 Mon Sep 17 00:00:00 2001
> Message-ID: <e178de931859399abdd4bd3300b7ce7c5593a6e3.1712026332.git.zhengjunjie@iscas.ac.cn>
> From: Zheng Junjie <zhengjunjie@iscas.ac.cn>
> Date: Tue, 2 Apr 2024 00:59:54 +0800
> Subject: [PATCH] vm: Don't add -enable-kvm when SYSTEM and %system are not
>  same.
>
> * gnu/system/vm.scm (common-qemu-options): Don't add -enable-kvm when SYSTEM
> and %system are not same.
>
> Change-Id: Ie6c602b297c39423a693fdc26bed1627266e5911

[...]

> +     #$@(if (and (not target)
> +                 (string=? system %system)
> +                 (file-exists? "/dev/kvm"))

I’m afraid this is too simple: we still want KVM when emulating i686 on
x86_64, or armhf on aarch64.

Does it really hurt to pass ‘-enable-kvm’?  Can you think of another
way?

Thanks,
Ludo’.
diff mbox

Patch

From e178de931859399abdd4bd3300b7ce7c5593a6e3 Mon Sep 17 00:00:00 2001
Message-ID: <e178de931859399abdd4bd3300b7ce7c5593a6e3.1712026332.git.zhengjunjie@iscas.ac.cn>
From: Zheng Junjie <zhengjunjie@iscas.ac.cn>
Date: Tue, 2 Apr 2024 00:59:54 +0800
Subject: [PATCH] vm: Don't add -enable-kvm when SYSTEM and %system are not
 same.

* gnu/system/vm.scm (common-qemu-options): Don't add -enable-kvm when SYSTEM
and %system are not same.

Change-Id: Ie6c602b297c39423a693fdc26bed1627266e5911
---
 gnu/system/vm.scm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index a2743453e7..42e9a08722 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -212,7 +212,8 @@  (define* (virtualized-operating-system os
 (define* (common-qemu-options image shared-fs
                               #:key
                               rw-image?
-                              (target (%current-target-system)))
+                              (target (%current-target-system))
+                              (system (%current-system)))
   "Return the a string-value gexp with the common QEMU options to boot IMAGE,
 with '-virtfs' options for the host file systems listed in SHARED-FS."
 
@@ -223,7 +224,9 @@  (define* (common-qemu-options image shared-fs
   #~(;; Only enable kvm if we see /dev/kvm exists.
      ;; This allows users without hardware virtualization to still use these
      ;; commands.
-     #$@(if (and (not target) (file-exists? "/dev/kvm"))
+     #$@(if (and (not target)
+                 (string=? system %system)
+                 (file-exists? "/dev/kvm"))
             '("-enable-kvm")
             '())
 
@@ -308,7 +311,8 @@  (define* (system-qemu-image/shared-store-script os
                                       (map file-system-mapping-source
                                            (cons %store-mapping mappings))
                                       #:rw-image? (not volatile?)
-                                      #:target target)
+                                      #:target target
+                                      #:system system)
               "-m " (number->string #$memory-size)
               #$@options))
 

base-commit: 9e9ec741d0dc5ce58f8d21d31800ff2cafce128f
prerequisite-patch-id: 6183d199c58355eea1a85e1f1fe51f2f5fe44f65
-- 
2.41.0