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

[BALATON Zoltan]

On Tue, 28 Jan 2025, Thomas Huth wrote:
> On 28/01/2025 19.35, Cédric Le Goater wrote:
> > On 1/28/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.
> > >
> > > 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).
> >
> > I think we need -nographic else OpenBIOS crashes.  I will check.
>
> Oh, you're right, OpenBIOS hangs with "milliseconds isn't unique" when

The "milliseconds isn't unique" is something you always get, this has 
something to do with how it's implemented in OpenBIOS but it was deemed 
harmless. (I don't remember the details but it's also unrelated to this.)

Regards,
BALATON Zoltan

> running "qemu-system-ppc64 -M mac99 -display none -serial stdio" ... that's 
> weird... so you need to keep the -nographic here, indeed!
>
> Thomas