[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/8] target/riscv: Settings for 128-bit extension support
|
From: |
Alistair Francis |
|
Subject: |
Re: [PATCH 1/8] target/riscv: Settings for 128-bit extension support |
|
Date: |
Tue, 31 Aug 2021 13:13:29 +1000 |
On Tue, Aug 31, 2021 at 5:26 AM Frédéric Pétrot
<frederic.petrot@univ-grenoble-alpes.fr> wrote:
>
> Starting 128-bit extension support implies a few modifications in the
> existing sources because checking for 32-bit is done by checking that
> it is not 64-bit and vice-versa.
> We now consider the 3 possible xlen values so as to allow correct
> compilation for both existing targets while setting the compilation
> framework so that it can also handle the riscv128-softmmu target.
> This includes gdb configuration files, that are just the bare copy of the
> 64-bit ones as gdb does not honor, yet, 128-bit CPUs.
> To consider the 3 xlen values, we had to add a misah field, representing the
> upper 64 bits of the misa register.
>
> Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
> Co-authored-by: Fabien Portas <fabien.portas@grenoble-inp.org>
> ---
> configs/devices/riscv128-softmmu/default.mak | 16 ++++++
> configs/targets/riscv128-softmmu.mak | 5 ++
> gdb-xml/riscv-128bit-cpu.xml | 48 ++++++++++++++++++
> gdb-xml/riscv-128bit-virtual.xml | 12 +++++
> include/hw/riscv/sifive_cpu.h | 4 ++
> target/riscv/Kconfig | 3 ++
> target/riscv/arch_dump.c | 3 +-
> target/riscv/cpu-param.h | 3 +-
> target/riscv/cpu.c | 51 +++++++++++++++++---
> target/riscv/cpu.h | 19 ++++++++
> target/riscv/gdbstub.c | 3 ++
> target/riscv/insn_trans/trans_rvd.c.inc | 10 ++--
> target/riscv/insn_trans/trans_rvf.c.inc | 2 +-
> target/riscv/translate.c | 45 ++++++++++++++++-
> 14 files changed, 209 insertions(+), 15 deletions(-)
> create mode 100644 configs/devices/riscv128-softmmu/default.mak
> create mode 100644 configs/targets/riscv128-softmmu.mak
> create mode 100644 gdb-xml/riscv-128bit-cpu.xml
> create mode 100644 gdb-xml/riscv-128bit-virtual.xml
Hey!
Thanks for the patches!
Overall this patch looks good.
It would greatly help reviewing and the speed in which this can be
merged if you can split it up more. A lot of these changes probably
can be separate patches (for example a patch to add misah). I know it
can sometimes seem a little silly, but it greatly helps with reviewing
when patches are small and self contained.
>
> diff --git a/configs/devices/riscv128-softmmu/default.mak
> b/configs/devices/riscv128-softmmu/default.mak
> new file mode 100644
> index 0000000000..31439dbcfe
> --- /dev/null
> +++ b/configs/devices/riscv128-softmmu/default.mak
> @@ -0,0 +1,16 @@
> +# Default configuration for riscv128-softmmu
> +
> +# Uncomment the following lines to disable these optional devices:
> +#
> +#CONFIG_PCI_DEVICES=n
> +CONFIG_SEMIHOSTING=y
> +CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
> +
> +# Boards:
> +#
> +CONFIG_SPIKE=n
> +CONFIG_SIFIVE_E=n
> +CONFIG_SIFIVE_U=n
> +CONFIG_RISCV_VIRT=y
> +CONFIG_MICROCHIP_PFSOC=n
> +CONFIG_SHAKTI_C=n
> diff --git a/configs/targets/riscv128-softmmu.mak
> b/configs/targets/riscv128-softmmu.mak
> new file mode 100644
> index 0000000000..e300c43c8e
> --- /dev/null
> +++ b/configs/targets/riscv128-softmmu.mak
> @@ -0,0 +1,5 @@
> +TARGET_ARCH=riscv128
> +TARGET_BASE_ARCH=riscv
> +TARGET_SUPPORTS_MTTCG=y
> +TARGET_XML_FILES= gdb-xml/riscv-128bit-cpu.xml gdb-xml/riscv-32bit-fpu.xml
> gdb-xml/riscv-64bit-fpu.xml gdb-xml/riscv-128bit-virtual.xml
> +TARGET_NEED_FDT=y
> diff --git a/gdb-xml/riscv-128bit-cpu.xml b/gdb-xml/riscv-128bit-cpu.xml
> new file mode 100644
> index 0000000000..c98168148f
> --- /dev/null
> +++ b/gdb-xml/riscv-128bit-cpu.xml
> @@ -0,0 +1,48 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2018-2019 Free Software Foundation, Inc.
> +
> + Copying and distribution of this file, with or without modification,
> + are permitted in any medium without royalty provided the copyright
> + notice and this notice are preserved. -->
> +
> +<!-- Register numbers are hard-coded in order to maintain backward
> + compatibility with older versions of tools that didn't use xml
> + register descriptions. -->
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> +<!-- FIXME : All GPRs are marked as 64-bits since gdb doesn't like 128-bit
> registers for now. -->
> +<feature name="org.gnu.gdb.riscv.cpu">
> + <reg name="zero" bitsize="64" type="int" regnum="0"/>
> + <reg name="ra" bitsize="64" type="code_ptr"/>
> + <reg name="sp" bitsize="64" type="data_ptr"/>
> + <reg name="gp" bitsize="64" type="data_ptr"/>
> + <reg name="tp" bitsize="64" type="data_ptr"/>
> + <reg name="t0" bitsize="64" type="int"/>
> + <reg name="t1" bitsize="64" type="int"/>
> + <reg name="t2" bitsize="64" type="int"/>
> + <reg name="fp" bitsize="64" type="data_ptr"/>
> + <reg name="s1" bitsize="64" type="int"/>
> + <reg name="a0" bitsize="64" type="int"/>
> + <reg name="a1" bitsize="64" type="int"/>
> + <reg name="a2" bitsize="64" type="int"/>
> + <reg name="a3" bitsize="64" type="int"/>
> + <reg name="a4" bitsize="64" type="int"/>
> + <reg name="a5" bitsize="64" type="int"/>
> + <reg name="a6" bitsize="64" type="int"/>
> + <reg name="a7" bitsize="64" type="int"/>
> + <reg name="s2" bitsize="64" type="int"/>
> + <reg name="s3" bitsize="64" type="int"/>
> + <reg name="s4" bitsize="64" type="int"/>
> + <reg name="s5" bitsize="64" type="int"/>
> + <reg name="s6" bitsize="64" type="int"/>
> + <reg name="s7" bitsize="64" type="int"/>
> + <reg name="s8" bitsize="64" type="int"/>
> + <reg name="s9" bitsize="64" type="int"/>
> + <reg name="s10" bitsize="64" type="int"/>
> + <reg name="s11" bitsize="64" type="int"/>
> + <reg name="t3" bitsize="64" type="int"/>
> + <reg name="t4" bitsize="64" type="int"/>
> + <reg name="t5" bitsize="64" type="int"/>
> + <reg name="t6" bitsize="64" type="int"/>
> + <reg name="pc" bitsize="64" type="code_ptr"/>
> +</feature>
> diff --git a/gdb-xml/riscv-128bit-virtual.xml
> b/gdb-xml/riscv-128bit-virtual.xml
> new file mode 100644
> index 0000000000..db9a0ff677
> --- /dev/null
> +++ b/gdb-xml/riscv-128bit-virtual.xml
> @@ -0,0 +1,12 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2018-2019 Free Software Foundation, Inc.
> +
> + Copying and distribution of this file, with or without modification,
> + are permitted in any medium without royalty provided the copyright
> + notice and this notice are preserved. -->
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> +<!-- FIXME : priv marked as 64-bits since gdb doesn't like 128-bit registers
> for now. -->
> +<feature name="org.gnu.gdb.riscv.virtual">
> + <reg name="priv" bitsize="64"/>
> +</feature>
> diff --git a/include/hw/riscv/sifive_cpu.h b/include/hw/riscv/sifive_cpu.h
> index 136799633a..2fd441664f 100644
> --- a/include/hw/riscv/sifive_cpu.hthat
> +++ b/include/hw/riscv/sifive_cpu.h
> @@ -26,6 +26,10 @@
> #elif defined(TARGET_RISCV64)
> #define SIFIVE_E_CPU TYPE_RISCV_CPU_SIFIVE_E51
> #define SIFIVE_U_CPU TYPE_RISCV_CPU_SIFIVE_U54
> +#elif defined(TARGET_RISCV128)
> +/* 128-bit uses 64-bit CPU for now, since no cpu implements RV128 */
> +#define SIFIVE_E_CPU TYPE_RISCV_CPU_SIFIVE_E51
> +#define SIFIVE_U_CPU TYPE_RISCV_CPU_SIFIVE_U54
> #endif
>
> #endif /* HW_SIFIVE_CPU_H */
> diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
> index b9e5932f13..f9ea52a59a 100644
> --- a/target/riscv/Kconfig
> +++ b/target/riscv/Kconfig
> @@ -3,3 +3,6 @@ config RISCV32
>
> config RISCV64
> bool
> +
> +config RISCV128
> + bool
> diff --git a/target/riscv/arch_dump.c b/target/riscv/arch_dump.c
> index 709f621d82..f756ed2988 100644
> --- a/target/riscv/arch_dump.c
> +++ b/target/riscv/arch_dump.c
> @@ -176,7 +176,8 @@ int cpu_get_dump_info(ArchDumpInfo *info,
>
> info->d_machine = EM_RISCV;
>
> -#if defined(TARGET_RISCV64)
> +#if defined(TARGET_RISCV64) || defined(TARGET_RISCV128)
> + /* FIXME : No 128-bit ELF class exists (for now), use 64-bit one. */
> info->d_class = ELFCLASS64;
> #else
> info->d_class = ELFCLASS32;
> diff --git a/target/riscv/cpu-param.h b/target/riscv/cpu-param.h
> index 80eb615f93..e6d0651f60 100644
> --- a/target/riscv/cpu-param.h
> +++ b/target/riscv/cpu-param.h
> @@ -8,7 +8,8 @@
> #ifndef RISCV_CPU_PARAM_H
> #define RISCV_CPU_PARAM_H 1
>
> -#if defined(TARGET_RISCV64)
> +/* 64-bit target, since QEMU isn't built to have TARGET_LONG_BITS over 64 */
> +#if defined(TARGET_RISCV64) || defined(TARGET_RISCV128)
> # define TARGET_LONG_BITS 64
> # define TARGET_PHYS_ADDR_SPACE_BITS 56 /* 44-bit PPN */
> # define TARGET_VIRT_ADDR_SPACE_BITS 48 /* sv48 */
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 991a6bb760..1f15026e9c 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -110,18 +110,38 @@ const char *riscv_cpu_get_trap_name(target_ulong cause,
> bool async)
>
> bool riscv_cpu_is_32bit(CPURISCVState *env)
> {
> - if (env->misa & RV64) {
> - return false;
> - }
> + return (env->misa & MXLEN_MASK) == RV32;
> +}
>
> - return true;
> +bool riscv_cpu_is_64bit(CPURISCVState *env)
> +{
> + return (env->misa & MXLEN_MASK) == RV64;
> }
>
> +#if defined(TARGET_RISCV128)
Don't add any TARGET_* defines.
We are trying to move to a point where the 64-bit RISC-V softmmu can
run 32-bit CPUs. Ideally we want the same with 128-bit. You don't have
to get that working, but don't add any compile time conditionals.
That applies to all code, not just this patch. Unless there is already
a conditional TARGET_* compile please don't add one.
Alistair
Re: [PATCH 4/8] target/riscv: 128-bit arithmetic and logic instructions, Richard Henderson, 2021/08/30
[PATCH 6/8] target/riscv: Support of compiler's 128-bit integer types, Frédéric Pétrot, 2021/08/30
[PATCH 5/8] target/riscv: 128-bit multiply and divide, Frédéric Pétrot, 2021/08/30
[PATCH 8/8] target/riscv: Support for 128-bit satp, Frédéric Pétrot, 2021/08/30
[PATCH 7/8] target/riscv: 128-bit support for some csrs, Frédéric Pétrot, 2021/08/30
Re: [PATCH 1/8] target/riscv: Settings for 128-bit extension support,
Alistair Francis <=