[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 12/14] target/riscv: rvk: add CSR support for Zkr

From: Weiwei Li
Subject: Re: [PATCH v7 12/14] target/riscv: rvk: add CSR support for Zkr
Date: Wed, 2 Mar 2022 08:57:05 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0

在 2022/3/1 下午11:59, Richard Henderson 写道:
On 2/28/22 16:27, Weiwei Li wrote:

在 2022/3/1 上午9:44, Weiwei Li 写道:

在 2022/3/1 上午4:11, Richard Henderson 写道:
On 2/28/22 04:48, Weiwei Li wrote:
+/* Crypto Extension */
+static RISCVException rmw_seed(CPURISCVState *env, int csrno,
+                              target_ulong *ret_value,
+                              target_ulong new_value, target_ulong write_mask)
+    if (!write_mask) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }

This is incorrect.  The error should only be with a write-mask of the actual x0 register, not another register which happens to contain 0.  There is in fact no way to diagnose exactly what you want here, which IIRC has an existing fixme comment somewhere.
Yeah. write_mask is also used in riscv_csrrw_check to check whether the read-only csr is written. We cannot distinguish x0 and reg which contains 0  here without changing total progress of csr read/write.

I seems misunderstand the code for csr read/write:  write_mask will be set zero only for read-only operation (CSRRS/CSRRC with rs1=x0 or CSRRSI/CSRRCI with uimm=0) via do_csrr --> helper_csrr -> riscv_csrrw call-chain.

The write_mask for do_csrw and do_csrrw will not be zero.

As said in the spec :

"TheseedCSR must be accessed with a read-write instruction. A read-only instruction such asCSRRS/CSRRC withrs1=x0orCSRRSI/CSRRCIwithuimm=0will raise an illegal instruction exception. "

So it's suitable to check write_mask here.

Consider CSRRS with rs1=x31.  In that case mask will be the value in x31.  Even if the value is 0, this is still considered a read-write instruction.
Yeah. I lost this kind of case . So this is a bug. Maybe we can add a new parameter like  'write' to helper_csrrw to indicate that write operation should be done.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]