[PATCH for 7.0 v1 0/8] misc testing, i386, docs, gitdm, gitlab

[PATCH for 7.0 v1 0/8] misc testing, i386, docs, gitdm, gitlab

[Alex Bennée]

Hi,

As per usual here are the collection of random fixes and tweaks as we
go through the release process. Most of these patches have been posted
individually before although the semihosting and gitlab patches are
new.

The following need review:

 - gitlab: include new aarch32 job in custom-runners
 - semihosting: clean up handling of expanded argv
 - tests/avocado: update aarch64_virt test to exercise -cpu max
 
Alex Bennée (7):
  tests/avocado: update aarch64_virt test to exercise -cpu max
  target/i386: force maximum rounding precision for fildl[l]
  tests/tcg: drop -cpu max from s390x sha512-mvx invocation
  mailmap/gitdm: more fixes for bad tags and authors
  docs/devel: try and impose some organisation
  semihosting: clean up handling of expanded argv
  gitlab: include new aarch32 job in custom-runners

Thomas Huth (1):
  tests/Makefile.include: Let "make clean" remove the TCG tests, too

 docs/devel/index-api.rst            | 15 ++++++++++
 docs/devel/index-build.rst          | 19 ++++++++++++
 docs/devel/index-internals.rst      | 21 +++++++++++++
 docs/devel/index-process.rst        | 17 +++++++++++
 docs/devel/index-tcg.rst            | 16 ++++++++++
 docs/devel/index.rst                | 46 +++++------------------------
 semihosting/config.c                |  6 ++--
 target/i386/tcg/fpu_helper.c        | 13 ++++++++
 .gitlab-ci.d/custom-runners.yml     |  1 +
 .mailmap                            |  6 ++++
 contrib/gitdm/aliases               |  4 ++-
 tests/Makefile.include              |  2 +-
 tests/avocado/boot_linux_console.py | 22 +++++++++-----
 tests/tcg/s390x/Makefile.target     |  2 --
 14 files changed, 136 insertions(+), 54 deletions(-)
 create mode 100644 docs/devel/index-api.rst
 create mode 100644 docs/devel/index-build.rst
 create mode 100644 docs/devel/index-internals.rst
 create mode 100644 docs/devel/index-process.rst
 create mode 100644 docs/devel/index-tcg.rst

-- 
2.30.2

[PATCH v1 1/8] tests/Makefile.include: Let "make clean" remove the TCG tests, too

[Alex Bennée]

From: Thomas Huth <thuth@redhat.com>

"make clean" should clear all binaries that have been built, but so
far it left the TCG tests still in place. Let's make sure that they
are now removed, too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20220301085900.1443232-1-thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index e7153c8e91..7a932caf91 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -162,6 +162,6 @@ check-build: run-ninja
 check-clean:
 	rm -rf $(TESTS_VENV_DIR) $(TESTS_RESULTS_DIR)
 
-clean: check-clean
+clean: check-clean clean-tcg
 
 endif
-- 
2.30.2

[PATCH v1 2/8] tests/avocado: update aarch64_virt test to exercise -cpu max

[Alex Bennée]

The Fedora 29 kernel is quite old and importantly fails when running
in LPA2 scenarios. As it's not really exercising much of the CPU space
replace it with a custom 5.16.12 kernel with all the architecture
options turned on. There is a minimal buildroot initramfs included in
the kernel which has a few tools for stress testing the memory
subsystem. The userspace also targets the Neoverse N1 processor so
will fail without additional v8.x+ features.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20220307172249.3125779-1-alex.bennee@linaro.org>
---
 tests/avocado/boot_linux_console.py | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/tests/avocado/boot_linux_console.py b/tests/avocado/boot_linux_console.py
index 9c618d4809..ca3a40e91d 100644
--- a/tests/avocado/boot_linux_console.py
+++ b/tests/avocado/boot_linux_console.py
@@ -12,6 +12,7 @@
 import lzma
 import gzip
 import shutil
+import time
 
 from avocado import skip
 from avocado import skipUnless
@@ -330,25 +331,30 @@ def test_aarch64_virt(self):
         :avocado: tags=arch:aarch64
         :avocado: tags=machine:virt
         :avocado: tags=accel:tcg
-        :avocado: tags=cpu:cortex-a53
+        :avocado: tags=cpu:max
         """
-        kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
-                      '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
-                      '/vmlinuz')
-        kernel_hash = '8c73e469fc6ea06a58dc83a628fc695b693b8493'
+        kernel_url = ('https://fileserver.linaro.org/s/'
+                      'z6B2ARM7DQT3HWN/download')
+
+        kernel_hash = 'ed11daab50c151dde0e1e9c9cb8b2d9bd3215347'
         kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
         self.vm.set_console()
         kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
                                'console=ttyAMA0')
         self.require_accelerator("tcg")
-        self.vm.add_args('-cpu', 'cortex-a53',
+        self.vm.add_args('-cpu', 'max,pauth-impdef=on',
                          '-accel', 'tcg',
                          '-kernel', kernel_path,
                          '-append', kernel_command_line)
         self.vm.launch()
-        console_pattern = 'Kernel command line: %s' % kernel_command_line
-        self.wait_for_console_pattern(console_pattern)
+        self.wait_for_console_pattern('Welcome to Buildroot')
+        time.sleep(0.1)
+        exec_command(self, 'root')
+        time.sleep(0.1)
+        exec_command(self, 'cat /proc/self/maps')
+        time.sleep(0.1)
+
 
     def test_aarch64_xlnx_versal_virt(self):
         """
