Subject: Re: [PATCH 9/9] gnu: Add ovmf-loongarch64.

From: Maxim Cournoyer <[email-masked]>
Date: Sat, 22 Mar 2025 16:33:37 +0900

Hello,

Efraim Flashner <[email-masked]> writes:

> * gnu/packages/firmware.scm (ovmf-loongarch64): New variable.
> (make-ovmf-firmware)[arguments]: Adjust the 'build phase to add a case
> for loongarch64.
>
> Change-Id: Ibb916b50fd3dc7831ac895cb8b59695688750377
> ---
>  gnu/packages/firmware.scm | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/gnu/packages/firmware.scm b/gnu/packages/firmware.scm
> index fe2956fbfdf..6f784a281d3 100644
> --- a/gnu/packages/firmware.scm
> +++ b/gnu/packages/firmware.scm
> @@ -1090,6 +1090,8 @@ (define* (make-ovmf-firmware arch)
>                                  "ArmVirtPkg/ArmVirtQemu.dsc")
>                                 ("riscv64"
>                                  "OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc")
> +                               ("loongarch64"
> +                                "OvmfPkg/LoongArchVirt/LoongArchVirtQemu.dsc")
>                                 (_ #t)))))
>              (add-before 'install 'install-efi-shell
>                (lambda _
> @@ -1247,6 +1249,25 @@ (define-public ovmf-riscv64
>                        (install-file "RISCV_VIRT_CODE.fd" fmw)
>                        (install-file "RISCV_VIRT_VARS.fd" fmw))))))))))))
>  
> +(define-public ovmf-loongarch64
> +  (let ((base (make-ovmf-firmware "loongarch64")))
> +    (package
> +      (inherit base)
> +      (arguments
> +        (substitute-keyword-arguments (package-arguments base)
> +          ((#:phases phases)
> +           #~(modify-phases #$phases
> +               (replace 'install
> +                 (lambda _
> +                   (let ((fmw (string-append #$output "/share/firmware")))
> +                     (mkdir-p fmw)

Nitpick: fw is more commond than fwm as an abbrevation.

> +                     (with-directory-excursion "Build/LoongArchVirtQemu/RELEASE_GCC/FV"

Other nitpick: I'd break the directory part on a newline to fit under 80
columns, per our code style guidelines.

Otherwise, my general comments elsewhere apply it too, but it LGTM
otherwise.

I think we should add the QEMU firmware descriptors everywhere so that
they'll be usable with the libvirt-configuration firmwares field I
proposed in bug#77110.  Perhaps we can apply that one first to make this
slightly easier (it provided some `ovmf-aux-file' procedure you'll
probably want to use to locate the a locally kept .json QEMU firmware
descriptor file).

-- 
Thanks,
Maxim