mbox series

[v6,00/15] target/riscv: Rationalize XLEN and operand length

Message ID 20211020031709.359469-1-richard.henderson@linaro.org
Headers show
Series target/riscv: Rationalize XLEN and operand length | expand

Message

Richard Henderson Oct. 20, 2021, 3:16 a.m. UTC
This is a partial patch set attempting to set things in the
right direction for both the UXL and RV128 patch sets.


r~


Changes for v6:
  * Rebase on riscv-to-apply.next.

Changes for v5:
  * Fix cpu_dump, which asserted for -accel qtest.
    Instead of filtering CSRs explicitly in cpu_dump,
    let the riscv_csr_operations predicate do the job.
    This means we won't dump S-mode registers when RVS
    is not enabled, much like we currently filter on RVH.

Changes for v4:
  * Use riscv_csrrw_debug for cpu_dump.
    This fixes the issue that Alistair pointed out wrt the
    MSTATUS.SD bit not being correct in the dump; note that
    gdbstub already uses riscv_csrrw_debug, and so did not
    have a problem.
  * Align the registers in cpu_dump.

Changes for v3:
  * Fix CONFIG_ typo.
  * Fix ctzw typo.
  * Mark get_xlen unused (clang werror)
  * Compute MSTATUS_SD on demand.

Changes for v2:
  * Set mxl/sxl/uxl at reset.
  * Set sxl/uxl in write_mstatus.


Richard Henderson (15):
  target/riscv: Move cpu_get_tb_cpu_state out of line
  target/riscv: Create RISCVMXL enumeration
  target/riscv: Split misa.mxl and misa.ext
  target/riscv: Replace riscv_cpu_is_32bit with riscv_cpu_mxl
  target/riscv: Add MXL/SXL/UXL to TB_FLAGS
  target/riscv: Use REQUIRE_64BIT in amo_check64
  target/riscv: Properly check SEW in amo_op
  target/riscv: Replace is_32bit with get_xl/get_xlen
  target/riscv: Replace DisasContext.w with DisasContext.ol
  target/riscv: Use gen_arith_per_ol for RVM
  target/riscv: Adjust trans_rev8_32 for riscv64
  target/riscv: Use gen_unary_per_ol for RVB
  target/riscv: Use gen_shift*_per_ol for RVB, RVI
  target/riscv: Use riscv_csrrw_debug for cpu_dump
  target/riscv: Compute mstatus.sd on demand

 target/riscv/cpu.h                      |  73 +++------
 target/riscv/cpu_bits.h                 |   8 +-
 hw/riscv/boot.c                         |   2 +-
 linux-user/elfload.c                    |   2 +-
 linux-user/riscv/cpu_loop.c             |   2 +-
 semihosting/arm-compat-semi.c           |   2 +-
 target/riscv/cpu.c                      | 195 +++++++++++++-----------
 target/riscv/cpu_helper.c               |  92 ++++++++++-
 target/riscv/csr.c                      | 104 ++++++++-----
 target/riscv/gdbstub.c                  |  10 +-
 target/riscv/machine.c                  |  10 +-
 target/riscv/monitor.c                  |   4 +-
 target/riscv/translate.c                | 174 +++++++++++++++------
 target/riscv/insn_trans/trans_rvb.c.inc | 140 +++++++++--------
 target/riscv/insn_trans/trans_rvi.c.inc |  44 +++---
 target/riscv/insn_trans/trans_rvm.c.inc |  36 ++++-
 target/riscv/insn_trans/trans_rvv.c.inc |  29 ++--
 17 files changed, 576 insertions(+), 351 deletions(-)

-- 
2.25.1

Comments

Alistair Francis Oct. 20, 2021, 11:02 a.m. UTC | #1
On Wed, Oct 20, 2021 at 1:26 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> This is a partial patch set attempting to set things in the

> right direction for both the UXL and RV128 patch sets.

>

>

> r~

>

>

> Changes for v6:

>   * Rebase on riscv-to-apply.next.

>

> Changes for v5:

>   * Fix cpu_dump, which asserted for -accel qtest.

>     Instead of filtering CSRs explicitly in cpu_dump,

>     let the riscv_csr_operations predicate do the job.

>     This means we won't dump S-mode registers when RVS

>     is not enabled, much like we currently filter on RVH.

>

> Changes for v4:

>   * Use riscv_csrrw_debug for cpu_dump.

>     This fixes the issue that Alistair pointed out wrt the

>     MSTATUS.SD bit not being correct in the dump; note that

>     gdbstub already uses riscv_csrrw_debug, and so did not

>     have a problem.

>   * Align the registers in cpu_dump.

>

> Changes for v3:

>   * Fix CONFIG_ typo.

>   * Fix ctzw typo.

>   * Mark get_xlen unused (clang werror)

>   * Compute MSTATUS_SD on demand.

>

> Changes for v2:

>   * Set mxl/sxl/uxl at reset.

>   * Set sxl/uxl in write_mstatus.

>

>

> Richard Henderson (15):

>   target/riscv: Move cpu_get_tb_cpu_state out of line

>   target/riscv: Create RISCVMXL enumeration

>   target/riscv: Split misa.mxl and misa.ext

>   target/riscv: Replace riscv_cpu_is_32bit with riscv_cpu_mxl

>   target/riscv: Add MXL/SXL/UXL to TB_FLAGS

>   target/riscv: Use REQUIRE_64BIT in amo_check64

>   target/riscv: Properly check SEW in amo_op

>   target/riscv: Replace is_32bit with get_xl/get_xlen

>   target/riscv: Replace DisasContext.w with DisasContext.ol

>   target/riscv: Use gen_arith_per_ol for RVM

>   target/riscv: Adjust trans_rev8_32 for riscv64

>   target/riscv: Use gen_unary_per_ol for RVB

>   target/riscv: Use gen_shift*_per_ol for RVB, RVI

>   target/riscv: Use riscv_csrrw_debug for cpu_dump

>   target/riscv: Compute mstatus.sd on demand


Thanks!

Applied to riscv-to-apply.next

Alistair

>

>  target/riscv/cpu.h                      |  73 +++------

>  target/riscv/cpu_bits.h                 |   8 +-

>  hw/riscv/boot.c                         |   2 +-

>  linux-user/elfload.c                    |   2 +-

>  linux-user/riscv/cpu_loop.c             |   2 +-

>  semihosting/arm-compat-semi.c           |   2 +-

>  target/riscv/cpu.c                      | 195 +++++++++++++-----------

>  target/riscv/cpu_helper.c               |  92 ++++++++++-

>  target/riscv/csr.c                      | 104 ++++++++-----

>  target/riscv/gdbstub.c                  |  10 +-

>  target/riscv/machine.c                  |  10 +-

>  target/riscv/monitor.c                  |   4 +-

>  target/riscv/translate.c                | 174 +++++++++++++++------

>  target/riscv/insn_trans/trans_rvb.c.inc | 140 +++++++++--------

>  target/riscv/insn_trans/trans_rvi.c.inc |  44 +++---

>  target/riscv/insn_trans/trans_rvm.c.inc |  36 ++++-

>  target/riscv/insn_trans/trans_rvv.c.inc |  29 ++--

>  17 files changed, 576 insertions(+), 351 deletions(-)

>

> --

> 2.25.1

>

>
LIU Zhiwei Oct. 22, 2021, 8:26 a.m. UTC | #2
Hi Richard,

I am implementing the UXL based on this patch set and have a question
about how to process  the PC register.

As the specification said, "PC bits above XLEN are ignored, and when
the PC is written, it is sign-extended to fill the widest supported XLEN."
We still need special process of PC for exceptions or jump instructions.

I have two methods to implement  PC register access,
but not sure which is the right way.

