Re: [PATCH 1/2] target/riscv: fix access permission checks for CSR_SSP

[Deepak Gupta <debug@rivosinc.com>]

On Thu, Mar 06, 2025 at 04:20:56PM +1000, Alistair Francis wrote:
>On Thu, Mar 6, 2025 at 4:12 PM Deepak Gupta <debug@rivosinc.com> wrote:
>>
>> On Thu, Mar 06, 2025 at 03:20:55PM +1000, Alistair Francis wrote:
>> >On Tue, Feb 18, 2025 at 12:56 PM Deepak Gupta <debug@rivosinc.com> wrote:
>> >>
>> >> Commit:8205bc1 ("target/riscv: introduce ssp and enabling controls for
>> >> zicfiss") introduced CSR_SSP but it mis-interpreted the spec on access
>> >> to CSR_SSP in M-mode. Gated to CSR_SSP is not gated via `xSSE`. But
>> >> rather rules clearly specified in section "2.2.4. Shadow Stack Pointer"
>> >
>> >Do you mean "22.2.1. Shadow Stack Pointer (ssp) CSR access contr" in
>> >the priv spec?
>>
>> No I meant 2.2.4 of zicfiss specification. Section 22.2.1 of priv spec
>> says same.
>
>I meant that it's now just in the priv spec, the zicfiss spec is no
>longer maintained so we should just reference the priv spec

Got it.

>
>Alistair
>
>>
>> >
>> >> of `zicfiss` specification. Thanks to Adam Zabrocki for bringing this
>> >> to attention.
>> >
>> >The thanks should probably be below the line
>>
>> Sure
>>
>> >
>> >>
>> >> Fixes: 8205bc127a83 ("target/riscv: introduce ssp and enabling controls
>> >> for zicfiss"
>> >>
>> >> Reported-by: Adam Zabrocki <azabrocki@nvidia.com>
>> >> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>> >
>> >The actual change looks good:
>> >
>> >Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>> >
>> >Alistair
>> >
>> >> ---
>> >>  target/riscv/csr.c | 5 +++++
>> >>  1 file changed, 5 insertions(+)
>> >>
>> >> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> >> index afb7544f07..75c661d2a1 100644
>> >> --- a/target/riscv/csr.c
>> >> +++ b/target/riscv/csr.c
>> >> @@ -191,6 +191,11 @@ static RISCVException cfi_ss(CPURISCVState *env, int csrno)
>> >>          return RISCV_EXCP_ILLEGAL_INST;
>> >>      }
>> >>
>> >> +    /* If ext implemented, M-mode always have access to SSP CSR */
>> >> +    if (env->priv == PRV_M) {
>> >> +        return RISCV_EXCP_NONE;
>> >> +    }
>> >> +
>> >>      /* if bcfi not active for current env, access to csr is illegal */
>> >>      if (!cpu_get_bcfien(env)) {
>> >>  #if !defined(CONFIG_USER_ONLY)
>> >> --
>> >> 2.34.1
>> >>
>> >>