diff mbox series

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

Message ID 20230424114533.1937153-2-iii@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series tests/tcg/s390x: Enable the multiarch system tests | expand

Commit Message

Ilya Leoshkevich April 24, 2023, 11:45 a.m. UTC
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

Comments

Alex Bennée April 24, 2023, 1 p.m. UTC | #1
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>
Ilya Leoshkevich April 24, 2023, 1:10 p.m. UTC | #2
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.

[...]
>
Thomas Huth April 25, 2023, 7:19 a.m. UTC | #3
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
Ilya Leoshkevich April 25, 2023, 10:27 a.m. UTC | #4
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 mbox series

Patch

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>