From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from zen.linaroharston ([85.9.250.243]) by smtp.gmail.com with ESMTPSA id u16-20020a7bc050000000b003f080b2f9f4sm16285328wmc.27.2023.06.21.07.44.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Jun 2023 07:44:04 -0700 (PDT) Received: from zen (localhost [127.0.0.1]) by zen.linaroharston (Postfix) with ESMTP id 5F16C1FFBB; Wed, 21 Jun 2023 15:44:04 +0100 (BST) References: <20230606132743.1386003-1-iii@linux.ibm.com> <20230606132743.1386003-9-iii@linux.ibm.com> <87legdglka.fsf@linaro.org> <4cf02e6ef71c6f2bff38f06b433e99526ee244d4.camel@linux.ibm.com> User-agent: mu4e 1.11.6; emacs 29.0.92 From: Alex =?utf-8?Q?Benn=C3=A9e?= To: Ilya Leoshkevich Cc: Laurent Vivier , Peter Maydell , Richard Henderson , David Hildenbrand , Philippe =?utf-8?Q?Mathieu-Daud=C3=A9?= , qemu-devel@nongnu.org, qemu-arm@nongnu.org, qemu-s390x@nongnu.org Subject: Re: [PATCH v3 8/8] tests/tcg: Add a test for info proc mappings Date: Wed, 21 Jun 2023 15:43:49 +0100 In-reply-to: <4cf02e6ef71c6f2bff38f06b433e99526ee244d4.camel@linux.ibm.com> Message-ID: <87h6r0ho3w.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-TUID: qQlA39N2i+n8 Ilya Leoshkevich writes: > On Wed, 2023-06-21 at 11:21 +0100, Alex Benn=C3=A9e wrote: >>=20 >> Ilya Leoshkevich writes: >>=20 >> > Add a small test to prevent regressions. >> > Since there are issues with how GDB interprets QEMU's target.xml, >> > enable the test only on aarch64 and s390x for now. >> >=20 >> > Signed-off-by: Ilya Leoshkevich >> > --- >> > =C2=A0tests/tcg/aarch64/Makefile.target=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 3 +- >> > =C2=A0tests/tcg/multiarch/Makefile.target=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 7 +++ >> > =C2=A0.../multiarch/gdbstub/test-proc-mappings.py=C2=A0=C2=A0 | 55 >> > +++++++++++++++++++ >> > =C2=A0tests/tcg/s390x/Makefile.target=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 2 +- >> > =C2=A04 files changed, 65 insertions(+), 2 deletions(-) >> > =C2=A0create mode 100644 tests/tcg/multiarch/gdbstub/test-proc- >> > mappings.py >> >=20 >> > diff --git a/tests/tcg/aarch64/Makefile.target >> > b/tests/tcg/aarch64/Makefile.target >> > index 03157954871..38402b0ba1f 100644 >> > --- a/tests/tcg/aarch64/Makefile.target >> > +++ b/tests/tcg/aarch64/Makefile.target >> > @@ -97,7 +97,8 @@ run-gdbstub-sve-ioctls: sve-ioctls >> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0--bin $< --test $(AARCH64_SRC)/gdbstub/test-sve- >> > ioctl.py, \ >> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0basic gdbstub SVE ZLEN= support) >> > =C2=A0 >> > -EXTRA_RUNS +=3D run-gdbstub-sysregs run-gdbstub-sve-ioctls >> > +EXTRA_RUNS +=3D run-gdbstub-sysregs run-gdbstub-sve-ioctls \ >> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 run-gdbstub-proc-mappings >> > =C2=A0endif >> > =C2=A0endif >> > =C2=A0 >> > diff --git a/tests/tcg/multiarch/Makefile.target >> > b/tests/tcg/multiarch/Makefile.target >> > index 373db696481..cbc0b75787a 100644 >> > --- a/tests/tcg/multiarch/Makefile.target >> > +++ b/tests/tcg/multiarch/Makefile.target >> > @@ -81,6 +81,13 @@ run-gdbstub-qxfer-auxv-read: sha1 >> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0--bin $< --test $(MULTIARCH_SRC)/gdbstub/test- >> > qxfer-auxv-read.py, \ >> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0basic gdbstub qXfer:au= xv:read support) >> > =C2=A0 >> > +run-gdbstub-proc-mappings: sha1 >> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0$(call run-test, $@, $(GDB_= SCRIPT) \ >> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0--gdb $(HAVE_GDB_BIN) \ >> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \ >> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0--bin $< --test $(MULTIARCH_SRC)/gdbstub/test-proc- >> > mappings.py, \ >> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0proc mappings support) >> > + >>=20 >> I wondered if it makes more sense to keep the extra test >> configuration >> logic in multiarch: >>=20 >> =C2=A0 run-gdbstub-proc-mappings: sha1 >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 $(call run-test, = $@, $(GDB_SCRIPT) \ >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 --gdb $(HAVE_GDB_BIN) \ >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \ >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 --bin $< --test $(MULTIARCH_SRC)/gdbstub/tes= t-proc- >> mappings.py, \ >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 proc mappings sup= port) >>=20 >> =C2=A0 # only enable for s390x and aarch64 for now >> =C2=A0 ifneq (,$(findstring aarch64,$(TARGET_NAME))) >> =C2=A0 EXTRA_RUNS +=3D run-gdbstub-proc-mappings >> =C2=A0 else ifneq (,$(findstring s390x,$(TARGET_NAME))) >> =C2=A0 EXTRA_RUNS +=3D run-gdbstub-proc-mappings >> =C2=A0 endif >>=20 >> but it still ends up pretty ugly. Is the gdb handling fixed for other >> arches in other versions. Maybe we could probe gdb for support and >> wrap >> the whole stanza in something like: >>=20 >> =C2=A0 ifeq ($(HOST_GDB_SUPPORTS_PROC_MAPPING),y) >> =C2=A0 ... >> =C2=A0 endif >>=20 >> ? > > I think I better add the check to the test itself, because otherwise we > have to probe GDB against QEMU binary we just built, which sounds > unnecessarily complicated. > > The error message on all arches without this series is: > > warning: unable to open /proc file '/proc/1/maps' > > The error message on x86_64 (expected) with this series is: > > Not supported on this target. > > So we can simply exit(0) from the test if we see this. That seems a simpler solution, lets do that. --=20 Alex Benn=C3=A9e Virtualisation Tech Lead @ Linaro