First, normally process the PC register as the specification points.
That means expanding  the PC when setting the global TCGv variable
cpu_pc, and truncating the pc_first and  pc_next fields in
DisasContextBase before decoding instructions.    I am not sure
whether the sign-extended pc is compatible with QEMU common code.

Second,  ignore ignore the PC special process for jump instructions.
Just expand or truncate the PC register when exception processing,
gdb read, or cpu dump registers. It is not a stright way,  but I think 
it is still right.

Hope to get you advice. Thanks very much.

Best Regards,
Zhiwei

On 2021/10/20 上午11:16, Richard Henderson wrote:
> This is a partial patch set attempting to set things in the

> right direction for both the UXL and RV128 patch sets.

>

>

> r~

>

>

> Changes for v6:

>    * Rebase on riscv-to-apply.next.

>

> Changes for v5:

>    * Fix cpu_dump, which asserted for -accel qtest.

>      Instead of filtering CSRs explicitly in cpu_dump,

>      let the riscv_csr_operations predicate do the job.

>      This means we won't dump S-mode registers when RVS

>      is not enabled, much like we currently filter on RVH.

>

> Changes for v4:

>    * Use riscv_csrrw_debug for cpu_dump.

>      This fixes the issue that Alistair pointed out wrt the

>      MSTATUS.SD bit not being correct in the dump; note that

>      gdbstub already uses riscv_csrrw_debug, and so did not

>      have a problem.

>    * Align the registers in cpu_dump.

>

> Changes for v3:

>    * Fix CONFIG_ typo.

>    * Fix ctzw typo.

>    * Mark get_xlen unused (clang werror)

>    * Compute MSTATUS_SD on demand.

>

> Changes for v2:

>    * Set mxl/sxl/uxl at reset.

>    * Set sxl/uxl in write_mstatus.

>

>

> Richard Henderson (15):

>    target/riscv: Move cpu_get_tb_cpu_state out of line

>    target/riscv: Create RISCVMXL enumeration

>    target/riscv: Split misa.mxl and misa.ext

>    target/riscv: Replace riscv_cpu_is_32bit with riscv_cpu_mxl

>    target/riscv: Add MXL/SXL/UXL to TB_FLAGS

>    target/riscv: Use REQUIRE_64BIT in amo_check64

>    target/riscv: Properly check SEW in amo_op

>    target/riscv: Replace is_32bit with get_xl/get_xlen

>    target/riscv: Replace DisasContext.w with DisasContext.ol

>    target/riscv: Use gen_arith_per_ol for RVM

>    target/riscv: Adjust trans_rev8_32 for riscv64

>    target/riscv: Use gen_unary_per_ol for RVB

>    target/riscv: Use gen_shift*_per_ol for RVB, RVI

>    target/riscv: Use riscv_csrrw_debug for cpu_dump

>    target/riscv: Compute mstatus.sd on demand

>

>   target/riscv/cpu.h                      |  73 +++------

>   target/riscv/cpu_bits.h                 |   8 +-

>   hw/riscv/boot.c                         |   2 +-

>   linux-user/elfload.c                    |   2 +-

>   linux-user/riscv/cpu_loop.c             |   2 +-

>   semihosting/arm-compat-semi.c           |   2 +-

>   target/riscv/cpu.c                      | 195 +++++++++++++-----------

>   target/riscv/cpu_helper.c               |  92 ++++++++++-

>   target/riscv/csr.c                      | 104 ++++++++-----

>   target/riscv/gdbstub.c                  |  10 +-

>   target/riscv/machine.c                  |  10 +-

>   target/riscv/monitor.c                  |   4 +-

>   target/riscv/translate.c                | 174 +++++++++++++++------

>   target/riscv/insn_trans/trans_rvb.c.inc | 140 +++++++++--------

>   target/riscv/insn_trans/trans_rvi.c.inc |  44 +++---

>   target/riscv/insn_trans/trans_rvm.c.inc |  36 ++++-