-- 
2.30.2

[PATCH v1 3/8] target/i386: force maximum rounding precision for fildl[l]

[Alex Bennée]

The instruction description says "It is loaded without rounding
errors." which implies we should have the widest rounding mode
possible.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/888
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/i386/tcg/fpu_helper.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index cdd8e9f947..ebf5e73df9 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -237,24 +237,37 @@ void helper_fldl_ST0(CPUX86State *env, uint64_t val)
     merge_exception_flags(env, old_flags);
 }
 
+static FloatX80RoundPrec tmp_maximise_precision(float_status *st)
+{
+    FloatX80RoundPrec old = get_floatx80_rounding_precision(st);
+    set_floatx80_rounding_precision(floatx80_precision_x, st);
+    return old;
+}
+
 void helper_fildl_ST0(CPUX86State *env, int32_t val)
 {
     int new_fpstt;
+    FloatX80RoundPrec old = tmp_maximise_precision(&env->fp_status);
 
     new_fpstt = (env->fpstt - 1) & 7;
     env->fpregs[new_fpstt].d = int32_to_floatx80(val, &env->fp_status);
     env->fpstt = new_fpstt;
     env->fptags[new_fpstt] = 0; /* validate stack entry */
+
+    set_floatx80_rounding_precision(old, &env->fp_status);
 }
 
 void helper_fildll_ST0(CPUX86State *env, int64_t val)
 {
     int new_fpstt;
+    FloatX80RoundPrec old = tmp_maximise_precision(&env->fp_status);
 
     new_fpstt = (env->fpstt - 1) & 7;
     env->fpregs[new_fpstt].d = int64_to_floatx80(val, &env->fp_status);
     env->fpstt = new_fpstt;
     env->fptags[new_fpstt] = 0; /* validate stack entry */
+
+    set_floatx80_rounding_precision(old, &env->fp_status);
 }
 
 uint32_t helper_fsts_ST0(CPUX86State *env)
-- 
2.30.2

[PATCH v1 4/8] tests/tcg: drop -cpu max from s390x sha512-mvx invocation

[Alex Bennée]

With -cpu max we get a warning:

  qemu-s390x: warning: 'msa5-base' requires 'kimd-sha-512'.

But dropping the -cpu max and it still runs fine.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
---
 tests/tcg/s390x/Makefile.target | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 257c568c58..7aa502a557 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -34,6 +34,4 @@ sha512-mvx: CFLAGS=-march=z13 -mvx -O3
 sha512-mvx: sha512.c
 	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
 
-run-sha512-mvx: QEMU_OPTS+=-cpu max
-
 TESTS+=sha512-mvx
-- 
2.30.2

[PATCH v1 5/8] mailmap/gitdm: more fixes for bad tags and authors

[Alex Bennée]

