* [PATCH v2 01/10] CODING_STYLE.rst: flesh out our naming conventions.
2020-09-09 11:27 [PATCH v2 00/10] testing and misc updates Alex Bennée
@ 2020-09-09 11:27 ` Alex Bennée
2020-09-09 13:44 ` Thomas Huth
2020-09-09 11:27 ` [PATCH v2 02/10] usb-host: restrict workaround to new libusb versions Alex Bennée
` (8 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Alex Bennée @ 2020-09-09 11:27 UTC (permalink / raw)
To: qemu-devel
Cc: fam, berrange, Alex Bennée, richard.henderson, f4bug, cota,
aurelien
Mention a few of the more common naming conventions we follow in the
code base including common variable names and function prefix and
suffix examples.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200903112107.27367-2-alex.bennee@linaro.org>
---
v4
- Incorporated Paolo's suggested paragraph
---
CODING_STYLE.rst | 37 +++++++++++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/CODING_STYLE.rst b/CODING_STYLE.rst
index 427699e0e42..8b13ef0669e 100644
--- a/CODING_STYLE.rst
+++ b/CODING_STYLE.rst
@@ -109,8 +109,41 @@ names are lower_case_with_underscores_ending_with_a_t, like the POSIX
uint64_t and family. Note that this last convention contradicts POSIX
and is therefore likely to be changed.
-When wrapping standard library functions, use the prefix ``qemu_`` to alert
-readers that they are seeing a wrapped version; otherwise avoid this prefix.
+Variable Naming Conventions
+---------------------------
+
+A number of short naming conventions exist for variables that use
+common QEMU types. For example, the architecture independent CPUState
+is often held as a ``cs`` pointer variable, whereas the concrete
+CPUArchState is usually held in a pointer called ``env``.
+
+Likewise, in device emulation code the common DeviceState is usually
+called ``dev``.
+
+Function Naming Conventions
+---------------------------
+
+Wrapped version of standard library or GLib functions use a ``qemu_``
+prefix to alert readers that they are seeing a wrapped version, for
+example ``qemu_strtol`` or ``qemu_mutex_lock``. Other utility functions
+that are widely called from across the codebase should not have any
+prefix, for example ``pstrcpy`` or bit manipulation functions such as
+``find_first_bit``.
+
+The ``qemu_`` prefix is also used for functions that modify global
+emulator state, for example ``qemu_add_vm_change_state_handler``.
+However, if there is an obvious subsystem-specific prefix it should be
+used instead.
+
+Public functions from a file or subsystem (declared in headers) tend
+to have a consistent prefix to show where they came from. For example,
+``tlb_`` for functions from ``cputlb.c`` or ``cpu_`` for functions
+from cpus.c.
+
+If there are two versions of a function to be called with or without a
+lock held, the function that expects the lock to be already held
+usually uses the suffix ``_locked``.
+
Block structure
===============
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 01/10] CODING_STYLE.rst: flesh out our naming conventions.
2020-09-09 11:27 ` [PATCH v2 01/10] CODING_STYLE.rst: flesh out our naming conventions Alex Bennée
@ 2020-09-09 13:44 ` Thomas Huth
0 siblings, 0 replies; 17+ messages in thread
From: Thomas Huth @ 2020-09-09 13:44 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: fam, berrange, richard.henderson, f4bug, cota, aurelien
On 09/09/2020 13.27, Alex Bennée wrote:
> Mention a few of the more common naming conventions we follow in the
> code base including common variable names and function prefix and
> suffix examples.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20200903112107.27367-2-alex.bennee@linaro.org>
>
> ---
> v4
> - Incorporated Paolo's suggested paragraph
> ---
> CODING_STYLE.rst | 37 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/CODING_STYLE.rst b/CODING_STYLE.rst
> index 427699e0e42..8b13ef0669e 100644
> --- a/CODING_STYLE.rst
> +++ b/CODING_STYLE.rst
> @@ -109,8 +109,41 @@ names are lower_case_with_underscores_ending_with_a_t, like the POSIX
> uint64_t and family. Note that this last convention contradicts POSIX
> and is therefore likely to be changed.
>
> -When wrapping standard library functions, use the prefix ``qemu_`` to alert
> -readers that they are seeing a wrapped version; otherwise avoid this prefix.
> +Variable Naming Conventions
> +---------------------------
> +
> +A number of short naming conventions exist for variables that use
> +common QEMU types. For example, the architecture independent CPUState
> +is often held as a ``cs`` pointer variable, whereas the concrete
> +CPUArchState is usually held in a pointer called ``env``.
> +
> +Likewise, in device emulation code the common DeviceState is usually
> +called ``dev``.
> +
> +Function Naming Conventions
> +---------------------------
> +
> +Wrapped version of standard library or GLib functions use a ``qemu_``
> +prefix to alert readers that they are seeing a wrapped version, for
> +example ``qemu_strtol`` or ``qemu_mutex_lock``. Other utility functions
> +that are widely called from across the codebase should not have any
> +prefix, for example ``pstrcpy`` or bit manipulation functions such as
> +``find_first_bit``.
> +
> +The ``qemu_`` prefix is also used for functions that modify global
> +emulator state, for example ``qemu_add_vm_change_state_handler``.
> +However, if there is an obvious subsystem-specific prefix it should be
> +used instead.
> +
> +Public functions from a file or subsystem (declared in headers) tend
> +to have a consistent prefix to show where they came from. For example,
> +``tlb_`` for functions from ``cputlb.c`` or ``cpu_`` for functions
> +from cpus.c.
> +
> +If there are two versions of a function to be called with or without a
> +lock held, the function that expects the lock to be already held
> +usually uses the suffix ``_locked``.
> +
Sounds good to me now.
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 02/10] usb-host: restrict workaround to new libusb versions
2020-09-09 11:27 [PATCH v2 00/10] testing and misc updates Alex Bennée
2020-09-09 11:27 ` [PATCH v2 01/10] CODING_STYLE.rst: flesh out our naming conventions Alex Bennée
@ 2020-09-09 11:27 ` Alex Bennée
2020-09-09 11:27 ` [PATCH v2 03/10] tests/meson.build: fp tests don't need CONFIG_TCG Alex Bennée
` (7 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Alex Bennée @ 2020-09-09 11:27 UTC (permalink / raw)
To: qemu-devel
Cc: fam, berrange, Alex Bennée, richard.henderson, f4bug,
Philippe Mathieu-Daudé, cota, Gerd Hoffmann, aurelien
From: Gerd Hoffmann <kraxel@redhat.com>
Fixes build failures with old kernels (USBDEVFS_GET_SPEED missing),
on the assumtion that distros with old kernels also have old libusb.
Reported-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200902081445.3291-1-kraxel@redhat.com>
Message-Id: <20200903112107.27367-5-alex.bennee@linaro.org>
---
hw/usb/host-libusb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 08604f787fd..c5d38cb09c0 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -942,7 +942,7 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev, int hostfd)
usb_host_ep_update(s);
libusb_speed = libusb_get_device_speed(dev);
-#ifdef CONFIG_LINUX
+#if LIBUSB_API_VERSION >= 0x01000107 && defined(CONFIG_LINUX)
if (hostfd && libusb_speed == 0) {
/*
* Workaround libusb bug: libusb_get_device_speed() does not
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 03/10] tests/meson.build: fp tests don't need CONFIG_TCG
2020-09-09 11:27 [PATCH v2 00/10] testing and misc updates Alex Bennée
2020-09-09 11:27 ` [PATCH v2 01/10] CODING_STYLE.rst: flesh out our naming conventions Alex Bennée
2020-09-09 11:27 ` [PATCH v2 02/10] usb-host: restrict workaround to new libusb versions Alex Bennée
@ 2020-09-09 11:27 ` Alex Bennée
2020-09-09 13:03 ` Philippe Mathieu-Daudé
2020-09-09 11:27 ` [PATCH v2 04/10] target/mips: simplify gen_compute_imm_branch logic Alex Bennée
` (6 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Alex Bennée @ 2020-09-09 11:27 UTC (permalink / raw)
To: qemu-devel
Cc: fam, berrange, Alex Bennée, richard.henderson, f4bug,
Philippe Mathieu-Daudé, cota, aurelien
As the tests build only softfloat.c no actual TCG machinary is neede
to test them (as is evidenced by GCC check-softfloat). Might as well
fix the wording on Travis while at it.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200903112107.27367-7-alex.bennee@linaro.org>
---
.travis.yml | 2 +-
tests/meson.build | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index 1d0ade0a133..65341634d02 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -138,7 +138,7 @@ jobs:
# Just build tools and run minimal unit and softfloat checks
- - name: "GCC check-softfloat (user)"
+ - name: "GCC check-unit and check-softfloat"
env:
- BASE_CONFIG="--enable-tools"
- CONFIG="--disable-user --disable-system"
diff --git a/tests/meson.build b/tests/meson.build
index 998e4c48f90..dae8a77df14 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -259,8 +259,9 @@ test('decodetree', sh,
workdir: meson.current_source_dir() / 'decode',
suite: 'decodetree')
+subdir('fp')
+
if 'CONFIG_TCG' in config_host
- subdir('fp')
if 'CONFIG_PLUGIN' in config_host
subdir('plugin')
endif
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 03/10] tests/meson.build: fp tests don't need CONFIG_TCG
2020-09-09 11:27 ` [PATCH v2 03/10] tests/meson.build: fp tests don't need CONFIG_TCG Alex Bennée
@ 2020-09-09 13:03 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-09 13:03 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: fam, berrange, Philippe Mathieu-Daudé, richard.henderson,
cota, aurelien
On 9/9/20 1:27 PM, Alex Bennée wrote:
> As the tests build only softfloat.c no actual TCG machinary is neede
Typo "needed".
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> to test them (as is evidenced by GCC check-softfloat). Might as well
> fix the wording on Travis while at it.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20200903112107.27367-7-alex.bennee@linaro.org>
> ---
> .travis.yml | 2 +-
> tests/meson.build | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 1d0ade0a133..65341634d02 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -138,7 +138,7 @@ jobs:
>
>
> # Just build tools and run minimal unit and softfloat checks
> - - name: "GCC check-softfloat (user)"
> + - name: "GCC check-unit and check-softfloat"
> env:
> - BASE_CONFIG="--enable-tools"
> - CONFIG="--disable-user --disable-system"
> diff --git a/tests/meson.build b/tests/meson.build
> index 998e4c48f90..dae8a77df14 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -259,8 +259,9 @@ test('decodetree', sh,
> workdir: meson.current_source_dir() / 'decode',
> suite: 'decodetree')
>
> +subdir('fp')
> +
> if 'CONFIG_TCG' in config_host
> - subdir('fp')
> if 'CONFIG_PLUGIN' in config_host
> subdir('plugin')
> endif
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 04/10] target/mips: simplify gen_compute_imm_branch logic
2020-09-09 11:27 [PATCH v2 00/10] testing and misc updates Alex Bennée
` (2 preceding siblings ...)
2020-09-09 11:27 ` [PATCH v2 03/10] tests/meson.build: fp tests don't need CONFIG_TCG Alex Bennée
@ 2020-09-09 11:27 ` Alex Bennée
2020-09-09 11:27 ` [PATCH v2 05/10] docs/system/deprecated: mark ppc64abi32-linux-user for deprecation Alex Bennée
` (5 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Alex Bennée @ 2020-09-09 11:27 UTC (permalink / raw)
To: qemu-devel
Cc: fam, Aleksandar Rikalo, berrange, Alex Bennée,
richard.henderson, f4bug, Jiaxun Yang, Aleksandar Markovic, cota,
aurelien
One of the Travis builds was complaining about:
qemu/include/tcg/tcg.h:437:12: error: ‘cond’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
return (TCGCond)(c ^ 1);
../target/mips/translate.c:20031:13: note: ‘cond’ was declared here
TCGCond cond;
Rather than figure out exactly which one was causing the complaint I
just defaulted to TCG_COND_ALWAYS and allowed that state to double up
for the now defunct bcond_compute variable.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200903112107.27367-8-alex.bennee@linaro.org>
---
target/mips/translate.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/target/mips/translate.c b/target/mips/translate.c
index 899b90ae0ff..398edf72898 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20028,8 +20028,7 @@ static void gen_pool32axf_nanomips_insn(CPUMIPSState *env, DisasContext *ctx)
static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
int rt, int32_t imm, int32_t offset)
{
- TCGCond cond;
- int bcond_compute = 0;
+ TCGCond cond = TCG_COND_ALWAYS;
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
@@ -20046,7 +20045,6 @@ static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
/* Treat as NOP */
goto out;
} else {
- bcond_compute = 1;
cond = TCG_COND_EQ;
}
break;
@@ -20065,7 +20063,6 @@ static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
tcg_gen_shri_tl(t0, t0, imm);
tcg_gen_andi_tl(t0, t0, 1);
tcg_gen_movi_tl(t1, 0);
- bcond_compute = 1;
if (opc == NM_BBEQZC) {
cond = TCG_COND_EQ;
} else {
@@ -20080,7 +20077,6 @@ static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
} else if (rt == 0 && imm != 0) {
/* Unconditional branch */
} else {
- bcond_compute = 1;
cond = TCG_COND_NE;
}
break;
@@ -20088,24 +20084,20 @@ static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
if (rt == 0 && imm == 0) {
/* Unconditional branch */
} else {
- bcond_compute = 1;
cond = TCG_COND_GE;
}
break;
case NM_BLTIC:
- bcond_compute = 1;
cond = TCG_COND_LT;
break;
case NM_BGEIUC:
if (rt == 0 && imm == 0) {
/* Unconditional branch */
} else {
- bcond_compute = 1;
cond = TCG_COND_GEU;
}
break;
case NM_BLTIUC:
- bcond_compute = 1;
cond = TCG_COND_LTU;
break;
default:
@@ -20118,7 +20110,7 @@ static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
clear_branch_hflags(ctx);
ctx->base.is_jmp = DISAS_NORETURN;
- if (bcond_compute == 0) {
+ if (cond == TCG_COND_ALWAYS) {
/* Uncoditional compact branch */
gen_goto_tb(ctx, 0, ctx->btarget);
} else {
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 05/10] docs/system/deprecated: mark ppc64abi32-linux-user for deprecation
2020-09-09 11:27 [PATCH v2 00/10] testing and misc updates Alex Bennée
` (3 preceding siblings ...)
2020-09-09 11:27 ` [PATCH v2 04/10] target/mips: simplify gen_compute_imm_branch logic Alex Bennée
@ 2020-09-09 11:27 ` Alex Bennée
2020-09-09 12:59 ` Philippe Mathieu-Daudé
2020-09-09 11:27 ` [PATCH v2 06/10] configure: don't enable ppc64abi32-linux-user by default Alex Bennée
` (4 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Alex Bennée @ 2020-09-09 11:27 UTC (permalink / raw)
To: qemu-devel
Cc: fam, berrange, reviewer:Incompatible changes, Alex Bennée,
richard.henderson, f4bug, cota, aurelien, David Gibson
It's buggy and we are not sure anyone uses it.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
---
docs/system/deprecated.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 851dbdeb8ab..a158e765c33 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -424,6 +424,13 @@ linux-user mode) is deprecated and will be removed in a future version
of QEMU. Support for this CPU was removed from the upstream Linux
kernel in 2018, and has also been dropped from glibc.
+``ppc64abi32`` CPUs (since 5.2.0)
+'''''''''''''''''''''''''''''''''
+
+The ``ppc64abi32`` architecture has a number of issues which regularly
+trip up our CI testing and is suspected to be quite broken. For that
+reason the maintainers strongly suspect no one actually uses it.
+
Related binaries
----------------
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 05/10] docs/system/deprecated: mark ppc64abi32-linux-user for deprecation
2020-09-09 11:27 ` [PATCH v2 05/10] docs/system/deprecated: mark ppc64abi32-linux-user for deprecation Alex Bennée
@ 2020-09-09 12:59 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-09 12:59 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: fam, berrange, reviewer:Incompatible changes, richard.henderson,
cota, aurelien, David Gibson
On 9/9/20 1:27 PM, Alex Bennée wrote:
> It's buggy and we are not sure anyone uses it.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> docs/system/deprecated.rst | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 851dbdeb8ab..a158e765c33 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -424,6 +424,13 @@ linux-user mode) is deprecated and will be removed in a future version
> of QEMU. Support for this CPU was removed from the upstream Linux
> kernel in 2018, and has also been dropped from glibc.
>
> +``ppc64abi32`` CPUs (since 5.2.0)
> +'''''''''''''''''''''''''''''''''
> +
> +The ``ppc64abi32`` architecture has a number of issues which regularly
> +trip up our CI testing and is suspected to be quite broken. For that
> +reason the maintainers strongly suspect no one actually uses it.
> +
> Related binaries
> ----------------
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 06/10] configure: don't enable ppc64abi32-linux-user by default
2020-09-09 11:27 [PATCH v2 00/10] testing and misc updates Alex Bennée
` (4 preceding siblings ...)
2020-09-09 11:27 ` [PATCH v2 05/10] docs/system/deprecated: mark ppc64abi32-linux-user for deprecation Alex Bennée
@ 2020-09-09 11:27 ` Alex Bennée
2020-09-09 12:57 ` Philippe Mathieu-Daudé
2020-09-09 11:27 ` [PATCH v2 07/10] hw/i386: make explicit clearing of pch_rev_id Alex Bennée
` (3 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Alex Bennée @ 2020-09-09 11:27 UTC (permalink / raw)
To: qemu-devel
Cc: fam, berrange, Alex Bennée, richard.henderson, f4bug, cota,
aurelien
The user can still enable this explicitly but they will get a warning
at the end of configure for their troubles. This also drops any builds
of ppc64abi32 from our CI tests.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
configure | 46 +++++++++++++++++++++++++++-------------------
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/configure b/configure
index 4231d56bcc0..2b5492a0d63 100755
--- a/configure
+++ b/configure
@@ -542,6 +542,8 @@ gettext=""
bogus_os="no"
malloc_trim=""
+deprecated_features=""
+
# parse CC options first
for opt do
optarg=$(expr "x$opt" : 'x[^=]*=\(.*\)')
@@ -1720,26 +1722,25 @@ if [ "$bsd_user" = "yes" ]; then
mak_wilds="${mak_wilds} $source_path/default-configs/*-bsd-user.mak"
fi
-if test -z "$target_list_exclude"; then
- for config in $mak_wilds; do
- default_target_list="${default_target_list} $(basename "$config" .mak)"
- done
-else
- exclude_list=$(echo "$target_list_exclude" | sed -e 's/,/ /g')
- for config in $mak_wilds; do
- target="$(basename "$config" .mak)"
- exclude="no"
- for excl in $exclude_list; do
- if test "$excl" = "$target"; then
- exclude="yes"
- break;
- fi
- done
- if test "$exclude" = "no"; then
- default_target_list="${default_target_list} $target"
+if test -z "$target_list_exclude" -a -z "$target_list"; then
+ # if the user doesn't specify anything lets skip deprecating stuff
+ target_list_exclude=ppc64abi32-linux-user
+fi
+
+exclude_list=$(echo "$target_list_exclude" | sed -e 's/,/ /g')
+for config in $mak_wilds; do
+ target="$(basename "$config" .mak)"
+ exclude="no"
+ for excl in $exclude_list; do
+ if test "$excl" = "$target"; then
+ exclude="yes"
+ break;
fi
done
-fi
+ if test "$exclude" = "no"; then
+ default_target_list="${default_target_list} $target"
+ fi
+done
# Enumerate public trace backends for --help output
trace_backend_list=$(echo $(grep -le '^PUBLIC = True$' "$source_path"/scripts/tracetool/backend/*.py | sed -e 's/^.*\/\(.*\)\.py$/\1/'))
@@ -7557,7 +7558,7 @@ TARGET_SYSTBL=""
case "$target_name" in
i386)
mttcg="yes"
- gdb_xml_files="i386-32bit.xml"
+ gdb_xml_files="i386-32bit.xml"
TARGET_SYSTBL_ABI=i386
TARGET_SYSTBL=syscall_32.tbl
;;
@@ -7667,6 +7668,7 @@ case "$target_name" in
TARGET_SYSTBL_ABI=common,nospu,32
echo "TARGET_ABI32=y" >> $config_target_mak
gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml power-vsx.xml"
+ deprecated_features="ppc64abi32 ${deprecated_features}"
;;
riscv32)
TARGET_BASE_ARCH=riscv
@@ -8011,6 +8013,12 @@ fi
touch ninjatool.stamp
fi
+if test -n "${deprecated_features}"; then
+ echo "Warning, deprecated features enabled."
+ echo "Please see docs/system/deprecated.rst"
+ echo " features: ${deprecated_features}"
+fi
+
# Save the configure command line for later reuse.
cat <<EOD >config.status
#!/bin/sh
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 06/10] configure: don't enable ppc64abi32-linux-user by default
2020-09-09 11:27 ` [PATCH v2 06/10] configure: don't enable ppc64abi32-linux-user by default Alex Bennée
@ 2020-09-09 12:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-09 12:57 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: fam, berrange, richard.henderson, cota, qemu-ppc, aurelien
Cc'ing qemu-ppc@
On 9/9/20 1:27 PM, Alex Bennée wrote:
> The user can still enable this explicitly but they will get a warning
> at the end of configure for their troubles. This also drops any builds
> of ppc64abi32 from our CI tests.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> configure | 46 +++++++++++++++++++++++++++-------------------
> 1 file changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/configure b/configure
> index 4231d56bcc0..2b5492a0d63 100755
> --- a/configure
> +++ b/configure
> @@ -542,6 +542,8 @@ gettext=""
> bogus_os="no"
> malloc_trim=""
>
> +deprecated_features=""
> +
> # parse CC options first
> for opt do
> optarg=$(expr "x$opt" : 'x[^=]*=\(.*\)')
> @@ -1720,26 +1722,25 @@ if [ "$bsd_user" = "yes" ]; then
> mak_wilds="${mak_wilds} $source_path/default-configs/*-bsd-user.mak"
> fi
>
> -if test -z "$target_list_exclude"; then
> - for config in $mak_wilds; do
> - default_target_list="${default_target_list} $(basename "$config" .mak)"
> - done
> -else
> - exclude_list=$(echo "$target_list_exclude" | sed -e 's/,/ /g')
> - for config in $mak_wilds; do
> - target="$(basename "$config" .mak)"
> - exclude="no"
> - for excl in $exclude_list; do
> - if test "$excl" = "$target"; then
> - exclude="yes"
> - break;
> - fi
> - done
> - if test "$exclude" = "no"; then
> - default_target_list="${default_target_list} $target"
> +if test -z "$target_list_exclude" -a -z "$target_list"; then
> + # if the user doesn't specify anything lets skip deprecating stuff
> + target_list_exclude=ppc64abi32-linux-user
> +fi
> +
> +exclude_list=$(echo "$target_list_exclude" | sed -e 's/,/ /g')
> +for config in $mak_wilds; do
> + target="$(basename "$config" .mak)"
> + exclude="no"
> + for excl in $exclude_list; do
> + if test "$excl" = "$target"; then
> + exclude="yes"
> + break;
> fi
> done
> -fi
> + if test "$exclude" = "no"; then
> + default_target_list="${default_target_list} $target"
> + fi
> +done
>
> # Enumerate public trace backends for --help output
> trace_backend_list=$(echo $(grep -le '^PUBLIC = True$' "$source_path"/scripts/tracetool/backend/*.py | sed -e 's/^.*\/\(.*\)\.py$/\1/'))
> @@ -7557,7 +7558,7 @@ TARGET_SYSTBL=""
> case "$target_name" in
> i386)
> mttcg="yes"
> - gdb_xml_files="i386-32bit.xml"
> + gdb_xml_files="i386-32bit.xml"
> TARGET_SYSTBL_ABI=i386
> TARGET_SYSTBL=syscall_32.tbl
> ;;
> @@ -7667,6 +7668,7 @@ case "$target_name" in
> TARGET_SYSTBL_ABI=common,nospu,32
> echo "TARGET_ABI32=y" >> $config_target_mak
> gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml power-spe.xml power-vsx.xml"
> + deprecated_features="ppc64abi32 ${deprecated_features}"
> ;;
> riscv32)
> TARGET_BASE_ARCH=riscv
> @@ -8011,6 +8013,12 @@ fi
> touch ninjatool.stamp
> fi
>
> +if test -n "${deprecated_features}"; then
> + echo "Warning, deprecated features enabled."
> + echo "Please see docs/system/deprecated.rst"
> + echo " features: ${deprecated_features}"
> +fi
Nice!
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> +
> # Save the configure command line for later reuse.
> cat <<EOD >config.status
> #!/bin/sh
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 07/10] hw/i386: make explicit clearing of pch_rev_id
2020-09-09 11:27 [PATCH v2 00/10] testing and misc updates Alex Bennée
` (5 preceding siblings ...)
2020-09-09 11:27 ` [PATCH v2 06/10] configure: don't enable ppc64abi32-linux-user by default Alex Bennée
@ 2020-09-09 11:27 ` Alex Bennée
2020-09-09 12:58 ` Philippe Mathieu-Daudé
2020-09-09 11:27 ` [PATCH v2 08/10] tests: bump avocado version Alex Bennée
` (2 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Alex Bennée @ 2020-09-09 11:27 UTC (permalink / raw)
To: qemu-devel
Cc: fam, berrange, Eduardo Habkost, Michael S. Tsirkin,
Alex Bennée, richard.henderson, f4bug, cota, Paolo Bonzini,
aurelien, Richard Henderson
Some compilers (notably the Xenial gcc in Travis) fail to spot that
this will always be set if pch_dev_id != 0xffff. Given this is setup
code and using _Pragma to override is equally as ugly lets just remove
the doubt from the compilers mind.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
hw/i386/pc_piix.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 32b1453e6a8..33fa035fb7f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -930,7 +930,7 @@ void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id)
struct PCIDevice *bridge_dev;
int i, num;
uint16_t pch_dev_id = 0xffff;
- uint8_t pch_rev_id;
+ uint8_t pch_rev_id = 0;
num = ARRAY_SIZE(igd_combo_id_infos);
for (i = 0; i < num; i++) {
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 07/10] hw/i386: make explicit clearing of pch_rev_id
2020-09-09 11:27 ` [PATCH v2 07/10] hw/i386: make explicit clearing of pch_rev_id Alex Bennée
@ 2020-09-09 12:58 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-09 12:58 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: fam, berrange, Eduardo Habkost, Michael S. Tsirkin,
richard.henderson, cota, Paolo Bonzini, aurelien,
Richard Henderson
On 9/9/20 1:27 PM, Alex Bennée wrote:
> Some compilers (notably the Xenial gcc in Travis) fail to spot that
> this will always be set if pch_dev_id != 0xffff. Given this is setup
> code and using _Pragma to override is equally as ugly lets just remove
> the doubt from the compilers mind.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> hw/i386/pc_piix.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 32b1453e6a8..33fa035fb7f 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -930,7 +930,7 @@ void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id)
> struct PCIDevice *bridge_dev;
> int i, num;
> uint16_t pch_dev_id = 0xffff;
> - uint8_t pch_rev_id;
> + uint8_t pch_rev_id = 0;
>
> num = ARRAY_SIZE(igd_combo_id_infos);
> for (i = 0; i < num; i++) {
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 08/10] tests: bump avocado version
2020-09-09 11:27 [PATCH v2 00/10] testing and misc updates Alex Bennée
` (6 preceding siblings ...)
2020-09-09 11:27 ` [PATCH v2 07/10] hw/i386: make explicit clearing of pch_rev_id Alex Bennée
@ 2020-09-09 11:27 ` Alex Bennée
2020-09-09 11:27 ` [PATCH v2 09/10] tests/acceptance: Add Test.fetch_asset(cancel_on_missing=True) Alex Bennée
2020-09-09 11:27 ` [PATCH v2 10/10] plugins: move the more involved plugins to contrib Alex Bennée
9 siblings, 0 replies; 17+ messages in thread
From: Alex Bennée @ 2020-09-09 11:27 UTC (permalink / raw)
To: qemu-devel
Cc: fam, berrange, Alex Bennée, Philippe Mathieu-Daudé,
richard.henderson, f4bug, Willian Rampazzo, Pavel Dovgalyuk, cota,
Cleber Rosa, Pavel Dovgalyuk, aurelien
From: Pavel Dovgalyuk <Pavel.Dovgaluk@gmail.com>
Reverse debugging test uses gdb remote client of avocado framework.
This client was fixed since the currently used version 76.
Therefore this patch bumps the version to 81 and fixes command
line version compatibility issue.
Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
Message-Id: <159903462803.28509.16851113546106095750.stgit@pasha-ThinkPad-X280>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200908202352.298506-2-philmd@redhat.com>
---
tests/Makefile.include | 2 +-
tests/requirements.txt | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/Makefile.include b/tests/Makefile.include
index f93e6112202..2baebc179e7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -127,7 +127,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
--show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
--filter-by-tags-include-empty --filter-by-tags-include-empty-key \
$(AVOCADO_TAGS) \
- $(if $(GITLAB_CI),,--failfast=on) tests/acceptance, \
+ $(if $(GITLAB_CI),,--failfast) tests/acceptance, \
"AVOCADO", "tests/acceptance")
# Consolidated targets
diff --git a/tests/requirements.txt b/tests/requirements.txt
index f9c84b4ba1a..036691c9221 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -1,5 +1,5 @@
# Add Python module requirements, one per line, to be installed
# in the tests/venv Python virtual environment. For more info,
# refer to: https://pip.pypa.io/en/stable/user_guide/#id1
-avocado-framework==76.0
+avocado-framework==81.0
pycdlib==1.9.0
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 09/10] tests/acceptance: Add Test.fetch_asset(cancel_on_missing=True)
2020-09-09 11:27 [PATCH v2 00/10] testing and misc updates Alex Bennée
` (7 preceding siblings ...)
2020-09-09 11:27 ` [PATCH v2 08/10] tests: bump avocado version Alex Bennée
@ 2020-09-09 11:27 ` Alex Bennée
2020-09-09 11:27 ` [PATCH v2 10/10] plugins: move the more involved plugins to contrib Alex Bennée
9 siblings, 0 replies; 17+ messages in thread
From: Alex Bennée @ 2020-09-09 11:27 UTC (permalink / raw)
To: qemu-devel
Cc: fam, berrange, Philippe Mathieu-Daudé, richard.henderson,
f4bug, Wainer dos Santos Moschetta, Alex Bennée, cota,
Cleber Rosa, aurelien
From: Philippe Mathieu-Daudé <philmd@redhat.com>
Invert the default of avocado.Test.fetch_asset 'cancel_on_missing'
keyword: accept missing artefacts by default. If a test is certain
an artifact can't be missing, it will set cancel_on_missing=False.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200908202352.298506-3-philmd@redhat.com>
---
tests/acceptance/avocado_qemu/__init__.py | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index db9c0f5d79d..4cda0371879 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -192,3 +192,15 @@ class Test(avocado.Test):
def tearDown(self):
for vm in self._vms.values():
vm.shutdown()
+
+ def fetch_asset(self, name,
+ asset_hash=None, algorithm=None,
+ locations=None, expire=None,
+ find_only=False, cancel_on_missing=True):
+ return super(Test, self).fetch_asset(name,
+ asset_hash=asset_hash,
+ algorithm=algorithm,
+ locations=locations,
+ expire=expire,
+ find_only=find_only,
+ cancel_on_missing=cancel_on_missing)
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 10/10] plugins: move the more involved plugins to contrib
2020-09-09 11:27 [PATCH v2 00/10] testing and misc updates Alex Bennée
` (8 preceding siblings ...)
2020-09-09 11:27 ` [PATCH v2 09/10] tests/acceptance: Add Test.fetch_asset(cancel_on_missing=True) Alex Bennée
@ 2020-09-09 11:27 ` Alex Bennée
2020-09-09 13:57 ` Thomas Huth
9 siblings, 1 reply; 17+ messages in thread
From: Alex Bennée @ 2020-09-09 11:27 UTC (permalink / raw)
To: qemu-devel
Cc: fam, berrange, Alex Bennée, richard.henderson, f4bug, cota,
aurelien
We have an exploding complexity problem in the testing so lets just
move the more involved plugins into contrib. tests/plugins still exist
for the basic plugins that exercise the API. We restore the old
pre-meson style Makefile for contrib as it also doubles as a guide for
out-of-tree plugin builds.
While we are at it add some examples to the documentation and a
specific plugins build target.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200904134007.4175-4-alex.bennee@linaro.org>
---
v4
- expand MAINTAINERS
---
docs/devel/tcg-plugins.rst | 142 ++++++++++++++++++
configure | 2 +
Makefile | 11 ++
{tests/plugin => contrib/plugins}/hotblocks.c | 0
{tests/plugin => contrib/plugins}/hotpages.c | 0
{tests/plugin => contrib/plugins}/howvec.c | 0
{tests/plugin => contrib/plugins}/lockstep.c | 0
MAINTAINERS | 3 +-
contrib/plugins/Makefile | 42 ++++++
tests/Makefile.include | 2 +-
tests/plugin/meson.build | 4 +-
tests/tcg/Makefile.target | 3 +-
12 files changed, 203 insertions(+), 6 deletions(-)
rename {tests/plugin => contrib/plugins}/hotblocks.c (100%)
rename {tests/plugin => contrib/plugins}/hotpages.c (100%)
rename {tests/plugin => contrib/plugins}/howvec.c (100%)
rename {tests/plugin => contrib/plugins}/lockstep.c (100%)
create mode 100644 contrib/plugins/Makefile
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index a05990906cc..e079695caf9 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -134,3 +134,145 @@ longer want to instrument the code. This operation is asynchronous
which means callbacks may still occur after the uninstall operation is
requested. The plugin isn't completely uninstalled until the safe work
has executed while all vCPUs are quiescent.
+
+Example Plugins
+===============
+
+There are a number of plugins included with QEMU and you are
+encouraged to contribute them upstream. There is a `contrib/plugins`
+directory where they can go.
+
+- tests/plugins
+
+These are some basic plugins that are used to test and exercise the
+API during the `make check-tcg` target.
+
+- contrib/plugins/hotblocks.c
+
+The hotblocks plugin allows you to examine the where hot paths of
+execution are in your program. Once the program has finished you will
+get a sorted list of blocks reporting the starting PC, translation
+count, number of instructions and execution count. This will work best
+with linux-user execution as system emulation tends to generate
+re-translations as blocks from different programs get swapped in and
+out of system memory.
+
+If your program is single-threaded you can use the `inline` option for
+slightly faster (but not thread safe) counters.
+
+Example::
+
+ ./aarch64-linux-user/qemu-aarch64 -plugin contrib/plugins/libhotblocks.so -d plugin ./tests/tcg/aarch64-linux-user/sha1
+ SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6
+ collected 903 entries in the hash table
+ pc, tcount, icount, ecount
+ 0x0000000041ed10, 1, 5, 66087
+ 0x000000004002b0, 1, 4, 66087
+ ...
+
+- contrib/plugins/hotpages.c
+
+Similar to hotblocks but this time tracks memory accesses::
+
+ ./aarch64-linux-user/qemu-aarch64 -plugin contrib/plugins/libhotpages.so -d plugin ./tests/tcg/aarch64-linux-user/sha1
+ SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6
+ Addr, RCPUs, Reads, WCPUs, Writes
+ 0x000055007fe000, 0x0001, 31747952, 0x0001, 8835161
+ 0x000055007ff000, 0x0001, 29001054, 0x0001, 8780625
+ 0x00005500800000, 0x0001, 687465, 0x0001, 335857
+ 0x0000000048b000, 0x0001, 130594, 0x0001, 355
+ 0x0000000048a000, 0x0001, 1826, 0x0001, 11
+
+- contrib/plugins/howvec.c
+
+This is an instruction classifier so can be used to count different
+types of instructions. It has a number of options to refine which get
+counted. You can give an argument for a class of instructions to break
+it down fully, so for example to see all the system registers
+accesses::
+
+ ./aarch64-softmmu/qemu-system-aarch64 $(QEMU_ARGS) \
+ -append "root=/dev/sda2 systemd.unit=benchmark.service" \
+ -smp 4 -plugin ./contrib/plugins/libhowvec.so,arg=sreg -d plugin
+
+which will lead to a sorted list after the class breakdown::
+
+ Instruction Classes:
+ Class: UDEF not counted
+ Class: SVE (68 hits)
+ Class: PCrel addr (47789483 hits)
+ Class: Add/Sub (imm) (192817388 hits)
+ Class: Logical (imm) (93852565 hits)
+ Class: Move Wide (imm) (76398116 hits)
+ Class: Bitfield (44706084 hits)
+ Class: Extract (5499257 hits)
+ Class: Cond Branch (imm) (147202932 hits)
+ Class: Exception Gen (193581 hits)
+ Class: NOP not counted
+ Class: Hints (6652291 hits)
+ Class: Barriers (8001661 hits)
+ Class: PSTATE (1801695 hits)
+ Class: System Insn (6385349 hits)
+ Class: System Reg counted individually
+ Class: Branch (reg) (69497127 hits)
+ Class: Branch (imm) (84393665 hits)
+ Class: Cmp & Branch (110929659 hits)
+ Class: Tst & Branch (44681442 hits)
+ Class: AdvSimd ldstmult (736 hits)
+ Class: ldst excl (9098783 hits)
+ Class: Load Reg (lit) (87189424 hits)
+ Class: ldst noalloc pair (3264433 hits)
+ Class: ldst pair (412526434 hits)
+ Class: ldst reg (imm) (314734576 hits)
+ Class: Loads & Stores (2117774 hits)
+ Class: Data Proc Reg (223519077 hits)
+ Class: Scalar FP (31657954 hits)
+ Individual Instructions:
+ Instr: mrs x0, sp_el0 (2682661 hits) (op=0xd5384100/ System Reg)
+ Instr: mrs x1, tpidr_el2 (1789339 hits) (op=0xd53cd041/ System Reg)
+ Instr: mrs x2, tpidr_el2 (1513494 hits) (op=0xd53cd042/ System Reg)
+ Instr: mrs x0, tpidr_el2 (1490823 hits) (op=0xd53cd040/ System Reg)
+ Instr: mrs x1, sp_el0 (933793 hits) (op=0xd5384101/ System Reg)
+ Instr: mrs x2, sp_el0 (699516 hits) (op=0xd5384102/ System Reg)
+ Instr: mrs x4, tpidr_el2 (528437 hits) (op=0xd53cd044/ System Reg)
+ Instr: mrs x30, ttbr1_el1 (480776 hits) (op=0xd538203e/ System Reg)
+ Instr: msr ttbr1_el1, x30 (480713 hits) (op=0xd518203e/ System Reg)
+ Instr: msr vbar_el1, x30 (480671 hits) (op=0xd518c01e/ System Reg)
+ ...
+
+To find the argument shorthand for the class you need to examine the
+source code of the plugin at the moment, specifically the `*opt`
+argument in the InsnClassExecCount tables.
+
+- contrib/plugins/lockstep.c
+
+This is a debugging tool for developers who want to find out when and
+where execution diverges after a subtle change to TCG code generation.
+It is not an exact science and results are likely to be mixed once
+asynchronous events are introduced. While the use of -icount can
+introduce determinism to the execution flow it doesn't always follow
+the translation sequence will be exactly the same. Typically this is
+caused by a timer firing to service the GUI causing a block to end
+early. However in some cases it has proved to be useful in pointing
+people at roughly where execution diverges. The only argument you need
+for the plugin is a path for the socket the two instances will
+communicate over::
+
+
+ ./sparc-softmmu/qemu-system-sparc -monitor none -parallel none \
+ -net none -M SS-20 -m 256 -kernel day11/zImage.elf \
+ -plugin ./contrib/plugins/liblockstep.so,arg=lockstep-sparc.sock \
+ -d plugin,nochain
+
+which will eventually report::
+
+ qemu-system-sparc: warning: nic lance.0 has no peer
+ @ 0x000000ffd06678 vs 0x000000ffd001e0 (2/1 since last)
+ @ 0x000000ffd07d9c vs 0x000000ffd06678 (3/1 since last)
+ Δ insn_count @ 0x000000ffd07d9c (809900609) vs 0x000000ffd06678 (809900612)
+ previously @ 0x000000ffd06678/10 (809900609 insns)
+ previously @ 0x000000ffd001e0/4 (809900599 insns)
+ previously @ 0x000000ffd080ac/2 (809900595 insns)
+ previously @ 0x000000ffd08098/5 (809900593 insns)
+ previously @ 0x000000ffd080c0/1 (809900588 insns)
+
diff --git a/configure b/configure
index 2b5492a0d63..2b6a1196da5 100755
--- a/configure
+++ b/configure
@@ -7855,6 +7855,7 @@ DIRS="$DIRS tests/qtest tests/qemu-iotests tests/vm tests/fp tests/qgraph"
DIRS="$DIRS docs docs/interop fsdev scsi"
DIRS="$DIRS pc-bios/optionrom pc-bios/s390-ccw"
DIRS="$DIRS roms/seabios"
+DIRS="$DIRS contrib/plugins/"
LINKS="Makefile"
LINKS="$LINKS tests/tcg/lm32/Makefile"
LINKS="$LINKS tests/tcg/Makefile.target"
@@ -7866,6 +7867,7 @@ LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
LINKS="$LINKS tests/acceptance tests/data"
LINKS="$LINKS tests/qemu-iotests/check"
LINKS="$LINKS python"
+LINKS="$LINKS contrib/plugins/Makefile "
UNLINK="pc-bios/keymaps"
for bios_file in \
$source_path/pc-bios/*.bin \
diff --git a/Makefile b/Makefile
index d6c5c9fdef1..b63f7dce529 100644
--- a/Makefile
+++ b/Makefile
@@ -106,6 +106,12 @@ config-host.mak: $(SRC_PATH)/configure $(SRC_PATH)/pc-bios $(SRC_PATH)/VERSION
# Force configure to re-run if the API symbols are updated
ifeq ($(CONFIG_PLUGIN),y)
config-host.mak: $(SRC_PATH)/plugins/qemu-plugins.symbols
+
+.PHONY: plugins
+plugins:
+ $(call quiet-command,\
+ $(MAKE) $(SUBDIR_MAKEFLAGS) -C contrib/plugins V="$(V)", \
+ "BUILD", "example plugins")
endif
else
@@ -256,6 +262,11 @@ help:
$(call print-help,cscope,Generate cscope index)
$(call print-help,sparse,Run sparse on the QEMU source)
@echo ''
+ifeq ($(CONFIG_PLUGIN),y)
+ @echo 'Plugin targets:'
+ $(call print-help,plugins,Build the example TCG plugins)
+ @echo ''
+endif
@echo 'Cleaning targets:'
$(call print-help,clean,Remove most generated files but keep the config)
$(call print-help,distclean,Remove all generated files)
diff --git a/tests/plugin/hotblocks.c b/contrib/plugins/hotblocks.c
similarity index 100%
rename from tests/plugin/hotblocks.c
rename to contrib/plugins/hotblocks.c
diff --git a/tests/plugin/hotpages.c b/contrib/plugins/hotpages.c
similarity index 100%
rename from tests/plugin/hotpages.c
rename to contrib/plugins/hotpages.c
diff --git a/tests/plugin/howvec.c b/contrib/plugins/howvec.c
similarity index 100%
rename from tests/plugin/howvec.c
rename to contrib/plugins/howvec.c
diff --git a/tests/plugin/lockstep.c b/contrib/plugins/lockstep.c
similarity index 100%
rename from tests/plugin/lockstep.c
rename to contrib/plugins/lockstep.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 7d0a5e91e4f..018c4f94a93 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2745,7 +2745,8 @@ M: Alex Bennée <alex.bennee@linaro.org>
S: Maintained
F: docs/devel/tcg-plugins.rst
F: plugins/
-F: tests/plugin
+F: tests/plugin/
+F: contrib/plugins/
AArch64 TCG target
M: Richard Henderson <richard.henderson@linaro.org>
diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
new file mode 100644
index 00000000000..7801b08b0d6
--- /dev/null
+++ b/contrib/plugins/Makefile
@@ -0,0 +1,42 @@
+# -*- Mode: makefile -*-
+#
+# This Makefile example is fairly independent from the main makefile
+# so users can take and adapt it for their build. We only really
+# include config-host.mak so we don't have to repeat probing for
+# cflags that the main configure has already done for us.
+#
+
+BUILD_DIR := $(CURDIR)/../..
+
+include $(BUILD_DIR)/config-host.mak
+
+VPATH += $(SRC_PATH)/contrib/plugins
+
+NAMES :=
+NAMES += hotblocks
+NAMES += hotpages
+NAMES += howvec
+NAMES += lockstep
+
+SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
+
+# The main QEMU uses Glib extensively so it's perfectly fine to use it
+# in plugins (which many example do).
+CFLAGS = $(GLIB_CFLAGS)
+CFLAGS += -fPIC
+CFLAGS += $(if $(findstring no-psabi,$(QEMU_CFLAGS)),-Wpsabi)
+CFLAGS += -I$(SRC_PATH)/include/qemu
+
+all: $(SONAMES)
+
+%.o: %.c
+ $(CC) $(CFLAGS) -c -o $@ $<
+
+lib%.so: %.o
+ $(CC) -shared -Wl,-soname,$@ -o $@ $^ $(LDLIBS)
+
+clean:
+ rm -f *.o *.so *.d
+ rm -Rf .libs
+
+.PHONY: all clean
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 2baebc179e7..40d909badcb 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -50,7 +50,7 @@ RUN_TCG_TARGET_RULES=$(patsubst %,run-tcg-tests-%, $(TARGET_DIRS))
$(foreach PROBE_TARGET,$(TARGET_DIRS), \
$(eval -include $(SRC_PATH)/tests/tcg/Makefile.prereqs))
-build-tcg-tests-%: $(if $(CONFIG_PLUGIN),plugins)
+build-tcg-tests-%: $(if $(CONFIG_PLUGIN),test-plugins)
$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) \
-f $(SRC_PATH)/tests/tcg/Makefile.qemu \
SRC_PATH=$(SRC_PATH) \
diff --git a/tests/plugin/meson.build b/tests/plugin/meson.build
index dbbdcbaa670..1eacfa6e355 100644
--- a/tests/plugin/meson.build
+++ b/tests/plugin/meson.build
@@ -1,7 +1,7 @@
t = []
-foreach i : ['bb', 'empty', 'insn', 'mem', 'hotblocks', 'howvec', 'hotpages', 'lockstep']
+foreach i : ['bb', 'empty', 'insn', 'mem']
t += shared_module(i, files(i + '.c'),
include_directories: '../../include/qemu',
dependencies: glib)
endforeach
-alias_target('plugins', t)
+alias_target('test-plugins', t)
diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
index 4b2b696fcee..2ae86776cdc 100644
--- a/tests/tcg/Makefile.target
+++ b/tests/tcg/Makefile.target
@@ -129,8 +129,7 @@ ifeq ($(CONFIG_PLUGIN),y)
PLUGIN_SRC=$(SRC_PATH)/tests/plugin
PLUGIN_LIB=../../plugin
VPATH+=$(PLUGIN_LIB)
-PLUGINS=$(filter-out liblockstep.so,\
- $(patsubst %.c, lib%.so, $(notdir $(wildcard $(PLUGIN_SRC)/*.c))))
+PLUGINS=$(patsubst %.c, lib%.so, $(notdir $(wildcard $(PLUGIN_SRC)/*.c)))
# We need to ensure expand the run-plugin-TEST-with-PLUGIN
# pre-requistes manually here as we can't use stems to handle it. We
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 10/10] plugins: move the more involved plugins to contrib
2020-09-09 11:27 ` [PATCH v2 10/10] plugins: move the more involved plugins to contrib Alex Bennée
@ 2020-09-09 13:57 ` Thomas Huth
0 siblings, 0 replies; 17+ messages in thread
From: Thomas Huth @ 2020-09-09 13:57 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: fam, berrange, richard.henderson, f4bug, cota, aurelien
On 09/09/2020 13.27, Alex Bennée wrote:
> We have an exploding complexity problem in the testing so lets just
> move the more involved plugins into contrib. tests/plugins still exist
> for the basic plugins that exercise the API. We restore the old
> pre-meson style Makefile for contrib as it also doubles as a guide for
> out-of-tree plugin builds.
>
> While we are at it add some examples to the documentation and a
> specific plugins build target.
[...]
> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
> index a05990906cc..e079695caf9 100644
> --- a/docs/devel/tcg-plugins.rst
> +++ b/docs/devel/tcg-plugins.rst
> @@ -134,3 +134,145 @@ longer want to instrument the code. This operation is asynchronous
> which means callbacks may still occur after the uninstall operation is
> requested. The plugin isn't completely uninstalled until the safe work
> has executed while all vCPUs are quiescent.
> +
> +Example Plugins
> +===============
> +
> +There are a number of plugins included with QEMU and you are
> +encouraged to contribute them upstream.
s/contribute them/contribute your own plugins/ ?
> There is a `contrib/plugins`
> +directory where they can go.
> +
> +- tests/plugins
> +
> +These are some basic plugins that are used to test and exercise the
> +API during the `make check-tcg` target.
> +
> +- contrib/plugins/hotblocks.c
> +
> +The hotblocks plugin allows you to examine the where hot paths of
> +execution are in your program. Once the program has finished you will
> +get a sorted list of blocks reporting the starting PC, translation
> +count, number of instructions and execution count. This will work best
> +with linux-user execution as system emulation tends to generate
> +re-translations as blocks from different programs get swapped in and
> +out of system memory.
> +
> +If your program is single-threaded you can use the `inline` option for
> +slightly faster (but not thread safe) counters.
> +
> +Example::
> +
> + ./aarch64-linux-user/qemu-aarch64 -plugin contrib/plugins/libhotblocks.so -d plugin ./tests/tcg/aarch64-linux-user/sha1
Maybe break long lines with a backslash ?
> + SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6
> + collected 903 entries in the hash table
> + pc, tcount, icount, ecount
> + 0x0000000041ed10, 1, 5, 66087
> + 0x000000004002b0, 1, 4, 66087
> + ...
> +
> +- contrib/plugins/hotpages.c
> +
> +Similar to hotblocks but this time tracks memory accesses::
> +
> + ./aarch64-linux-user/qemu-aarch64 -plugin contrib/plugins/libhotpages.so -d plugin ./tests/tcg/aarch64-linux-user/sha1
dito ?
Thomas
^ permalink raw reply [flat|nested] 17+ messages in thread