Re: [PATCH 3/6] target/riscv: Set VS* bits to one in mideleg when H-Ext is enabled

[Daniel Henrique Barboza]

On 5/18/23 08:38, Rajnesh Kanwal wrote:
> With H-Ext supported, VS bits are all hardwired to one in MIDELEG
> denoting always delegated interrupts. This is being done in rmw_mideleg
> but given mideleg is used in other places when routing interrupts
> this change initializes it in riscv_cpu_realize to be on the safe side.
>
> Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> ---
>   target/riscv/cpu.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index db0875fb43..90460cfe64 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1288,6 +1288,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> **errp)
>           return;
>       }
>   
> +    /* With H-Ext VSSIP, VSTIP, VSEIP and SGEIP are hardwired to one. */
> +    if (riscv_has_ext(env, RVH)) {
> +        env->mideleg = MIP_VSSIP | MIP_VSTIP | MIP_VSEIP | MIP_SGEIP;
> +    }
> +

This change breaks linux-user build:

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 -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/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -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 -fstack-protector-strong -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:1366:12: error: ‘CPURISCVState’ {aka ‘struct 
CPUArchState’} has no member named ‘mideleg’
 1366 |         env->mideleg = MIP_VSSIP | MIP_VSTIP | MIP_VSEIP | MIP_SGEIP;
      |            ^~
[1720/2798] Compiling C object 
libqemu-riscv64-linux-user.fa.p/linux-user_riscv_cpu_loop.



The reason is that 'mideleg' is a system emulation attribute only (i.e. defined
in a #ifndef CONFIG_USER_ONLY block). There's a block like that right before 
this
point where riscv_timer_init() is being called. I suggest moving this code 
there.


Thanks,


Daniel



>       riscv_cpu_register_gdb_regs_for_features(cs);
>   
>       qemu_init_vcpu(cs);