[PATCH v2 0/3] tests/tcg/s390x: Enable the multiarch system tests

[PATCH v2 0/3] tests/tcg/s390x: Enable the multiarch system tests

[Ilya Leoshkevich]

v1: https://lists.gnu.org/archive/html/qemu-devel/2023-04/msg03746.html
v1 -> v2: Use cpu_to_le16() and friends (Thomas).

Hi,

I noticed that Alex added "undefine MULTIARCH_TESTS" to
tests/tcg/s390x/Makefile.softmmu-target in the plugin patch, and
thought that it may better to just enable them, which this series
does.

Patch 1 makes the QEMU headers available to the tests.

Patch 2 fixes an endianness issue in the memory test.

Patch 3 enables the multiarch system test. The main difficulty is
outputting characters via SCLP, which is sidestepped by reusing the
pc-bios/s390-ccw implementation.

Best regards,
Ilya

Ilya Leoshkevich (3):
  tests/tcg: Make the QEMU headers available to the tests
  tests/tcg/multiarch: Make the system memory test work on big-endian
  tests/tcg/s390x: Enable the multiarch system tests

 tests/include/qemu/testdep.h            | 14 +++++++++
 tests/tcg/Makefile.target               |  4 +--
 tests/tcg/aarch64/sve-ioctls.c          |  1 +
 tests/tcg/aarch64/sysregs.c             |  1 +
 tests/tcg/multiarch/system/memory.c     | 26 ++++++++--------
 tests/tcg/s390x/Makefile.softmmu-target | 40 +++++++++++++++++--------
 tests/tcg/s390x/console.c               | 12 ++++++++
 tests/tcg/s390x/head64.S                | 31 +++++++++++++++++++
 8 files changed, 102 insertions(+), 27 deletions(-)
 create mode 100644 tests/include/qemu/testdep.h
 create mode 100644 tests/tcg/s390x/console.c
 create mode 100644 tests/tcg/s390x/head64.S

-- 
2.39.2

[PATCH v2 1/3] tests/tcg: Make the QEMU headers available to the tests

[Ilya Leoshkevich]

The QEMU headers contain macros and functions that are useful in the
test context. Add them to tests' include path. Also provide a header
similar to "qemu/osdep.h" for use in the freestanding environment.

Tests that include <sys/auxv.h> get QEMU's copy of <elf.h>, which does
not work without <stdint.h>. Make use of the new header in these tests
in order to fix this.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/include/qemu/testdep.h   | 14 ++++++++++++++
 tests/tcg/Makefile.target      |  4 ++--
 tests/tcg/aarch64/sve-ioctls.c |  1 +
 tests/tcg/aarch64/sysregs.c    |  1 +
 4 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 tests/include/qemu/testdep.h

diff --git a/tests/include/qemu/testdep.h b/tests/include/qemu/testdep.h
new file mode 100644
index 00000000000..ddf7c543bf4
--- /dev/null
+++ b/tests/include/qemu/testdep.h
@@ -0,0 +1,14 @@
+/*
+ * Common dependencies for QEMU tests.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef QEMU_TESTDEP_H
+#define QEMU_TESTDEP_H
+
+#include <stdint.h>
+#include "qemu/compiler.h"
+
+#define g_assert_not_reached __builtin_trap
+
+#endif
diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
index 8318caf9247..5474395e693 100644
--- a/tests/tcg/Makefile.target
+++ b/tests/tcg/Makefile.target
@@ -85,8 +85,8 @@ TESTS=
 # additional tests which may re-use existing binaries
 EXTRA_TESTS=
 
-# Start with a blank slate, the build targets get to add stuff first
-CFLAGS=
+# Start with the minimal build flags, the build targets will extend them
+CFLAGS=-I$(SRC_PATH)/include -I$(SRC_PATH)/tests/include
 LDFLAGS=
 
 QEMU_OPTS=
diff --git a/tests/tcg/aarch64/sve-ioctls.c b/tests/tcg/aarch64/sve-ioctls.c
index 9544dffa0ee..11a0a4e47ff 100644
--- a/tests/tcg/aarch64/sve-ioctls.c
+++ b/tests/tcg/aarch64/sve-ioctls.c
@@ -8,6 +8,7 @@
  *
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
+#include "qemu/testdep.h"
 #include <sys/prctl.h>
 #include <asm/hwcap.h>
 #include <stdio.h>
diff --git a/tests/tcg/aarch64/sysregs.c b/tests/tcg/aarch64/sysregs.c
index 46b931f781d..35ec25026a9 100644
--- a/tests/tcg/aarch64/sysregs.c
+++ b/tests/tcg/aarch64/sysregs.c
@@ -11,6 +11,7 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 
+#include "qemu/testdep.h"
 #include <asm/hwcap.h>
 #include <stdio.h>
 #include <sys/auxv.h>
-- 
2.39.2

[PATCH v2 2/3] tests/tcg/multiarch: Make the system memory test work on big-endian

[Ilya Leoshkevich]

Make sure values are stored in memory as little-endian regardless of
the host endianness.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/multiarch/system/memory.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/tests/tcg/multiarch/system/memory.c b/tests/tcg/multiarch/system/memory.c
index 214f7d4f54b..3a15f3f494a 100644
--- a/tests/tcg/multiarch/system/memory.c
+++ b/tests/tcg/multiarch/system/memory.c
@@ -12,9 +12,11 @@
  *   - sign extension when loading
  */
 
