| Message ID | 20200228163840.23585-26-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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) Alex, Thomas, can we enable AVOCADO_ALLOW_UNTRUSTED_CODE on GitLab to avoid such regressions? Regards, Phil.
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) > > 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
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> >> >> 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 > >
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); --- > >>> >>> 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 >> >> >
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 > >> > >> > > >
On 10/23/20 5:43 PM, Igor Mammedov wrote: > On Mon, 19 Oct 2020 11:43:13 +0200 > Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >>>>> 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. Hmm this is the 'ARM kernel loader' API in "arm/boot.h": struct arm_boot_info { uint64_t ram_size; const char *kernel_filename; const char *kernel_cmdline; const char *initrd_filename; const char *dtb_filename; and: void (*write_secondary_boot)(ARMCPU *cpu, const struct arm_boot_info *info); void (*secondary_cpu_reset_hook)(ARMCPU *cpu, const struct arm_boot_info *info); Are you saying arm_boot_info should hold a pointer to MachineState* instead of duplicating?
On Fri, 23 Oct 2020 19:39:16 +0200 Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > On 10/23/20 5:43 PM, Igor Mammedov wrote: > > On Mon, 19 Oct 2020 11:43:13 +0200 > > Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > >>>>> 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. > > Hmm this is the 'ARM kernel loader' API in "arm/boot.h": > > struct arm_boot_info { > uint64_t ram_size; > const char *kernel_filename; > const char *kernel_cmdline; > const char *initrd_filename; > const char *dtb_filename; > > and: > > void (*write_secondary_boot)(ARMCPU *cpu, > const struct arm_boot_info *info); > void (*secondary_cpu_reset_hook)(ARMCPU *cpu, > const struct arm_boot_info *info); > > Are you saying arm_boot_info should hold a pointer to MachineState* > instead of duplicating? yep, some parts of it (fdt related) already use MachineState* so it's complete rewrite. The same probably applies to the fields you've just quoted. >
On Fri, 23 Oct 2020 at 20:04, Igor Mammedov <imammedo@redhat.com> wrote: > > On Fri, 23 Oct 2020 19:39:16 +0200 > Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > Are you saying arm_boot_info should hold a pointer to MachineState* > > instead of duplicating? > > yep, some parts of it (fdt related) already use MachineState* so it's > complete rewrite. The same probably applies to the fields you've just > quoted. Hmm, maybe, maybe not. The original design idea here was that the boot loader code took a structure defining only the things that the bootloader needed to know. It doesn't really need to know about all the stuff that's in MachineState, which is the state structure for the machine. thanks -- PMM
On Sun, 25 Oct 2020 17:03:43 +0000 Peter Maydell <peter.maydell@linaro.org> wrote: > On Fri, 23 Oct 2020 at 20:04, Igor Mammedov <imammedo@redhat.com> wrote: > > > > On Fri, 23 Oct 2020 19:39:16 +0200 > > Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > > Are you saying arm_boot_info should hold a pointer to MachineState* > > > instead of duplicating? > > > > yep, some parts of it (fdt related) already use MachineState* so it's > > complete rewrite. The same probably applies to the fields you've just > > quoted. > > Hmm, maybe, maybe not. The original design idea here was that > the boot loader code took a structure defining only the things > that the bootloader needed to know. It doesn't really need to > know about all the stuff that's in MachineState, which is > the state structure for the machine. Yep It doesn't need all data the MachineState contains, but then we end up with this kind of bugs which could be avoided if duplication were not there. And some of the fields in MachineState are pure bootloader data. > > thanks > -- PMM >
On Mon, 26 Oct 2020 at 13:37, Igor Mammedov <imammedo@redhat.com> wrote: > Peter Maydell <peter.maydell@linaro.org> wrote: > > Hmm, maybe, maybe not. The original design idea here was that > > the boot loader code took a structure defining only the things > > that the bootloader needed to know. It doesn't really need to > > know about all the stuff that's in MachineState, which is > > the state structure for the machine. > > Yep It doesn't need all data the MachineState contains, but then we end up > with this kind of bugs which could be avoided if duplication were not there. > And some of the fields in MachineState are pure bootloader data. I notice we already have arm_load_kernel() take a MachineState* and fill in the info->kernel_filename etc from the MachineState fields. I suppose we could do the same for a few more fields. I'm not very fond of the way that function takes the MachineState*, though. I think it would be nicer if the MachineState had a separate sub-struct which was "this is the stuff that's just data for the bootloader" and passed that, rather than throwing the entire state struct pointer around. thanks -- PMM
On Mon, 26 Oct 2020 14:26:59 +0000 Peter Maydell <peter.maydell@linaro.org> wrote: > On Mon, 26 Oct 2020 at 13:37, Igor Mammedov <imammedo@redhat.com> wrote: > > Peter Maydell <peter.maydell@linaro.org> wrote: > > > Hmm, maybe, maybe not. The original design idea here was that > > > the boot loader code took a structure defining only the things > > > that the bootloader needed to know. It doesn't really need to > > > know about all the stuff that's in MachineState, which is > > > the state structure for the machine. > > > > Yep It doesn't need all data the MachineState contains, but then we end up > > with this kind of bugs which could be avoided if duplication were not there. > > And some of the fields in MachineState are pure bootloader data. > > I notice we already have arm_load_kernel() take a MachineState* > and fill in the info->kernel_filename etc from the MachineState > fields. I suppose we could do the same for a few more fields. > I'm not very fond of the way that function takes the MachineState*, > though. I think it would be nicer if the MachineState had a > separate sub-struct which was "this is the stuff that's just > data for the bootloader" and passed that, rather than throwing > the entire state struct pointer around. this should work for the most of copied fields but not for all, (ram_size in this case). > > thanks > -- PMM >
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()