Re: [PATCH] tests/functional: Add a ppc64 mac99 test

[Philippe Mathieu-Daudé]

On 28/1/25 19:20, Thomas Huth wrote:
> On 28/01/2025 19.07, Cédric Le Goater wrote:
> > The test sequence boots from disk a mac99 machine in 64-bit mode, in
> > which case the CPU is a PPC 970.
> >
> > The buildroot rootfs is built with config :
> >
> > BR2_powerpc64=y
> > BR2_powerpc_970=y
> >
> > and the kernel with the g5 deconfig.
> >
> > Signed-off-by: Cédric Le Goater <clg@redhat.com>
> > ---
> >   MAINTAINERS                          |  1 +
> >   tests/functional/meson.build         |  2 ++
> >   tests/functional/test_ppc64_mac99.py | 45 ++++++++++++++++++++++++++++
> >   3 files changed, 48 insertions(+)
> >   create mode 100644 tests/functional/test_ppc64_mac99.py
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 3a2291d17d7e..ebf249173f00 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1446,6 +1446,7 @@ F: include/hw/pci-host/uninorth.h
> >   F: include/hw/input/adb*
> >   F: pc-bios/qemu_vga.ndrv
> >   F: tests/functional/test_ppc_mac.py
> > +F: tests/functional/test_ppc64_mac99.py
> >   Old World (g3beige)
> >   M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> > index 01a87b03e553..821a0a7c49ac 100644
> > --- a/tests/functional/meson.build
> > +++ b/tests/functional/meson.build
> > @@ -41,6 +41,7 @@ test_timeouts = {
> >     'ppc64_powernv' : 480,
> >     'ppc64_pseries' : 480,
> >     'ppc64_tuxrun' : 420,
> > +  'ppc64_mac99' : 120,
> >     'riscv64_tuxrun' : 120,
> >     's390x_ccw_virtio' : 420,
> >     'sh4_tuxrun' : 240,
> > @@ -181,6 +182,7 @@ tests_ppc64_system_thorough = [
> >     'ppc64_powernv',
> >     'ppc64_pseries',
> >     'ppc64_tuxrun',
> > +  'ppc64_mac99',
> >   ]
> >   tests_riscv32_system_quick = [
> > diff --git a/tests/functional/test_ppc64_mac99.py b/tests/functional/ 
> > test_ppc64_mac99.py
> > new file mode 100644
> > index 000000000000..ba582d267dae
> > --- /dev/null
> > +++ b/tests/functional/test_ppc64_mac99.py
> > @@ -0,0 +1,45 @@
> > +#!/usr/bin/env python3
> > +#
> > +# Functional test that boots a mac99 machine with a PPC970 CPU
> > +#
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +from qemu_test import LinuxKernelTest, Asset
> > +from qemu_test import exec_command_and_wait_for_pattern
> > +
> > +class mac99Test(LinuxKernelTest):
> > +
> > +    ASSET_BR2_MAC99_LINUX = Asset(
> > +        'https://github.com/legoater/qemu-ppc-boot/raw/refs/heads/ 
> > main/buildroot/qemu_ppc64_mac99-2023.11-8-gdcd9f0f6eb-20240105/vmlinux',
> > +        
> > 'd59307437e4365f2cced0bbd1b04949f7397b282ef349b7cafd894d74aadfbff')
> > +
> > +    ASSET_BR2_MAC99_ROOTFS = Asset(
> > +        'https://github.com/legoater/qemu-ppc-boot/raw/refs/heads/ 
> > main//buildroot/qemu_ppc64_mac99-2023.11-8-gdcd9f0f6eb-20240105/ 
> > rootfs.ext2',
> > +        
> > 'bbd5fd8af62f580bc4e585f326fe584e22856572633a8333178ea6d4ed4955a4')
> > +
> > +    def test_ppc64_mac99_buildroot(self):
> > +        self.require_netdev('user')
> > +
> > +        linux_path = self.ASSET_BR2_MAC99_LINUX.fetch()
> > +        rootfs_path = self.ASSET_BR2_MAC99_ROOTFS.fetch()
> > +
> > +        self.set_machine('mac99')
>
> Nit: Move the set_machine to the top of the function, so that the assets 
> don't have to be fetched in case the machine is not available.

Nice to know, this is not obvious. Should we check for unset machine
in fetch()? Or add fetch_for_machine()... Nitpicking yet more :p

> With that nit fixed:
> Reviewed-by: Thomas Huth <thuth@redhat.com>
>
>
> > +        self.vm.set_console()
> > +
> > +        self.vm.add_args('-kernel', linux_path,
> > +                         '-append', 'root=/dev/sda',
> > +                         '-drive', f'file={rootfs_path},format=raw',
> > +                         '-net', 'nic,model=sungem', '-net', 'user',
> > +                         '-snapshot', '-nographic')
>
> I just also noticed that we already have "-display none" in python/qemu/ 
> machine/machine.py, so you likely don't need the -nographic here (but 
> looks like we've got that wrong in a bunch of tests already).

Maybe warn in QEMUMachine::_pre_launch() if "-display none" && '-nographic'?

> > +        self.vm.launch()