>   target/riscv/insn_trans/trans_rvv.c.inc |  29 ++--

>   17 files changed, 576 insertions(+), 351 deletions(-)

>
Richard Henderson Oct. 22, 2021, 3:50 p.m. UTC | #3
On 10/22/21 1:26 AM, LIU Zhiwei wrote:
> As the specification said, "PC bits above XLEN are ignored, and when

> the PC is written, it is sign-extended to fill the widest supported XLEN."

> We still need special process of PC for exceptions or jump instructions.

> 

> I have two methods to implement  PC register access,

> but not sure which is the right way.

> 

> First, normally process the PC register as the specification points.

> That means expanding  the PC when setting the global TCGv variable

> cpu_pc, and truncating the pc_first and  pc_next fields in

> DisasContextBase before decoding instructions.    I am not sure

> whether the sign-extended pc is compatible with QEMU common code.


I think extending on write is the correct thing.  Jumps, exception entry and return, gdb 
write.

Note that the read from PC for translation is in cpu_get_tb_cpu_state, before translation. 
  You should not need to change anything wrt pc_first/pc_next/etc, because it will already 
have been done.

> Second,  ignore ignore the PC special process for jump instructions.

> Just expand or truncate the PC register when exception processing,

> gdb read, or cpu dump registers. It is not a stright way,  but I think it is still right.


I think you could make it work that way, but I don't know that it's less difficult, or 
that you'd have fewer changes.


r~
LIU Zhiwei Oct. 25, 2021, 9:24 a.m. UTC | #4
On 2021/10/22 下午11:50, Richard Henderson wrote:
> On 10/22/21 1:26 AM, LIU Zhiwei wrote:

>> As the specification said, "PC bits above XLEN are ignored, and when

>> the PC is written, it is sign-extended to fill the widest supported 

>> XLEN."

>> We still need special process of PC for exceptions or jump instructions.

>>

>> I have two methods to implement  PC register access,

>> but not sure which is the right way.

>>

>> First, normally process the PC register as the specification points.

>> That means expanding  the PC when setting the global TCGv variable

>> cpu_pc, and truncating the pc_first and  pc_next fields in

>> DisasContextBase before decoding instructions.    I am not sure

>> whether the sign-extended pc is compatible with QEMU common code.

>

> I think extending on write is the correct thing. 


Thanks. I will pick this way.

> Jumps, exception entry and return, gdb write.


If we carefully process jumps and gdb write, I think we can omit 
exception entry and return. Is it right?

>

> Note that the read from PC for translation is in cpu_get_tb_cpu_state, 

> before translation.  You should not need to change anything wrt 

> pc_first/pc_next/etc, because it will already have been done.


Good! Thanks.

Best Regards,
Zhiwei

>

>> Second,  ignore ignore the PC special process for jump instructions.

>> Just expand or truncate the PC register when exception processing,

>> gdb read, or cpu dump registers. It is not a stright way,  but I 

>> think it is still right.

>

> I think you could make it work that way, but I don't know that it's 

> less difficult, or that you'd have fewer changes.

>

>

> r~
Richard Henderson Oct. 25, 2021, 2:58 p.m. UTC | #5
On 10/25/21 2:24 AM, LIU Zhiwei wrote:
>> I think extending on write is the correct thing. 

> 

> Thanks. I will pick this way.

> 

>> Jumps, exception entry and return, gdb write.

> 

> If we carefully process jumps and gdb write, I think we can omit exception entry and 

> return. Is it right?


No, though you'd probably have to create a special test case to see it.

On exception return, a 64-bit OS can write a 64-bit value into SEPC.  But when SEPC is 
copied to PC during SRET, UXL should be examined and the assignment should extend for a 
32-bit user program.

Exception entry would seem to be limited on its face by SXLEN; STVEC will always contain 
the same number of bits as PC, so no (additional) extension would be required.  But we do 
have to be careful to interpret the target_ulong value properly for the current SXLEN.


r~