Re: [PULL 25/33] tests/acceptance: Add a test for the N800 and N810 arm machines

[Igor Mammedov]

On Mon, 19 Oct 2020 11:43:13 +0200
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> On 10/19/20 11:30 AM, Philippe Mathieu-Daudé wrote:
> > On 10/19/20 8:31 AM, Thomas Huth wrote:  
> >> On 17/10/2020 19.51, Philippe Mathieu-Daudé wrote:  
> >>> Hi Peter, Igor, Thomas,
> >>>
> >>> On 2/28/20 5:38 PM, Peter Maydell wrote:  
> >>>> From: Thomas Huth <thuth@redhat.com>
> >>>>
> >>>> Old kernels from the Meego project can be used to check that Linux
> >>>> is at least starting on these machines.
> >>>>
> >>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >>>> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> >>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >>>> Message-id: 20200225172501.29609-2-philmd@redhat.com
> >>>> Message-Id: <20200129131920.22302-1-thuth@redhat.com>
> >>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >>>> ---
> >>>>    MAINTAINERS                          |  1 +
> >>>>    tests/acceptance/machine_arm_n8x0.py | 49 
> >>>> ++++++++++++++++++++++++++++
> >>>>    2 files changed, 50 insertions(+)
> >>>>    create mode 100644 tests/acceptance/machine_arm_n8x0.py
> >>>>
> >>>> diff --git a/MAINTAINERS b/MAINTAINERS
> >>>> index b66c46dcb9f..264374adbe8 100644
> >>>> --- a/MAINTAINERS
> >>>> +++ b/MAINTAINERS
> >>>> @@ -686,6 +686,7 @@ F: hw/rtc/twl92230.c
> >>>>    F: include/hw/display/blizzard.h
> >>>>    F: include/hw/input/tsc2xxx.h
> >>>>    F: include/hw/misc/cbus.h
> >>>> +F: tests/acceptance/machine_arm_n8x0.py
> >>>>      Palm
> >>>>    M: Andrzej Zaborowski <balrogg@gmail.com>
> >>>> diff --git a/tests/acceptance/machine_arm_n8x0.py
> >>>> b/tests/acceptance/machine_arm_n8x0.py
> >>>> new file mode 100644
> >>>> index 00000000000..e5741f2d8d1
> >>>> --- /dev/null
> >>>> +++ b/tests/acceptance/machine_arm_n8x0.py
> >>>> @@ -0,0 +1,49 @@
> >>>> +# Functional test that boots a Linux kernel and checks the console
> >>>> +#
> >>>> +# Copyright (c) 2020 Red Hat, Inc.
> >>>> +#
> >>>> +# Author:
> >>>> +#  Thomas Huth <thuth@redhat.com>
> >>>> +#
> >>>> +# This work is licensed under the terms of the GNU GPL, version 2 or
> >>>> +# later.  See the COPYING file in the top-level directory.
> >>>> +
> >>>> +import os
> >>>> +
> >>>> +from avocado import skipUnless
> >>>> +from avocado_qemu import Test
> >>>> +from avocado_qemu import wait_for_console_pattern
> >>>> +
> >>>> +class N8x0Machine(Test):
> >>>> +    """Boots the Linux kernel and checks that the console is 
> >>>> operational"""
> >>>> +
> >>>> +    timeout = 90
> >>>> +
> >>>> +    def __do_test_n8x0(self):
> >>>> +        kernel_url = ('http://stskeeps.subnetmask.net/meego-n8x0/'
> >>>> +                      'meego-arm-n8x0-1.0.80.20100712.1431-'
> >>>> +                      'vmlinuz-2.6.35~rc4-129.1-n8x0')
> >>>> +        kernel_hash = 'e9d5ab8d7548923a0061b6fbf601465e479ed269'
> >>>> +        kernel_path = self.fetch_asset(kernel_url, 
> >>>> asset_hash=kernel_hash)
> >>>> +
> >>>> +        self.vm.set_console(console_index=1)
> >>>> +        self.vm.add_args('-kernel', kernel_path,
> >>>> +                         '-append', 'printk.time=0 console=ttyS1')
> >>>> +        self.vm.launch()
> >>>> +        wait_for_console_pattern(self, 'TSC2005 driver initializing')
> >>>> +
> >>>> +    @skipUnless(os.getenv('AVOCADO_ALLOW_UNTRUSTED_CODE'), 
> >>>> 'untrusted code')
> >>>> +    def test_n800(self):
> >>>> +        """
> >>>> +        :avocado: tags=arch:arm
> >>>> +        :avocado: tags=machine:n800
> >>>> +        """
> >>>> +        self.__do_test_n8x0()
> >>>> +
> >>>> +    @skipUnless(os.getenv('AVOCADO_ALLOW_UNTRUSTED_CODE'), 
> >>>> 'untrusted code')
> >>>> +    def test_n810(self):
> >>>> +        """
> >>>> +        :avocado: tags=arch:arm
> >>>> +        :avocado: tags=machine:n810
> >>>> +        """
> >>>> +        self.__do_test_n8x0()
> >>>>  
> >>>
> >>> FYI this test is failing:
> >>>
> >>> qemu-system-arm: kernel 'meego-arm-n8x0-1.0.80.20100712.1431-vml
> >>> inuz-2.6.35~rc4-129.1-n8x0' is too large to fit in RAM (kernel size 
> >>> 1964608,
> >>> RAM size 0)  
> > 
> > FWIW:
> > 
> > 7998beb9c2e280f0b7424223747941f106e2e854 is the first bad commit
> > commit 7998beb9c2e280f0b7424223747941f106e2e854
> > Author: Igor Mammedov <imammedo@redhat.com>
> > Date:   Wed Feb 19 11:08:59 2020 -0500
> > 
> >      arm/nseries: use memdev for RAM
> > 
> >      memory_region_allocate_system_memory() API is going away, so
> >      replace it with memdev allocated MemoryRegion. The later is
> >      initialized by generic code, so board only needs to opt in
> >      to memdev scheme by providing
> >        MachineClass::default_ram_id
> >      and using MachineState::ram instead of manually initializing
> >      RAM memory region.
> > 
> >      PS:
> >       while at it add check for user supplied RAM size and error
> >       out if it mismatches board expected value.
> > 
> >      Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >      Reviewed-by: Andrew Jones <drjones@redhat.com>
> >      Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >      Message-Id: <20200219160953.13771-26-imammedo@redhat.com>  
> 
> This fixes the issue:
> 
> -- >8 --  
> diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
> index e48092ca047..76fd7fe9854 100644
> --- a/hw/arm/nseries.c
> +++ b/hw/arm/nseries.c
> @@ -1318,6 +1318,7 @@ static void n8x0_init(MachineState *machine,
>           g_free(sz);
>           exit(EXIT_FAILURE);
>       }
> +    binfo->ram_size = machine->ram_size;
> 
>       memory_region_add_subregion(get_system_memory(), OMAP2_Q2_BASE,
>                                   machine->ram);

we really should replace binfo->ram_size with machine->ram_size to avoid
duplicating the same data, but as a quick fix this should fix issue.

Acked-by: Igor Mammedov <imammedo@redhat.com>


> ---
> 
> >   
> >>>
> >>> Alex, Thomas, can we enable AVOCADO_ALLOW_UNTRUSTED_CODE on GitLab
> >>> to avoid such regressions?  
> >>
> >> Yes, please, if you've got some spare minutes to work on such a patch, 
> >> that
> >> would be great! ... I once already wanted to send such a patch, but IIRC
> >> there were some other ALLOW_UNTRUSTED_CODE tests failing at that poing in
> >> time, and I never got around to fix them...
> >>
> >>   Thomas
> >>
> >>  
> >   
>