Re: [PATCH v3 03/16] target/riscv: Restrict sysemu specific header to user emulation

[Daniel Henrique Barboza]

Phil,


This patch breaks the build for me. Applied on top of master:


FAILED: libqemu-riscv64-linux-user.fa.p/target_riscv_cpu.c.o
cc -m64 -mcx16 -Ilibqemu-riscv64-linux-user.fa.p -I. -I.. -Itarget/riscv -I../target/riscv 
-I../common-user/host/x86_64 -I../linux-user/include/host/x86_64 -I../linux-user/include 
-Ilinux-user -I../linux-user -I../linux-user/riscv -Iqapi -Itrace -Iui -Iui/shader 
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 
-fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong 
-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wundef -Wwrite-strings -Wmissing-prototypes 
-Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits 
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs 
-Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute 
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -isystem 
/home/danielhb/work/qemu/linux-headers -isystem linux-headers -iquote . -iquote 
/home/danielhb/work/qemu -iquote /home/danielhb/work/qemu/include -iquote 
/home/danielhb/work/qemu/host/include/x86_64 -iquote /home/danielhb/work/qemu/host/include/generic 
-iquote /home/danielhb/work/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
-D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -isystem../linux-headers 
-isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="riscv64-linux-user-config-target.h"' 
'-DCONFIG_DEVICES="riscv64-linux-user-config-devices.h"' -MD -MQ 
libqemu-riscv64-linux-user.fa.p/target_riscv_cpu.c.o -MF 
libqemu-riscv64-linux-user.fa.p/target_riscv_cpu.c.o.d -o 
libqemu-riscv64-linux-user.fa.p/target_riscv_cpu.c.o -c ../target/riscv/cpu.c
../target/riscv/cpu.c: In function ‘riscv_cpu_realize’:
../target/riscv/cpu.c:1461:9: error: implicit declaration of function 
‘tcg_enabled’ [-Werror=implicit-function-declaration]
 1461 |     if (tcg_enabled()) {
      |         ^~~~~~~~~~~
../target/riscv/cpu.c:1461:9: error: nested extern declaration of ‘tcg_enabled’ 
[-Werror=nested-externs]
cc1: all warnings being treated as errors


I added this 'tcg_enabled()' call in commit eddabb6b8, "target/riscv: skip
features setup for KVM CPUs", back in that review where we discussed whether
this should be a kvm_enabled() check or not.

The build breaks because we're moving the "sysemu/kvm.h" include away from the
file.

I added a patch on top of the series to fix the break:

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c96819daf7..78feb68e86 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -26,7 +26,6 @@
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/pmu.h"
 #include "sysemu/time_helper.h"
-#include "sysemu/tcg.h"
 #include "sysemu/kvm.h"
 #include "sysemu/kvm_riscv.h"
 #endif
@@ -39,6 +38,7 @@
 #include "migration/vmstate.h"
 #include "fpu/softfloat-helpers.h"
 #ifdef CONFIG_TCG
+#include "sysemu/tcg.h"
 #include "tcg/tcg.h"
 #endif
 
@@ -1468,6 +1468,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+#ifdef CONFIG_TCG
     if (tcg_enabled()) {
         riscv_cpu_realize_tcg(dev, &local_err);
         if (local_err != NULL) {
@@ -1475,6 +1476,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
             return;
         }
     }
+#endif
 
     riscv_cpu_finalize_features(cpu, &local_err);

I'm unsure if this is the right fix for what you're aiming to do here though.


Thanks,


Daniel


On 7/11/23 09:14, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   target/riscv/cpu.c        | 10 ++++++----
>   target/riscv/cpu_helper.c |  2 ++
>   target/riscv/csr.c        |  2 ++
>   3 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 9339c0241d..5712a54404 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -23,9 +23,14 @@
>   #include "qemu/log.h"
>   #include "cpu.h"
>   #include "cpu_vendorid.h"
> +#ifndef CONFIG_USER_ONLY
>   #include "pmu.h"
> -#include "internals.h"
>   #include "time_helper.h"
> +#include "sysemu/tcg.h"
> +#include "sysemu/kvm.h"
> +#include "kvm_riscv.h"
> +#endif
> +#include "internals.h"
>   #include "exec/exec-all.h"
>   #include "qapi/error.h"
>   #include "qapi/visitor.h"
> @@ -33,9 +38,6 @@
>   #include "hw/qdev-properties.h"
>   #include "migration/vmstate.h"
>   #include "fpu/softfloat-helpers.h"
> -#include "sysemu/kvm.h"
> -#include "sysemu/tcg.h"
> -#include "kvm_riscv.h"
>   #include "tcg/tcg.h"
>   
>   /* RISC-V CPU definitions */
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9f611d89bb..e8b7f70be3 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -28,7 +28,9 @@
>   #include "tcg/tcg-op.h"
>   #include "trace.h"
>   #include "semihosting/common-semi.h"
> +#ifndef CONFIG_USER_ONLY
>   #include "sysemu/cpu-timers.h"
> +#endif
>   #include "cpu_bits.h"
>   #include "debug.h"
>   #include "tcg/oversized-guest.h"
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index ea7585329e..e5737dcf58 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -21,8 +21,10 @@
>   #include "qemu/log.h"
>   #include "qemu/timer.h"
>   #include "cpu.h"
> +#ifndef CONFIG_USER_ONLY
>   #include "pmu.h"
>   #include "time_helper.h"
> +#endif
>   #include "qemu/main-loop.h"
>   #include "exec/exec-all.h"
>   #include "exec/tb-flush.h"