I was running some historical tags for the last 10 years and got the
following warnings:

  git log --use-mailmap --numstat --since "June 2010" | ~/src/gitdm.git/gitdm -n -l 5
  alarson@ddci.com is an author name, probably not what you want
  bad utf-8 ('utf-8' codec can't decode byte 0xe4 in position 552: invalid continuation byte) in patchm skipping
  bad utf-8 ('utf-8' codec can't decode byte 0xe4 in position 342: invalid continuation byte) in patchm skipping
  michael@ozlabs.org  is an author name, probably not what you want
  Oops...funky email nicta.com.au
  bad utf-8 ('utf-8' codec can't decode byte 0xe9 in position 232: invalid continuation byte) in patchm skipping
  Oops...funky email andreas.faerber
  Grabbing changesets...done
  Processed 76422 csets from 1902 developers

The following fixes try and alleviate that although I still get a
warning for Aaron which I think is from 9743cd5736.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Acked-by: Aaron Larson <alarson@ddci.com>
Cc: Andreas Färber <andreas.faerber@web.de>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Michael Ellerman <michael@ellerman.id.au>

---
v2
  - added Aaron's acked by
  - dropped Peter Chubb from CC (bouncing emails)
  - used Michaels preferred coding email
  - dropped the identity map in aliases (I think gitdm still gets reported by correct)
---
 .mailmap              | 6 ++++++
 contrib/gitdm/aliases | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 5113f55b3a..09dcd8c216 100644
--- a/.mailmap
+++ b/.mailmap
@@ -28,7 +28,11 @@ Thiemo Seufer <ths@networkno.de> ths <ths@c046a42c-6fe2-441c-8c8c-71466251a162>
 malc <av1474@comtv.ru> malc <malc@c046a42c-6fe2-441c-8c8c-71466251a162>
 
 # Corrupted Author fields
+Aaron Larson <alarson@ddci.com> alarson@ddci.com
+Andreas Färber <andreas.faerber@web.de> Andreas Färber <andreas.faerber>
+Jason Wang <jasowang@redhat.com> Jason Wang <jasowang>
 Marek Dolata <mkdolata@us.ibm.com> mkdolata@us.ibm.com <mkdolata@us.ibm.com>
+Michael Ellerman <mpe@ellerman.id.au> michael@ozlabs.org <michael@ozlabs.org>
 Nick Hudson <hnick@vmware.com> hnick@vmware.com <hnick@vmware.com>
 
 # There is also a:
@@ -70,6 +74,7 @@ Yongbok Kim <yongbok.kim@mips.com> <yongbok.kim@imgtec.com>
 # Also list preferred name forms where people have changed their
 # git author config, or had utf8/latin1 encoding issues.
 Aaron Lindsay <aaron@os.amperecomputing.com>
+Aaron Larson <alarson@ddci.com>
 Alexey Gerasimenko <x1917x@gmail.com>
 Alex Chen <alex.chen@huawei.com>
 Alex Ivanov <void@aleksoft.net>
@@ -144,6 +149,7 @@ Pan Nengyuan <pannengyuan@huawei.com>
 Pavel Dovgaluk <dovgaluk@ispras.ru>
 Pavel Dovgaluk <pavel.dovgaluk@gmail.com>
 Pavel Dovgaluk <Pavel.Dovgaluk@ispras.ru>
+Peter Chubb <peter.chubb@nicta.com.au>
 Peter Crosthwaite <crosthwaite.peter@gmail.com>
 Peter Crosthwaite <peter.crosthwaite@petalogix.com>
 Peter Crosthwaite <peter.crosthwaite@xilinx.com>
diff --git a/contrib/gitdm/aliases b/contrib/gitdm/aliases
index 4792413ce7..e26b00a71d 100644
--- a/contrib/gitdm/aliases
+++ b/contrib/gitdm/aliases
@@ -34,8 +34,10 @@ malc@c046a42c-6fe2-441c-8c8c-71466251a162 av1474@comtv.ru
 # canonical emails
 liq3ea@163.com liq3ea@gmail.com
 
-# some broken tags
+# some broken DCO tags
 yuval.shaia.ml.gmail.com yuval.shaia.ml@gmail.com
+jasowang jasowang@redhat.com
+nicta.com.au peter.chubb@nicta.com.au
 
 # There is also a:
 #    (no author) <(no author)@c046a42c-6fe2-441c-8c8c-71466251a162>
-- 
2.30.2

[PATCH v1 6/8] docs/devel: try and impose some organisation

[Alex Bennée]

We have a growing set of developer docs but the index is currently in
order of when stuff was added. Try and make things a bit easier to
find my adding sub indexes and organising into themes.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>

---
v2
  - fix whitespace issues
---
 docs/devel/index-api.rst       | 15 +++++++++++
 docs/devel/index-build.rst     | 19 ++++++++++++++
 docs/devel/index-internals.rst | 21 ++++++++++++++++
 docs/devel/index-process.rst   | 17 +++++++++++++
 docs/devel/index-tcg.rst       | 16 ++++++++++++
 docs/devel/index.rst           | 46 ++++++----------------------------
 6 files changed, 95 insertions(+), 39 deletions(-)
 create mode 100644 docs/devel/index-api.rst
 create mode 100644 docs/devel/index-build.rst
 create mode 100644 docs/devel/index-internals.rst
 create mode 100644 docs/devel/index-process.rst
 create mode 100644 docs/devel/index-tcg.rst

diff --git a/docs/devel/index-api.rst b/docs/devel/index-api.rst
new file mode 100644
index 0000000000..b749240272
--- /dev/null
+++ b/docs/devel/index-api.rst
@@ -0,0 +1,15 @@
+Internal QEMU APIs
+------------------
+
+Details about how QEMU's various internal APIs. Most of these are
+generated from in-code annotations to function prototypes.
+
+.. toctree::
+   :maxdepth: 2
+   :includehidden:
+
+   bitops
+   loads-stores
+   memory
+   modules
+   ui
diff --git a/docs/devel/index-build.rst b/docs/devel/index-build.rst
new file mode 100644
index 0000000000..d96894f07c
--- /dev/null
+++ b/docs/devel/index-build.rst
@@ -0,0 +1,19 @@
+QEMU Build and Test System
+--------------------------
+
+Details about how QEMU's build system works and how it is integrated
+into our testing infrastructure. You will need to understand some of
+the basics if you are adding new files and targets to the build.
+
+.. toctree::
+   :maxdepth: 2
+   :includehidden:
+
+   build-system
+   kconfig
+   testing
+   qtest
+   ci
+   qapi-code-gen
+   fuzzing
+   control-flow-integrity
diff --git a/docs/devel/index-internals.rst b/docs/devel/index-internals.rst
new file mode 100644
index 0000000000..bb118b8eaf
--- /dev/null
+++ b/docs/devel/index-internals.rst
@@ -0,0 +1,21 @@
+Internal Subsystem Information
+------------------------------
+
+Details about QEMU's various subsystems including how to add features to them.
+
+.. toctree::
+   :maxdepth: 2
+   :includehidden:
+
+   qom
+   atomics
+   block-coroutine-wrapper
+   clocks
+   ebpf_rss
+   migration
+   multi-process
+   reset
+   s390-dasd-ipl
+   tracing
+   vfio-migration
+   writing-monitor-commands
diff --git a/docs/devel/index-process.rst b/docs/devel/index-process.rst
new file mode 100644
index 0000000000..314e9e94cc
--- /dev/null
+++ b/docs/devel/index-process.rst
@@ -0,0 +1,17 @@
+QEMU Community Processes
+------------------------
+
+Notes about how to interact with the community and how and where to submit patches.
+
+.. toctree::
+   :maxdepth: 2
+   :includehidden:
+
+   code-of-conduct
+   conflict-resolution
+   style
+   submitting-a-patch
+   trivial-patches
+   stable-process
+   submitting-a-pull-request
+   secure-coding-practices
diff --git a/docs/devel/index-tcg.rst b/docs/devel/index-tcg.rst
new file mode 100644
index 0000000000..3acbd95d36
--- /dev/null
+++ b/docs/devel/index-tcg.rst
@@ -0,0 +1,16 @@
+TCG Emulation
+-------------
+
+Details about QEMU's Tiny Code Generator and the infrastructure
+associated with emulation. You do not need to worry about this if you
+are only implementing things for HW accelerated hypervisors.
+
+.. toctree::
+   :maxdepth: 2
+   :includehidden:
+
+   tcg
+   decodetree
+   multi-thread-tcg
+   tcg-icount
+   tcg-plugins
diff --git a/docs/devel/index.rst b/docs/devel/index.rst
index afd937535e..a68207052d 100644
--- a/docs/devel/index.rst
+++ b/docs/devel/index.rst
@@ -7,44 +7,12 @@ You only need to read it if you are interested in reading or
 modifying QEMU's source code.
 
 .. toctree::
-   :maxdepth: 2
+   :maxdepth: 1
    :includehidden:
 
-   code-of-conduct
-   conflict-resolution
-   build-system
-   style
-   kconfig
-   testing
-   fuzzing
-   control-flow-integrity
-   loads-stores
-   memory
-   migration
-   atomics
-   stable-process
-   ci
-   qtest
-   decodetree
-   secure-coding-practices
-   tcg
-   tcg-icount
-   tracing
-   multi-thread-tcg
-   tcg-plugins
-   bitops
-   ui
-   reset
-   s390-dasd-ipl
-   clocks
-   qom
-   modules
-   block-coroutine-wrapper
-   multi-process
-   ebpf_rss
-   vfio-migration
-   qapi-code-gen
-   writing-monitor-commands
-   trivial-patches
-   submitting-a-patch
-   submitting-a-pull-request
+
+   index-process
+   index-build
+   index-api
+   index-internals
+   index-tcg
-- 
2.30.2

[PATCH v1 7/8] semihosting: clean up handling of expanded argv

[Alex Bennée]

Another cleanup patch tripped over the fact we weren't being careful
in our casting. Fix the casts, allow for a non-const and switch from
g_realloc to g_renew.

The whole semihosting argument handling could do with some tests
though.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 semihosting/config.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/semihosting/config.c b/semihosting/config.c
index 137171b717..50d82108e6 100644
--- a/semihosting/config.c
+++ b/semihosting/config.c
@@ -51,7 +51,7 @@ typedef struct SemihostingConfig {
     bool enabled;
     SemihostingTarget target;
     Chardev *chardev;
-    const char **argv;
+    char **argv;
     int argc;
     const char *cmdline; /* concatenated argv */
 } SemihostingConfig;
@@ -98,8 +98,8 @@ static int add_semihosting_arg(void *opaque,
     if (strcmp(name, "arg") == 0) {
         s->argc++;
         /* one extra element as g_strjoinv() expects NULL-terminated array */
-        s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
-        s->argv[s->argc - 1] = val;
+        s->argv = g_renew(char *, s->argv, s->argc + 1);
+        s->argv[s->argc - 1] = g_strdup(val);
         s->argv[s->argc] = NULL;
     }
     return 0;
-- 
2.30.2

[PATCH v1 8/8] gitlab: include new aarch32 job in custom-runners

[Alex Bennée]

Without linking it in it won't be presented on the UI.

Fixes: cc44a16002 ("gitlab: add a new aarch32 custom runner definition")
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .gitlab-ci.d/custom-runners.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
index 3e76a2034a..15aaccc481 100644
--- a/.gitlab-ci.d/custom-runners.yml
+++ b/.gitlab-ci.d/custom-runners.yml
@@ -16,4 +16,5 @@ variables:
 include:
   - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml'
   - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml'
+  - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.yml'
   - local: '/.gitlab-ci.d/custom-runners/centos-stream-8-x86_64.yml'
-- 
2.30.2

Re: [PATCH v1 8/8] gitlab: include new aarch32 job in custom-runners

[Alex Bennée]

[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]

And of course after sending I immediately see the aarch32 job is misnamed.
Will send a v2 follow-up individually

On Tue, 15 Mar 2022 at 12:12, Alex Bennée <alex.bennee@linaro.org> wrote:

> Without linking it in it won't be presented on the UI.
>
> Fixes: cc44a16002 ("gitlab: add a new aarch32 custom runner definition")
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  .gitlab-ci.d/custom-runners.yml | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/.gitlab-ci.d/custom-runners.yml
> b/.gitlab-ci.d/custom-runners.yml
> index 3e76a2034a..15aaccc481 100644
> --- a/.gitlab-ci.d/custom-runners.yml
> +++ b/.gitlab-ci.d/custom-runners.yml
> @@ -16,4 +16,5 @@ variables:
>  include:
>    - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml'
>    - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml'
> +  - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.yml'
>    - local: '/.gitlab-ci.d/custom-runners/centos-stream-8-x86_64.yml'
> --
> 2.30.2
>
>

-- 
Alex Bennée
KVM/QEMU Hacker for Linaro

[-- Attachment #2: Type: text/html, Size: 1589 bytes --]

[PATCH v2] gitlab: include new aarch32 job in custom-runners

[Alex Bennée]

Without linking it in it won't be presented on the UI. Also while
doing that fix the misnamed job from 20.40 to 20.04.

Fixes: cc44a16002 ("gitlab: add a new aarch32 custom runner definition")
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - argggggg!!!!!
---
 .gitlab-ci.d/custom-runners.yml                                  | 1 +
 .../{ubuntu-20.40-aarch32.yml => ubuntu-20.04-aarch32.yml}       | 0
 2 files changed, 1 insertion(+)
 rename .gitlab-ci.d/custom-runners/{ubuntu-20.40-aarch32.yml => ubuntu-20.04-aarch32.yml} (100%)

diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
index 3e76a2034a..15aaccc481 100644
--- a/.gitlab-ci.d/custom-runners.yml
+++ b/.gitlab-ci.d/custom-runners.yml
@@ -16,4 +16,5 @@ variables:
 include:
   - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml'
   - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml'
+  - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.yml'
   - local: '/.gitlab-ci.d/custom-runners/centos-stream-8-x86_64.yml'
diff --git a/.gitlab-ci.d/custom-runners/ubuntu-20.40-aarch32.yml b/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.yml
similarity index 100%
rename from .gitlab-ci.d/custom-runners/ubuntu-20.40-aarch32.yml
rename to .gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.yml
-- 
2.30.2

Re: [PATCH v1 7/8] semihosting: clean up handling of expanded argv

[Philippe Mathieu-Daudé]

On 15/3/22 13:12, Alex Bennée wrote:
> Another cleanup patch tripped over the fact we weren't being careful
> in our casting. Fix the casts, allow for a non-const and switch from
> g_realloc to g_renew.
> 
> The whole semihosting argument handling could do with some tests
> though.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   semihosting/config.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/semihosting/config.c b/semihosting/config.c
> index 137171b717..50d82108e6 100644
> --- a/semihosting/config.c
> +++ b/semihosting/config.c
> @@ -51,7 +51,7 @@ typedef struct SemihostingConfig {
>       bool enabled;
>       SemihostingTarget target;
>       Chardev *chardev;
> -    const char **argv;
> +    char **argv;
>       int argc;
>       const char *cmdline; /* concatenated argv */
>   } SemihostingConfig;
> @@ -98,8 +98,8 @@ static int add_semihosting_arg(void *opaque,
>       if (strcmp(name, "arg") == 0) {
>           s->argc++;
>           /* one extra element as g_strjoinv() expects NULL-terminated array */
> -        s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
> -        s->argv[s->argc - 1] = val;
> +        s->argv = g_renew(char *, s->argv, s->argc + 1);
> +        s->argv[s->argc - 1] = g_strdup(val);

Why strdup()?

>           s->argv[s->argc] = NULL;
>       }
>       return 0;

Re: [PATCH v2] gitlab: include new aarch32 job in custom-runners

[Philippe Mathieu-Daudé]

On 15/3/22 13:19, Alex Bennée wrote:
> Without linking it in it won't be presented on the UI. Also while
> doing that fix the misnamed job from 20.40 to 20.04.
> 
> Fixes: cc44a16002 ("gitlab: add a new aarch32 custom runner definition")
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>    - argggggg!!!!!
> ---
>   .gitlab-ci.d/custom-runners.yml                                  | 1 +
>   .../{ubuntu-20.40-aarch32.yml => ubuntu-20.04-aarch32.yml}       | 0
>   2 files changed, 1 insertion(+)
>   rename .gitlab-ci.d/custom-runners/{ubuntu-20.40-aarch32.yml => ubuntu-20.04-aarch32.yml} (100%)
> 
> diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
> index 3e76a2034a..15aaccc481 100644
> --- a/.gitlab-ci.d/custom-runners.yml
> +++ b/.gitlab-ci.d/custom-runners.yml
> @@ -16,4 +16,5 @@ variables:
>   include:
>     - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml'
>     - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml'
> +  - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.yml'
>     - local: '/.gitlab-ci.d/custom-runners/centos-stream-8-x86_64.yml'
> diff --git a/.gitlab-ci.d/custom-runners/ubuntu-20.40-aarch32.yml b/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.yml
> similarity index 100%
> rename from .gitlab-ci.d/custom-runners/ubuntu-20.40-aarch32.yml
> rename to .gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.ym
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH v1 7/8] semihosting: clean up handling of expanded argv

[Alex Bennée]

Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> writes:

> On 15/3/22 13:12, Alex Bennée wrote:
>> Another cleanup patch tripped over the fact we weren't being careful
>> in our casting. Fix the casts, allow for a non-const and switch from
>> g_realloc to g_renew.
>> The whole semihosting argument handling could do with some tests
>> though.
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   semihosting/config.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>> diff --git a/semihosting/config.c b/semihosting/config.c
>> index 137171b717..50d82108e6 100644
>> --- a/semihosting/config.c
>> +++ b/semihosting/config.c
>> @@ -51,7 +51,7 @@ typedef struct SemihostingConfig {
>>       bool enabled;
>>       SemihostingTarget target;
>>       Chardev *chardev;
>> -    const char **argv;
>> +    char **argv;
>>       int argc;
>>       const char *cmdline; /* concatenated argv */
>>   } SemihostingConfig;
>> @@ -98,8 +98,8 @@ static int add_semihosting_arg(void *opaque,
>>       if (strcmp(name, "arg") == 0) {
>>           s->argc++;
>>           /* one extra element as g_strjoinv() expects NULL-terminated array */
>> -        s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
>> -        s->argv[s->argc - 1] = val;
>> +        s->argv = g_renew(char *, s->argv, s->argc + 1);
>> +        s->argv[s->argc - 1] = g_strdup(val);
>
> Why strdup()?

The compiler was having issues with adding a const char * into the array
and it was the quickest way to stop it complaining. I'm not sure what
guarantees you can make about a const char * after you leave the scope
of the function.

>
>>           s->argv[s->argc] = NULL;
>>       }
>>       return 0;


-- 
Alex Bennée

Re: [PATCH v1 7/8] semihosting: clean up handling of expanded argv

[Daniel P. Berrangé]

On Tue, Mar 15, 2022 at 01:59:59PM +0000, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> writes:
> 
> > On 15/3/22 13:12, Alex Bennée wrote:
> >> Another cleanup patch tripped over the fact we weren't being careful
> >> in our casting. Fix the casts, allow for a non-const and switch from
> >> g_realloc to g_renew.
> >> The whole semihosting argument handling could do with some tests
> >> though.
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> ---
> >>   semihosting/config.c | 6 +++---
> >>   1 file changed, 3 insertions(+), 3 deletions(-)
> >> diff --git a/semihosting/config.c b/semihosting/config.c
> >> index 137171b717..50d82108e6 100644
> >> --- a/semihosting/config.c
> >> +++ b/semihosting/config.c
> >> @@ -51,7 +51,7 @@ typedef struct SemihostingConfig {
> >>       bool enabled;
> >>       SemihostingTarget target;
> >>       Chardev *chardev;
> >> -    const char **argv;
> >> +    char **argv;
> >>       int argc;
> >>       const char *cmdline; /* concatenated argv */
> >>   } SemihostingConfig;
> >> @@ -98,8 +98,8 @@ static int add_semihosting_arg(void *opaque,
> >>       if (strcmp(name, "arg") == 0) {
> >>           s->argc++;
> >>           /* one extra element as g_strjoinv() expects NULL-terminated array */
> >> -        s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
> >> -        s->argv[s->argc - 1] = val;
> >> +        s->argv = g_renew(char *, s->argv, s->argc + 1);
> >> +        s->argv[s->argc - 1] = g_strdup(val);
> >
> > Why strdup()?
> 
> The compiler was having issues with adding a const char * into the array
> and it was the quickest way to stop it complaining. I'm not sure what
> guarantees you can make about a const char * after you leave the scope
> of the function.

No guarantees at all. This method was implicitly relying on the caller
never free'ing the const arg it passed in. That is indeed the case here,
because the arg came from a QemuOpts list. It is bad practice to rely
on such things though, so adding the strdup is sane IMHO.

I would have split the strdup out from the realloc -> renew change
though, since it is an independent cleanup.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v1 7/8] semihosting: clean up handling of expanded argv

[Peter Maydell]

On Tue, 15 Mar 2022 at 14:16, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, Mar 15, 2022 at 01:59:59PM +0000, Alex Bennée wrote:
> >
> > Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> writes:
> >
> > > On 15/3/22 13:12, Alex Bennée wrote:
> > >> Another cleanup patch tripped over the fact we weren't being careful
> > >> in our casting. Fix the casts, allow for a non-const and switch from
> > >> g_realloc to g_renew.
> > >> The whole semihosting argument handling could do with some tests
> > >> though.
> > >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > >> ---
> > >>   semihosting/config.c | 6 +++---
> > >>   1 file changed, 3 insertions(+), 3 deletions(-)
> > >> diff --git a/semihosting/config.c b/semihosting/config.c
> > >> index 137171b717..50d82108e6 100644
> > >> --- a/semihosting/config.c
> > >> +++ b/semihosting/config.c
> > >> @@ -51,7 +51,7 @@ typedef struct SemihostingConfig {
> > >>       bool enabled;
> > >>       SemihostingTarget target;
> > >>       Chardev *chardev;
> > >> -    const char **argv;
> > >> +    char **argv;
> > >>       int argc;
> > >>       const char *cmdline; /* concatenated argv */
> > >>   } SemihostingConfig;
> > >> @@ -98,8 +98,8 @@ static int add_semihosting_arg(void *opaque,
> > >>       if (strcmp(name, "arg") == 0) {
> > >>           s->argc++;
> > >>           /* one extra element as g_strjoinv() expects NULL-terminated array */
> > >> -        s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
> > >> -        s->argv[s->argc - 1] = val;
> > >> +        s->argv = g_renew(char *, s->argv, s->argc + 1);
> > >> +        s->argv[s->argc - 1] = g_strdup(val);
> > >
> > > Why strdup()?
> >
> > The compiler was having issues with adding a const char * into the array
> > and it was the quickest way to stop it complaining. I'm not sure what
> > guarantees you can make about a const char * after you leave the scope
> > of the function.
>
> No guarantees at all. This method was implicitly relying on the caller
> never free'ing the const arg it passed in. That is indeed the case here,
> because the arg came from a QemuOpts list. It is bad practice to rely
> on such things though, so adding the strdup is sane IMHO.
>
> I would have split the strdup out from the realloc -> renew change
> though, since it is an independent cleanup.

If we ever move the glib minimum-version up to 2.68, we could use
the g_strv_builder_new()/g_strv_builder_add() APIs in glib which
do exactly this job of "build up a NULL-terminated array of strings,
one string at a time".

-- PMM

Re: [PATCH v2] gitlab: include new aarch32 job in custom-runners

[Richard Henderson]

On 3/15/22 05:19, Alex Bennée wrote:
> Without linking it in it won't be presented on the UI. Also while
> doing that fix the misnamed job from 20.40 to 20.04.
> 
> Fixes: cc44a16002 ("gitlab: add a new aarch32 custom runner definition")
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v1 5/8] mailmap/gitdm: more fixes for bad tags and authors

[Michael Ellerman]

Alex Bennée <alex.bennee@linaro.org> writes:
> I was running some historical tags for the last 10 years and got the
> following warnings:
>
>   git log --use-mailmap --numstat --since "June 2010" | ~/src/gitdm.git/gitdm -n -l 5
>   alarson@ddci.com is an author name, probably not what you want
>   bad utf-8 ('utf-8' codec can't decode byte 0xe4 in position 552: invalid continuation byte) in patchm skipping
>   bad utf-8 ('utf-8' codec can't decode byte 0xe4 in position 342: invalid continuation byte) in patchm skipping
>   michael@ozlabs.org  is an author name, probably not what you want
>   Oops...funky email nicta.com.au
>   bad utf-8 ('utf-8' codec can't decode byte 0xe9 in position 232: invalid continuation byte) in patchm skipping
>   Oops...funky email andreas.faerber
>   Grabbing changesets...done
>   Processed 76422 csets from 1902 developers
>
> The following fixes try and alleviate that although I still get a
> warning for Aaron which I think is from 9743cd5736.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Acked-by: Aaron Larson <alarson@ddci.com>
> Cc: Andreas Färber <andreas.faerber@web.de>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael Ellerman <michael@ellerman.id.au>
>
> ---
> v2
>   - added Aaron's acked by
>   - dropped Peter Chubb from CC (bouncing emails)
>   - used Michaels preferred coding email

Thanks.

Acked-by: Michael Ellerman <mpe@ellerman.id.au>

cheers

Re: [PATCH for 7.0 v1 0/8] misc testing, i386, docs, gitdm, gitlab

[Alex Bennée]

Alex Bennée <alex.bennee@linaro.org> writes:

> Hi,
>
> As per usual here are the collection of random fixes and tweaks as we
> go through the release process. Most of these patches have been posted
> individually before although the semihosting and gitlab patches are
> new.
>
> The following need review:
>
>  - gitlab: include new aarch32 job in custom-runners
>  - semihosting: clean up handling of expanded argv
>  - tests/avocado: update aarch64_virt test to exercise -cpu max

Ping? Any more reviews before I generate the PR?

-- 
Alex Bennée

Re: [PATCH v1 2/8] tests/avocado: update aarch64_virt test to exercise -cpu max

[Thomas Huth]

On 15/03/2022 13.12, Alex Bennée wrote:
> The Fedora 29 kernel is quite old and importantly fails when running
> in LPA2 scenarios. As it's not really exercising much of the CPU space
> replace it with a custom 5.16.12 kernel with all the architecture
> options turned on. There is a minimal buildroot initramfs included in
> the kernel which has a few tools for stress testing the memory
> subsystem. The userspace also targets the Neoverse N1 processor so
> will fail without additional v8.x+ features.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20220307172249.3125779-1-alex.bennee@linaro.org>
> ---
>   tests/avocado/boot_linux_console.py | 22 ++++++++++++++--------
>   1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/avocado/boot_linux_console.py b/tests/avocado/boot_linux_console.py
> index 9c618d4809..ca3a40e91d 100644
> --- a/tests/avocado/boot_linux_console.py
> +++ b/tests/avocado/boot_linux_console.py
> @@ -12,6 +12,7 @@
>   import lzma
>   import gzip
>   import shutil
> +import time
>   
>   from avocado import skip
>   from avocado import skipUnless
> @@ -330,25 +331,30 @@ def test_aarch64_virt(self):
>           :avocado: tags=arch:aarch64
>           :avocado: tags=machine:virt
>           :avocado: tags=accel:tcg
> -        :avocado: tags=cpu:cortex-a53
> +        :avocado: tags=cpu:max
>           """
> -        kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
> -                      '/linux/releases/29/Everything/aarch64/os/images/pxeboot'
> -                      '/vmlinuz')
> -        kernel_hash = '8c73e469fc6ea06a58dc83a628fc695b693b8493'
> +        kernel_url = ('https://fileserver.linaro.org/s/'
> +                      'z6B2ARM7DQT3HWN/download')
> +
> +        kernel_hash = 'ed11daab50c151dde0e1e9c9cb8b2d9bd3215347'
>           kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>   
>           self.vm.set_console()
>           kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
>                                  'console=ttyAMA0')
>           self.require_accelerator("tcg")
> -        self.vm.add_args('-cpu', 'cortex-a53',
> +        self.vm.add_args('-cpu', 'max,pauth-impdef=on',
>                            '-accel', 'tcg',
>                            '-kernel', kernel_path,
>                            '-append', kernel_command_line)
>           self.vm.launch()
> -        console_pattern = 'Kernel command line: %s' % kernel_command_line
> -        self.wait_for_console_pattern(console_pattern)
> +        self.wait_for_console_pattern('Welcome to Buildroot')
> +        time.sleep(0.1)
> +        exec_command(self, 'root')
> +        time.sleep(0.1)
> +        exec_command(self, 'cat /proc/self/maps')
> +        time.sleep(0.1)

I can't say much about scope of aarch64 testing, but while you're at it, you 
also could move the new test to a separate file, like 
machine_aarch64_virt.py, so that it could be added to MAINTAINERS, too?

  Thomas

Re: [PATCH v1 7/8] semihosting: clean up handling of expanded argv

[Philippe Mathieu-Daudé]

On 15/3/22 15:15, Daniel P. Berrangé wrote:
> On Tue, Mar 15, 2022 at 01:59:59PM +0000, Alex Bennée wrote:
>>
>> Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> writes:
>>
>>> On 15/3/22 13:12, Alex Bennée wrote:
>>>> Another cleanup patch tripped over the fact we weren't being careful
>>>> in our casting. Fix the casts, allow for a non-const and switch from
>>>> g_realloc to g_renew.
>>>> The whole semihosting argument handling could do with some tests
>>>> though.
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> ---
>>>>    semihosting/config.c | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>> diff --git a/semihosting/config.c b/semihosting/config.c
>>>> index 137171b717..50d82108e6 100644
>>>> --- a/semihosting/config.c
>>>> +++ b/semihosting/config.c
>>>> @@ -51,7 +51,7 @@ typedef struct SemihostingConfig {
>>>>        bool enabled;
>>>>        SemihostingTarget target;
>>>>        Chardev *chardev;
>>>> -    const char **argv;
>>>> +    char **argv;
>>>>        int argc;
>>>>        const char *cmdline; /* concatenated argv */
>>>>    } SemihostingConfig;
>>>> @@ -98,8 +98,8 @@ static int add_semihosting_arg(void *opaque,
>>>>        if (strcmp(name, "arg") == 0) {
>>>>            s->argc++;
>>>>            /* one extra element as g_strjoinv() expects NULL-terminated array */
>>>> -        s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
>>>> -        s->argv[s->argc - 1] = val;
>>>> +        s->argv = g_renew(char *, s->argv, s->argc + 1);
>>>> +        s->argv[s->argc - 1] = g_strdup(val);
>>>
>>> Why strdup()?
>>
>> The compiler was having issues with adding a const char * into the array
>> and it was the quickest way to stop it complaining. I'm not sure what
>> guarantees you can make about a const char * after you leave the scope
>> of the function.
> 
> No guarantees at all. This method was implicitly relying on the caller
> never free'ing the const arg it passed in. That is indeed the case here,
> because the arg came from a QemuOpts list. It is bad practice to rely
> on such things though, so adding the strdup is sane IMHO.

I see, thanks.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH v1 2/8] tests/avocado: update aarch64_virt test to exercise -cpu max

[Richard Henderson]

On 3/15/22 05:12, Alex Bennée wrote:
> The Fedora 29 kernel is quite old and importantly fails when running
> in LPA2 scenarios. As it's not really exercising much of the CPU space
> replace it with a custom 5.16.12 kernel with all the architecture
> options turned on. There is a minimal buildroot initramfs included in
> the kernel which has a few tools for stress testing the memory
> subsystem. The userspace also targets the Neoverse N1 processor so
> will fail without additional v8.x+ features.

Maybe better as "would fail with a v8.0 cpu like cortex-a53"?
The way it is worded now suggests it won't work at all at present, since we don't 
completely support all features of the neoverse-n1.

> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20220307172249.3125779-1-alex.bennee@linaro.org>
> ---
>   tests/avocado/boot_linux_console.py | 22 ++++++++++++++--------
>   1 file changed, 14 insertions(+), 8 deletions(-)

Acked-by: Richard Henderson <richard.henderson@linaro.org>

>  (04/32) tests/avocado/boot_linux_console.py:BootLinuxConsole.test_aarch64_virt: PASS (3.66 s)

Tested-by: Richard Henderson <richard.henderson@linaro.org>


r~