On Thu, Oct 17, 2024 at 4:05 PM Frank Chang <frank.ch...@sifive.com> wrote:
>
> On Thu, Oct 17, 2024 at 7:18 PM Rajnesh Kanwal <rkan...@rivosinc.com> wrote:
>>
>> On Tue, Aug 27, 2024 at 10:28 AM Frank Chang <frank.ch...@sifive.com> wrote:
>> >
>> > Rajnesh Kanwal <rkan...@rivosinc.com> 於 2024年6月19日 週三 下午11:27寫道：
>> > >
>> > > This commit adds support for [m|s|vs]ctrcontrol, sctrstatus and
>> > > sctrdepth CSRs handling.
>> > >
>> > > Signed-off-by: Rajnesh Kanwal <rkan...@rivosinc.com>
>> > > ---
>> > > target/riscv/cpu.h | 5 ++
>> > > target/riscv/cpu_cfg.h | 2 +
>> > > target/riscv/csr.c | 128 +++++++++++++++++++++++++++++++++++++++++
>> > > 3 files changed, 135 insertions(+)
>> > >
>> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > > index a185e2d494..3d4d5172b8 100644
>> > > --- a/target/riscv/cpu.h
>> > > +++ b/target/riscv/cpu.h
>> > > @@ -263,6 +263,11 @@ struct CPUArchState {
>> > > target_ulong mcause;
>> > > target_ulong mtval; /* since: priv-1.10.0 */
>> > >
>> > > + uint64_t mctrctl;
>> > > + uint32_t sctrdepth;
>> > > + uint32_t sctrstatus;
>> > > + uint64_t vsctrctl;
>> > > +
>> > > /* Machine and Supervisor interrupt priorities */
>> > > uint8_t miprio[64];
>> > > uint8_t siprio[64];
>> > > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
>> > > index d9354dc80a..d329a65811 100644
>> > > --- a/target/riscv/cpu_cfg.h
>> > > +++ b/target/riscv/cpu_cfg.h
>> > > @@ -123,6 +123,8 @@ struct RISCVCPUConfig {
>> > > bool ext_zvfhmin;
>> > > bool ext_smaia;
>> > > bool ext_ssaia;
>> > > + bool ext_smctr;
>> > > + bool ext_ssctr;
>> >
>> > Base on: https://github.com/riscv/riscv-control-transfer-records/pull/29
>> > Smctr and Ssctr depend on both S-mode and Sscsrind.
>> > We should add the implied rules for Smctr and Ssctr.
>> >
>> > Regards,
>> > Frank Chang
>>
>> Hi Frank,
>>
>> Are you referring to the checks in riscv_cpu_validate_set_extensions()?
>> These checks are already present in the last patch.
>>
>> https://lore.kernel.org/qemu-riscv/20240619152708.135991-7-rkan...@rivosinc.com/
>>
>
> Hi Rajnesh,
>
> No, I was referring to the implied rules defined in:
> https://github.com/qemu/qemu/blob/master/target/riscv/cpu.c#L2630
>
> The implied rules will enable the implied extensions when the "parent"
> extension is enabled.
> Unless user turns them off explicitly from the command line,
> which is an error and will be caught by the check rules you mentioned above.
>
> The spec defines that:
> "Smctr and Ssctr depend on both the implementation of S-mode and the Sscsrind
> extension"
> which means that we should add RVS and Sscsrind extensions as the implied
> rules for Smctr and Ssctr respectively.
>
> The use of the word "depends" in the spec is quite ambiguous.
> But I once had a chance to ask Andrew Waterman about it,
> and he replied that we should treat “depends on” or “requires” as a synonym
> for “implies”.
>
>
> Regards,
> Frank Chang
>
Thanks for the explanation Frank. I will fix this in the next version.
>> >
>> >
>> > > bool ext_sscofpmf;
>> > > bool ext_smepmp;
>> > > bool rvv_ta_all_1s;
>> > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> > > index 2f92e4b717..0b5bf4d050 100644
>> > > --- a/target/riscv/csr.c
>> > > +++ b/target/riscv/csr.c
>> > > @@ -621,6 +621,48 @@ static RISCVException pointer_masking(CPURISCVState
>> > > *env, int csrno)
>> > > return RISCV_EXCP_ILLEGAL_INST;
>> > > }
>> > >
>> > > +/*
>> > > + * M-mode:
>> > > + * Without ext_smctr raise illegal inst excep.
>> > > + * Otherwise everything is accessible to m-mode.
>> > > + *
>> > > + * S-mode:
>> > > + * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
>> > > + * Otherwise everything other than mctrctl is accessible.
>> > > + *
>> > > + * VS-mode:
>> > > + * Without ext_ssctr or mstateen.ctr raise illegal inst excep.
>> > > + * Without hstateen.ctr raise virtual illegal inst excep.
>> > > + * Otherwise allow sctrctl (vsctrctl), sctrstatus, 0x200-0x2ff entry
>> > > range.
>> > > + * Always raise illegal instruction exception for sctrdepth.
>> > > + */
>> > > +static RISCVException ctr_mmode(CPURISCVState *env, int csrno)
>> > > +{
>> > > + /* Check if smctr-ext is present */
>> > > + if (riscv_cpu_cfg(env)->ext_smctr) {
>> > > + return RISCV_EXCP_NONE;
>> > > + }
>> > > +
>> > > + return RISCV_EXCP_ILLEGAL_INST;
>> > > +}
>> > > +
>> > > +static RISCVException ctr_smode(CPURISCVState *env, int csrno)
>> > > +{
>> > > + const RISCVCPUConfig *cfg = riscv_cpu_cfg(env);
>> > > +
>> > > + if (!cfg->ext_smctr && !cfg->ext_ssctr) {
>> > > + return RISCV_EXCP_ILLEGAL_INST;
>> > > + }
>> > > +
>> > > + RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_CTR);
>> > > + if (ret == RISCV_EXCP_NONE && csrno == CSR_SCTRDEPTH &&
>> > > + env->virt_enabled) {
>> > > + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>> > > + }
>> > > +
>> > > + return ret;
>> > > +}
>> > > +
>> > > static RISCVException aia_hmode(CPURISCVState *env, int csrno)
>> > > {
>> > > int ret;
>> > > @@ -3835,6 +3877,86 @@ static RISCVException write_satp(CPURISCVState
>> > > *env, int csrno,
>> > > return RISCV_EXCP_NONE;
>> > > }
>> > >
>> > > +static RISCVException rmw_sctrdepth(CPURISCVState *env, int csrno,
>> > > + target_ulong *ret_val,
>> > > + target_ulong new_val, target_ulong
>> > > wr_mask)
>> > > +{
>> > > + uint64_t mask = wr_mask & SCTRDEPTH_MASK;
>> > > +
>> > > + if (ret_val) {
>> > > + *ret_val = env->sctrdepth;
>> > > + }
>> > > +
>> > > + env->sctrdepth = (env->sctrdepth & ~mask) | (new_val & mask);
>> > > +
>> > > + /* Correct depth. */
>> > > + if (wr_mask & SCTRDEPTH_MASK) {
>> > > + uint64_t depth = get_field(env->sctrdepth, SCTRDEPTH_MASK);
>> > > +
>> > > + if (depth > SCTRDEPTH_MAX) {
>> > > + depth = SCTRDEPTH_MAX;
>> > > + env->sctrdepth = set_field(env->sctrdepth, SCTRDEPTH_MASK,
>> > > depth);
>> > > + }
>> > > +
>> > > + /* Update sctrstatus.WRPTR with a legal value */
>> > > + depth = 16 << depth;
>> > > + env->sctrstatus =
>> > > + env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK | (depth - 1));
>> > > + }
>> > > +
>> > > + return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > > +static RISCVException rmw_sctrstatus(CPURISCVState *env, int csrno,
>> > > + target_ulong *ret_val,
>> > > + target_ulong new_val, target_ulong
>> > > wr_mask)
>> > > +{
>> > > + uint32_t depth = 16 << get_field(env->sctrdepth, SCTRDEPTH_MASK);
>> > > + uint32_t mask = wr_mask & SCTRSTATUS_MASK;
>> > > +
>> > > + if (ret_val) {
>> > > + *ret_val = env->sctrstatus;
>> > > + }
>> > > +
>> > > + env->sctrstatus = (env->sctrstatus & ~mask) | (new_val & mask);
>> > > +
>> > > + /* Update sctrstatus.WRPTR with a legal value */
>> > > + env->sctrstatus = env->sctrstatus & (~SCTRSTATUS_WRPTR_MASK |
>> > > (depth - 1));
>> > > +
>> > > + return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > > +static RISCVException rmw_xctrctl(CPURISCVState *env, int csrno,
>> > > + target_ulong *ret_val,
>> > > + target_ulong new_val, target_ulong
>> > > wr_mask)
>> > > +{
>> > > + uint64_t csr_mask, mask = wr_mask;
>> > > + uint64_t *ctl_ptr = &env->mctrctl;
>> > > +
>> > > + if (csrno == CSR_MCTRCTL) {
>> > > + csr_mask = MCTRCTL_MASK;
>> > > + } else if (csrno == CSR_SCTRCTL && !env->virt_enabled) {
>> > > + csr_mask = SCTRCTL_MASK;
>> > > + } else {
>> > > + /*
>> > > + * This is for csrno == CSR_SCTRCTL and env->virt_enabled ==
>> > > true
>> > > + * or csrno == CSR_VSCTRCTL.
>> > > + */
>> > > + csr_mask = VSCTRCTL_MASK;
>> > > + ctl_ptr = &env->vsctrctl;
>> > > + }
>> > > +
>> > > + mask &= csr_mask;
>> > > +
>> > > + if (ret_val) {
>> > > + *ret_val = *ctl_ptr & csr_mask;
>> > > + }
>> > > +
>> > > + *ctl_ptr = (*ctl_ptr & ~mask) | (new_val & mask);
>> > > +
>> > > + return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > > static RISCVException read_vstopi(CPURISCVState *env, int csrno,
>> > > target_ulong *val)
>> > > {
>> > > @@ -5771,6 +5893,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>> > > [CSR_SPMBASE] = { "spmbase", pointer_masking, read_spmbase,
>> > > write_spmbase
>> > > },
>> > >
>> > > + [CSR_MCTRCTL] = { "mctrctl", ctr_mmode, NULL, NULL,
>> > > rmw_xctrctl },
>> > > + [CSR_SCTRCTL] = { "sctrctl", ctr_smode, NULL, NULL,
>> > > rmw_xctrctl },
>> > > + [CSR_VSCTRCTL] = { "vsctrctl", ctr_smode, NULL, NULL,
>> > > rmw_xctrctl },
>> > > + [CSR_SCTRDEPTH] = { "sctrdepth", ctr_smode, NULL, NULL,
>> > > rmw_sctrdepth },
>> > > + [CSR_SCTRSTATUS] = { "sctrstatus", ctr_smode, NULL, NULL,
>> > > rmw_sctrstatus },
>> > > +
>> > > /* Performance Counters */
>> > > [CSR_HPMCOUNTER3] = { "hpmcounter3", ctr, read_hpmcounter
>> > > },
>> > > [CSR_HPMCOUNTER4] = { "hpmcounter4", ctr, read_hpmcounter
>> > > },
>> > > --
>> > > 2.34.1
>> > >
>> > >
