| Message ID | 20230424114533.1937153-2-iii@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | tests/tcg/s390x: Enable the multiarch system tests | expand |
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>
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. [...] >
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
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.
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>
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