+#include "qemu/testdep.h"
 #include <stdint.h>
 #include <stdbool.h>
 #include <minilib.h>
+#include "qemu/bswap.h"
 
 #ifndef CHECK_UNALIGNED
 # error "Target does not specify CHECK_UNALIGNED"
@@ -40,8 +42,7 @@ static void pdot(int count)
 }
 
 /*
- * Helper macros for shift/extract so we can keep our endian handling
- * in one place.
+ * Helper macros for shift/extract.
  */
 #define BYTE_SHIFT(b, pos) ((uint64_t)b << (pos * 8))
 #define BYTE_EXTRACT(b, pos) ((b >> (pos * 8)) & 0xff)
@@ -49,9 +50,8 @@ static void pdot(int count)
 /*
  * Fill the data with ascending value bytes.
  *
- * Currently we only support Little Endian machines so write in
- * ascending address order. When we read higher address bytes should
- * either be zero or higher than the lower bytes.
+ * Store the words in the Little Endian byte order. When we read higher address
+ * bytes should either be zero or higher than the lower bytes.
  */
 
 static void init_test_data_u8(int unused_offset)
@@ -121,7 +121,7 @@ static void init_test_data_u16(int offset)
     for (i = 0; i < max; i++) {
         uint8_t low = count++, high = count++;
         word = BYTE_SHIFT(high, 1) | BYTE_SHIFT(low, 0);
-        *ptr++ = word;
+        *ptr++ = cpu_to_le16(word);
         pdot(i);
     }
     ml_printf("done @ %p\n", ptr);
@@ -142,7 +142,7 @@ static void init_test_data_u32(int offset)
         uint8_t b4 = count++, b3 = count++;
         uint8_t b2 = count++, b1 = count++;
         word = BYTE_SHIFT(b1, 3) | BYTE_SHIFT(b2, 2) | BYTE_SHIFT(b3, 1) | b4;
-        *ptr++ = word;
+        *ptr++ = cpu_to_le32(word);
         pdot(i);
     }
     ml_printf("done @ %p\n", ptr);
@@ -167,7 +167,7 @@ static void init_test_data_u64(int offset)
         word = BYTE_SHIFT(b1, 7) | BYTE_SHIFT(b2, 6) | BYTE_SHIFT(b3, 5) |
                BYTE_SHIFT(b4, 4) | BYTE_SHIFT(b5, 3) | BYTE_SHIFT(b6, 2) |
                BYTE_SHIFT(b7, 1) | b8;
-        *ptr++ = word;
+        *ptr++ = cpu_to_le64(word);
         pdot(i);
     }
     ml_printf("done @ %p\n", ptr);
