Re: [PATCH 1/9] grub-shell: Support riscv64-efi

[Glenn Washburn]

On Wed, 11 Sep 2024 12:37:27 +0300
Vladimir Serbinenko <phcoder@gmail.com> wrote:

> Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
> ---
>  tests/util/grub-shell.in | 43 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
> index ae5f711fe..04405a789 100644
> --- a/tests/util/grub-shell.in
> +++ b/tests/util/grub-shell.in
> @@ -33,6 +33,7 @@ export PATH
>  
>  trim=0
>  trim_head=664cbea8-132f-4770-8aa4-1696d59ac35c
> +trim_tail=a3f2a1b5-74fb-4c76-959b-4cf5ef5578b3

I think it makes sense to put this and the last changeset into a
separate preceding patch because logically its not quite related,
perhaps noting that its to prepare for a subsequent patch. If not, at
least something should be noted in the commit message.

>  
>  # Usage: usage
>  # Print the usage.
> @@ -303,6 +304,41 @@ case 
> "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
>       disk="device virtio-blk-device,drive=hd1 -drive if=none,id=hd1,file="
>       serial_port=efi0
>       ;;
> +     riscv64-efi)
> +     qemu=qemu-system-riscv64
> +     boot=hd
> +     console=console
> +     trim=1
> +     pflash_code=${srcdir}/RISCV_VIRT_CODE.fd
> +     pflash_vars=${srcdir}/RISCV_VIRT_VARS.fd
> +     if [ -f "$pflash_code" ]; then
> +         qemuopts="-drive 
> if=pflash,format=raw,unit=0,readonly=on,file=$pflash_code $qemuopts"
> +         if [ -f "$pflash_vars" ]; then
> +             qemuopts="-drive 
> if=pflash,format=raw,unit=1,snapshot=on,file=$pflash_vars $qemuopts"
> +         fi
> +     elif [ -f /usr/share/qemu-efi-riscv64/RISCV_VIRT_CODE.fd ]; then
> +         pflash_code=/usr/share/qemu-efi-riscv64/RISCV_VIRT_CODE.fd
> +         pflash_vars=/usr/share/qemu-efi-riscv64/RISCV_VIRT_VARS.fd
> +         qemuopts="-drive 
> if=pflash,format=raw,unit=0,readonly=on,file=$pflash_code $qemuopts"
> +         qemuopts="-drive 
> if=pflash,format=raw,unit=1,snapshot=on,file=$pflash_vars $qemuopts"
> +     elif [ -f /usr/share/edk2/riscv/RISCV_VIRT_CODE.fd ]; then
> +         # Fedora ships images that are under 32M and we need to fill them 
> to 32M

Tests are only supported under Debian. I don't mind supporting other
distros, but currently its not for other targets with respect to
finding the firmware. And if we are going to do it, it should be done
in a more target generic manner. This knowledge about how to run the
tests on Fedora codified here seems valuable though, so I'm not opposed
to this change and we can do what I suggest at a future date. But its
not ideal.

> +         cp /usr/share/edk2/riscv/RISCV_VIRT_CODE.fd "$work_directory"
> +         cp /usr/share/edk2/riscv/RISCV_VIRT_VARS.fd "$work_directory"
> +         pflash_code="$work_directory"/RISCV_VIRT_CODE.fd
> +         pflash_vars="$work_directory"/RISCV_VIRT_VARS.fd
> +         truncate -s 32M $pflash_code
> +         truncate -s 32M $pflash_vars            
> +         qemuopts="-drive 
> if=pflash,format=raw,unit=0,readonly=on,file=$pflash_code $qemuopts"
> +         qemuopts="-drive 
> if=pflash,format=raw,unit=1,snapshot=on,file=$pflash_vars $qemuopts"          
>  
> +     else
> +         echo "Firmware not found, please install either the appropriate 
> Debian/Fedora package or an appropriately named copy in the source 
> directory." >&2
> +         exit 1
> +     fi
> +     qemuopts="-machine virt $qemuopts"
> +     disk="device virtio-blk-device,drive=hd1 -drive if=none,id=hd1,file="
> +     serial_port=efi0
> +     ;;
>      loongarch64-efi)
>       qemu=qemu-system-loongarch64
>       boot=hd
> @@ -498,6 +534,9 @@ source "\$prefix/testcase.cfg"
>  # Stop serial output to suppress "ACPI shutdown failed" error.
>  EOF
>  # Attempt to switch to console on i386-ieee1275 causes "screen not found" 
> message

This comment now does not go with the originally intended code and I
doubt it should remain as is for the added change below. Also a comment
should be added describing why the added code to trim to the end of the
output is needed.

Glenn

> +if [ x"${grub_modinfo_target_cpu}-${grub_modinfo_platform}" = xriscv64-efi 
> ]; then
> +    echo "echo $trim_tail" >>${cfgfile}
> +fi
>  if [ x$console != x ] && [ 
> x"${grub_modinfo_target_cpu}-${grub_modinfo_platform}" != xi386-ieee1275 ]; 
> then
>      echo "terminal_output $console" >>${cfgfile}
>  fi
> @@ -547,6 +586,8 @@ if [ x$boot = xhd ]; then
>       device="device virtio-blk-device,drive=hd0 -drive if=none,id=hd0,file="
>      elif [ "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" = 
> loongarch64-efi ]; then
>       device="device virtio-blk-pci,drive=grubdisk -drive 
> if=none,id=grubdisk,file="
> +    elif [ "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" = 
> riscv64-efi ]; then
> +     device="device virtio-blk-device,drive=hd0,bootindex=1 -drive 
> if=none,id=hd0,file="
>      elif [ "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" = mips-arc 
> ]; then
>       device="hdb "
>      else
> @@ -612,7 +653,7 @@ fi
>  do_trim ()
>  {
>      if [ $trim = 1 ] || [ $trim = 2 ]; then
> -     awk '{ if (have_head == 1) print $0; } /^'"$trim_head"'/ { have_head=1; 
> }'
> +     awk ' /^'"$trim_tail"'/ { have_head=0; } { if (have_head == 1) print 
> $0; } /^'"$trim_head"'/ { have_head=1; }'
>      else
>       cat
>      fi