@@ -183,7 +183,7 @@ static bool read_test_data_u16(int offset)
 
     for (i = 0; i < max; i++) {
         uint8_t high, low;
-        word = *ptr++;
+        word = le16_to_cpu(*ptr++);
         high = (word >> 8) & 0xff;
         low = word & 0xff;
         if (high < low && high != 0) {
@@ -209,7 +209,7 @@ static bool read_test_data_u32(int offset)
     for (i = 0; i < max; i++) {
         uint8_t b1, b2, b3, b4;
         int zeros = 0;
-        word = *ptr++;
+        word = le32_to_cpu(*ptr++);
 
         b1 = word >> 24 & 0xff;
         b2 = word >> 16 & 0xff;
@@ -250,7 +250,7 @@ static bool read_test_data_u64(int offset)
     for (i = 0; i < max; i++) {
         uint8_t b1, b2, b3, b4, b5, b6, b7, b8;
         int zeros = 0;
-        word = *ptr++;
+        word = le64_to_cpu(*ptr++);
 
         b1 = ((uint64_t) (word >> 56)) & 0xff;
         b2 = ((uint64_t) (word >> 48)) & 0xff;
@@ -375,7 +375,7 @@ static bool read_test_data_s16(int offset, bool neg_first)
               offset, neg_first ? "neg" : "pos");
 
     for (i = 0; i < max; i++) {
-        int32_t data = *ptr++;
+        int32_t data = le16_to_cpu(*ptr++);
 
         if (neg_first && data < 0) {
             pdot(i);
@@ -400,7 +400,7 @@ static bool read_test_data_s32(int offset, bool neg_first)
               ptr, offset, neg_first ? "neg" : "pos");
 
     for (i = 0; i < max; i++) {
-        int64_t data = *ptr++;
+        int64_t data = le32_to_cpu(*ptr++);
 
         if (neg_first && data < 0) {
             pdot(i);
-- 
2.39.2

[PATCH v2 3/3] tests/tcg/s390x: Enable the multiarch system tests

[Ilya Leoshkevich]

Multiarch tests are written in C and need support for printing
characters. Instead of implementing the runtime from scratch, just
reuse the pc-bios/s390-ccw one.

Run tests with -nographic in order to enable SCLP (enable this for
the existing tests as well, since it does not hurt).

Use the default linker script for the new tests.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/s390x/Makefile.softmmu-target | 40 +++++++++++++++++--------
 tests/tcg/s390x/console.c               | 12 ++++++++
 tests/tcg/s390x/head64.S                | 31 +++++++++++++++++++
 3 files changed, 71 insertions(+), 12 deletions(-)
 create mode 100644 tests/tcg/s390x/console.c
 create mode 100644 tests/tcg/s390x/head64.S

diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target
index 3e7f72abcdc..e22df5e1c58 100644
--- a/tests/tcg/s390x/Makefile.softmmu-target
+++ b/tests/tcg/s390x/Makefile.softmmu-target
@@ -1,25 +1,41 @@
 S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
 VPATH+=$(S390X_SRC)
-QEMU_OPTS=-action panic=exit-failure -kernel
+QEMU_OPTS=-action panic=exit-failure -nographic -kernel
 LINK_SCRIPT=$(S390X_SRC)/softmmu.ld
-LDFLAGS=-nostdlib -static -Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none
+CFLAGS+=-ggdb -O0
+LDFLAGS=-nostdlib -static
 
 %.o: %.S
 	$(CC) -march=z13 -m64 -c $< -o $@
 
-%: %.o $(LINK_SCRIPT)
+%.o: %.c
+	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) -march=z13 -m64 -c $< -o $@
+
+%: %.o
 	$(CC) $< -o $@ $(LDFLAGS)
 
-TESTS += unaligned-lowcore
-TESTS += bal
-TESTS += sam
-TESTS += lpsw
-TESTS += lpswe-early
-TESTS += ssm-early
-TESTS += stosm-early
-TESTS += exrl-ssm-early
+ASM_TESTS =                                                                    \
+    bal                                                                        \
+    exrl-ssm-early                                                             \
+    sam                                                                        \
+    lpsw                                                                       \
+    lpswe-early                                                                \
+    ssm-early                                                                  \
+    stosm-early                                                                \
+    unaligned-lowcore
 
 include $(S390X_SRC)/pgm-specification.mak
 $(PGM_SPECIFICATION_TESTS): pgm-specification-softmmu.o
 $(PGM_SPECIFICATION_TESTS): LDFLAGS+=pgm-specification-softmmu.o
-TESTS += $(PGM_SPECIFICATION_TESTS)
+ASM_TESTS += $(PGM_SPECIFICATION_TESTS)
+
+$(ASM_TESTS): LDFLAGS += -Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none
+$(ASM_TESTS): $(LINK_SCRIPT)
+TESTS += $(ASM_TESTS)
+
+S390X_MULTIARCH_RUNTIME_OBJS = head64.o console.o $(MINILIB_OBJS)
+$(MULTIARCH_TESTS): $(S390X_MULTIARCH_RUNTIME_OBJS)
+$(MULTIARCH_TESTS): LDFLAGS += $(S390X_MULTIARCH_RUNTIME_OBJS)
+$(MULTIARCH_TESTS): CFLAGS += $(MINILIB_INC)
+memory: CFLAGS += -DCHECK_UNALIGNED=0
+TESTS += $(MULTIARCH_TESTS)
diff --git a/tests/tcg/s390x/console.c b/tests/tcg/s390x/console.c
new file mode 100644
index 00000000000..d43ce3f44b4
--- /dev/null
+++ b/tests/tcg/s390x/console.c
@@ -0,0 +1,12 @@
+/*
+ * Console code for multiarch tests.
+ * Reuses the pc-bios/s390-ccw implementation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "../../../pc-bios/s390-ccw/sclp.c"
+
+void __sys_outc(char c)
+{
+    write(1, &c, sizeof(c));
+}
diff --git a/tests/tcg/s390x/head64.S b/tests/tcg/s390x/head64.S
new file mode 100644
index 00000000000..c6f36dfea4b
--- /dev/null
+++ b/tests/tcg/s390x/head64.S
@@ -0,0 +1,31 @@
+/*
+ * Startup code for multiarch tests.
+ * Reuses the pc-bios/s390-ccw implementation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#define main main_pre
+#include "../../../pc-bios/s390-ccw/start.S"
+#undef main
+
+main_pre:
+    aghi %r15,-160                     /* reserve stack for C code */
+    brasl %r14,sclp_setup
+    brasl %r14,main
+    larl %r1,success_psw               /* check main() return code */
+    ltgr %r2,%r2
+    je 0f
+    larl %r1,failure_psw
+0:
+    lpswe 0(%r1)
+
+    .align 8
+success_psw:
+    .quad 0x2000180000000,0xfff        /* see is_special_wait_psw() */
+failure_psw:
+    .quad 0x2000180000000,0            /* disabled wait */
+
+    .section .bss
+    .align 0x1000
+stack:
+    .skip 0x8000
-- 
2.39.2

Re: [PATCH v2 1/3] tests/tcg: Make the QEMU headers available to the tests

[Alex Bennée]

Ilya Leoshkevich <iii@linux.ibm.com> writes:

> The QEMU headers contain macros and functions that are useful in the
> test context. Add them to tests' include path. Also provide a header
> similar to "qemu/osdep.h" for use in the freestanding environment.
>
> Tests that include <sys/auxv.h> get QEMU's copy of <elf.h>, which does
> not work without <stdint.h>. Make use of the new header in these tests
> in order to fix this.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  tests/include/qemu/testdep.h   | 14 ++++++++++++++
>  tests/tcg/Makefile.target      |  4 ++--
>  tests/tcg/aarch64/sve-ioctls.c |  1 +
>  tests/tcg/aarch64/sysregs.c    |  1 +
>  4 files changed, 18 insertions(+), 2 deletions(-)
>  create mode 100644 tests/include/qemu/testdep.h
>
> diff --git a/tests/include/qemu/testdep.h b/tests/include/qemu/testdep.h
> new file mode 100644
> index 00000000000..ddf7c543bf4
> --- /dev/null
> +++ b/tests/include/qemu/testdep.h
> @@ -0,0 +1,14 @@
> +/*
> + * Common dependencies for QEMU tests.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#ifndef QEMU_TESTDEP_H
> +#define QEMU_TESTDEP_H
> +
> +#include <stdint.h>
> +#include "qemu/compiler.h"
> +
> +#define g_assert_not_reached __builtin_trap
> +
> +#endif
> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
> index 8318caf9247..5474395e693 100644
> --- a/tests/tcg/Makefile.target
> +++ b/tests/tcg/Makefile.target
> @@ -85,8 +85,8 @@ TESTS=
>  # additional tests which may re-use existing binaries
>  EXTRA_TESTS=
>  
> -# Start with a blank slate, the build targets get to add stuff first
> -CFLAGS=
> +# Start with the minimal build flags, the build targets will extend them
> +CFLAGS=-I$(SRC_PATH)/include -I$(SRC_PATH)/tests/include
>  LDFLAGS=

Hmm I'm not so sure about this. The tests are deliberately minimal in
terms of their dependencies because its hard enough getting a plain
cross-compiler to work. Is there really much benefit to allowing this?
What happens when a user includes another header which relies on
functionality from one of the many libraries QEMU itself links to?

>  
>  QEMU_OPTS=
> diff --git a/tests/tcg/aarch64/sve-ioctls.c b/tests/tcg/aarch64/sve-ioctls.c
> index 9544dffa0ee..11a0a4e47ff 100644
> --- a/tests/tcg/aarch64/sve-ioctls.c
> +++ b/tests/tcg/aarch64/sve-ioctls.c
> @@ -8,6 +8,7 @@
>   *
>   * SPDX-License-Identifier: GPL-2.0-or-later
>   */
> +#include "qemu/testdep.h"
>  #include <sys/prctl.h>
>  #include <asm/hwcap.h>
>  #include <stdio.h>
> diff --git a/tests/tcg/aarch64/sysregs.c b/tests/tcg/aarch64/sysregs.c
> index 46b931f781d..35ec25026a9 100644
> --- a/tests/tcg/aarch64/sysregs.c
> +++ b/tests/tcg/aarch64/sysregs.c
> @@ -11,6 +11,7 @@
>   * SPDX-License-Identifier: GPL-2.0-or-later
>   */
>  
> +#include "qemu/testdep.h"
>  #include <asm/hwcap.h>
>  #include <stdio.h>
>  #include <sys/auxv.h>


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v2 1/3] tests/tcg: Make the QEMU headers available to the tests

[Ilya Leoshkevich]

On Mon, 2023-04-24 at 14:00 +0100, Alex Bennée wrote:
> 
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> 
> > The QEMU headers contain macros and functions that are useful in
> > the
> > test context. Add them to tests' include path. Also provide a
> > header
> > similar to "qemu/osdep.h" for use in the freestanding environment.
> > 
> > Tests that include <sys/auxv.h> get QEMU's copy of <elf.h>, which
> > does
> > not work without <stdint.h>. Make use of the new header in these
> > tests
> > in order to fix this.
> > 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  tests/include/qemu/testdep.h   | 14 ++++++++++++++
> >  tests/tcg/Makefile.target      |  4 ++--
> >  tests/tcg/aarch64/sve-ioctls.c |  1 +
> >  tests/tcg/aarch64/sysregs.c    |  1 +
> >  4 files changed, 18 insertions(+), 2 deletions(-)
> >  create mode 100644 tests/include/qemu/testdep.h
> > 
> > diff --git a/tests/include/qemu/testdep.h
> > b/tests/include/qemu/testdep.h
> > new file mode 100644
> > index 00000000000..ddf7c543bf4
> > --- /dev/null
> > +++ b/tests/include/qemu/testdep.h
> > @@ -0,0 +1,14 @@
> > +/*
> > + * Common dependencies for QEMU tests.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +#ifndef QEMU_TESTDEP_H
> > +#define QEMU_TESTDEP_H
> > +
> > +#include <stdint.h>
> > +#include "qemu/compiler.h"
> > +
> > +#define g_assert_not_reached __builtin_trap
> > +
> > +#endif
> > diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
> > index 8318caf9247..5474395e693 100644
> > --- a/tests/tcg/Makefile.target
> > +++ b/tests/tcg/Makefile.target
> > @@ -85,8 +85,8 @@ TESTS=
> >  # additional tests which may re-use existing binaries
> >  EXTRA_TESTS=
> >  
> > -# Start with a blank slate, the build targets get to add stuff
> > first
> > -CFLAGS=
> > +# Start with the minimal build flags, the build targets will
> > extend them
> > +CFLAGS=-I$(SRC_PATH)/include -I$(SRC_PATH)/tests/include
> >  LDFLAGS=
> 
> Hmm I'm not so sure about this. The tests are deliberately minimal in
> terms of their dependencies because its hard enough getting a plain
> cross-compiler to work. Is there really much benefit to allowing
> this?
> What happens when a user includes another header which relies on
> functionality from one of the many libraries QEMU itself links to?

I don't think this will work at all, because the idea here is to allow
using the code in the freestanding tests. However, at least bswap.h
seems to work just fine. Of course, there is now additional maintenance
overhead to keep it this way, but I would argue it's better than
making a copy.

[...]
> 

Re: [PATCH v2 1/3] tests/tcg: Make the QEMU headers available to the tests

[Thomas Huth]

On 24/04/2023 15.10, Ilya Leoshkevich wrote:
> On Mon, 2023-04-24 at 14:00 +0100, Alex Bennée wrote:
>>
>> Ilya Leoshkevich <iii@linux.ibm.com> writes:
>>
>>> The QEMU headers contain macros and functions that are useful in
>>> the
>>> test context. Add them to tests' include path. Also provide a
>>> header
>>> similar to "qemu/osdep.h" for use in the freestanding environment.
>>>
>>> Tests that include <sys/auxv.h> get QEMU's copy of <elf.h>, which
>>> does
>>> not work without <stdint.h>. Make use of the new header in these
>>> tests
>>> in order to fix this.
>>>
>>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>>> ---
>>>   tests/include/qemu/testdep.h   | 14 ++++++++++++++
>>>   tests/tcg/Makefile.target      |  4 ++--
>>>   tests/tcg/aarch64/sve-ioctls.c |  1 +
>>>   tests/tcg/aarch64/sysregs.c    |  1 +
>>>   4 files changed, 18 insertions(+), 2 deletions(-)
>>>   create mode 100644 tests/include/qemu/testdep.h
>>>
>>> diff --git a/tests/include/qemu/testdep.h
>>> b/tests/include/qemu/testdep.h
>>> new file mode 100644
>>> index 00000000000..ddf7c543bf4
>>> --- /dev/null
>>> +++ b/tests/include/qemu/testdep.h
>>> @@ -0,0 +1,14 @@
>>> +/*
>>> + * Common dependencies for QEMU tests.
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +#ifndef QEMU_TESTDEP_H
>>> +#define QEMU_TESTDEP_H
>>> +
>>> +#include <stdint.h>
>>> +#include "qemu/compiler.h"
>>> +
>>> +#define g_assert_not_reached __builtin_trap
>>> +
>>> +#endif
>>> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
>>> index 8318caf9247..5474395e693 100644
>>> --- a/tests/tcg/Makefile.target
>>> +++ b/tests/tcg/Makefile.target
>>> @@ -85,8 +85,8 @@ TESTS=
>>>   # additional tests which may re-use existing binaries
>>>   EXTRA_TESTS=
>>>   
>>> -# Start with a blank slate, the build targets get to add stuff
>>> first
>>> -CFLAGS=
>>> +# Start with the minimal build flags, the build targets will
>>> extend them
>>> +CFLAGS=-I$(SRC_PATH)/include -I$(SRC_PATH)/tests/include
>>>   LDFLAGS=
>>
>> Hmm I'm not so sure about this. The tests are deliberately minimal in
>> terms of their dependencies because its hard enough getting a plain
>> cross-compiler to work. Is there really much benefit to allowing
>> this?
>> What happens when a user includes another header which relies on
>> functionality from one of the many libraries QEMU itself links to?
> 
> I don't think this will work at all, because the idea here is to allow
> using the code in the freestanding tests. However, at least bswap.h
> seems to work just fine. Of course, there is now additional maintenance
> overhead to keep it this way, but I would argue it's better than
> making a copy.

If this is just about one single header, I guess a

#include "../../../include/qemu/bswap.h"

would be acceptable, too, instead?

  Thomas

Re: [PATCH v2 1/3] tests/tcg: Make the QEMU headers available to the tests

[Ilya Leoshkevich]

On Tue, 2023-04-25 at 09:19 +0200, Thomas Huth wrote:
> On 24/04/2023 15.10, Ilya Leoshkevich wrote:
> > On Mon, 2023-04-24 at 14:00 +0100, Alex Bennée wrote:
> > > 
> > > Ilya Leoshkevich <iii@linux.ibm.com> writes:
> > > 
> > > > The QEMU headers contain macros and functions that are useful
> > > > in
> > > > the
> > > > test context. Add them to tests' include path. Also provide a
> > > > header
> > > > similar to "qemu/osdep.h" for use in the freestanding
> > > > environment.
> > > > 
> > > > Tests that include <sys/auxv.h> get QEMU's copy of <elf.h>,
> > > > which
> > > > does
> > > > not work without <stdint.h>. Make use of the new header in
> > > > these
> > > > tests
> > > > in order to fix this.
> > > > 
> > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > > ---
> > > >   tests/include/qemu/testdep.h   | 14 ++++++++++++++
> > > >   tests/tcg/Makefile.target      |  4 ++--
> > > >   tests/tcg/aarch64/sve-ioctls.c |  1 +
> > > >   tests/tcg/aarch64/sysregs.c    |  1 +
> > > >   4 files changed, 18 insertions(+), 2 deletions(-)
> > > >   create mode 100644 tests/include/qemu/testdep.h
> > > > 
> > > > diff --git a/tests/include/qemu/testdep.h
> > > > b/tests/include/qemu/testdep.h
> > > > new file mode 100644
> > > > index 00000000000..ddf7c543bf4
> > > > --- /dev/null
> > > > +++ b/tests/include/qemu/testdep.h
> > > > @@ -0,0 +1,14 @@
> > > > +/*
> > > > + * Common dependencies for QEMU tests.
> > > > + *
> > > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > > + */
> > > > +#ifndef QEMU_TESTDEP_H
> > > > +#define QEMU_TESTDEP_H
> > > > +
> > > > +#include <stdint.h>
> > > > +#include "qemu/compiler.h"
> > > > +
> > > > +#define g_assert_not_reached __builtin_trap
> > > > +
> > > > +#endif
> > > > diff --git a/tests/tcg/Makefile.target
> > > > b/tests/tcg/Makefile.target
> > > > index 8318caf9247..5474395e693 100644
> > > > --- a/tests/tcg/Makefile.target
> > > > +++ b/tests/tcg/Makefile.target
> > > > @@ -85,8 +85,8 @@ TESTS=
> > > >   # additional tests which may re-use existing binaries
> > > >   EXTRA_TESTS=
> > > >   
> > > > -# Start with a blank slate, the build targets get to add stuff
> > > > first
> > > > -CFLAGS=
> > > > +# Start with the minimal build flags, the build targets will
> > > > extend them
> > > > +CFLAGS=-I$(SRC_PATH)/include -I$(SRC_PATH)/tests/include
> > > >   LDFLAGS=
> > > 
> > > Hmm I'm not so sure about this. The tests are deliberately
> > > minimal in
> > > terms of their dependencies because its hard enough getting a
> > > plain
> > > cross-compiler to work. Is there really much benefit to allowing
> > > this?
> > > What happens when a user includes another header which relies on
> > > functionality from one of the many libraries QEMU itself links
> > > to?
> > 
> > I don't think this will work at all, because the idea here is to
> > allow
> > using the code in the freestanding tests. However, at least bswap.h
> > seems to work just fine. Of course, there is now additional
> > maintenance
> > overhead to keep it this way, but I would argue it's better than
> > making a copy.
> 
> If this is just about one single header, I guess a
> 
> #include "../../../include/qemu/bswap.h"
> 
> would be acceptable, too, instead?
> 
>   Thomas

This would work, yes, but it would require essentially inlining
the new testdeps.h before it.