在 2022/3/17 下午12:39, Alistair Francis 写道:
On Thu, Mar 17, 2022 at 12:10 PM Weiwei Li <liwei...@iscas.ac.cn> wrote:
在 2022/3/17 上午6:35, Alistair Francis 写道:
On Thu, Mar 17, 2022 at 1:13 AM Weiwei Li <liwei...@iscas.ac.cn> wrote:
riscv_raise_exception(env, ret, GETPC());
@@ -90,7 +90,7 @@ void helper_csrw_i128(CPURISCVState *env, int csr,
{
RISCVException ret = riscv_csrrw_i128(env, csr, NULL,
int128_make128(srcl, srch),
- UINT128_MAX);
+ UINT128_MAX, true);
if (ret != RISCV_EXCP_NONE) {
riscv_raise_exception(env, ret, GETPC());
@@ -104,7 +104,7 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr,
Int128 rv = int128_zero();
RISCVException ret = riscv_csrrw_i128(env, csr, &rv,
int128_make128(srcl, srch),
- int128_make128(maskl, maskh));
+ int128_make128(maskl, maskh), true);
if (ret != RISCV_EXCP_NONE) {
riscv_raise_exception(env, ret, GETPC());
I am afraid the adding of "bool write_op" argument was done on many
functions, some of which do not look good to me, e.g.: predicate
funcs. v1 is much better.*>
Yeah, I agree adding argument to predicate is not a good idea. However
it seems that the csr(like seed)
itself cannot distinguish whether it's to be written or not except
these two ways(in v1 and v2).
Or we can take seed CSR as a special case in riscv_csrrw_check since no
other csr will limit itself
to be accessed only with read-write instruction currently.
Or maybe, is that possible we do something in trans_csrrs() instead?
That might be a better option. Unless there are other CSRs we expect
to do this I think trans_csrr*() is probably the place to have this,
similar to the `rd == 0` case.
It could also go in helper_csrr() which would just be a simple if check.
Alistair
Sorry. I don't understand what can be done in trans_csrr*(). As I said
in last email: trans_csr*
have made all the read operation will go to the helper_csrr. However
helper_csrr share the progress
of riscv_csrrw* with helper_csrrw/helper_csrw to implement its function.
Yep, so the problem you are trying to solve is the seed CSR access right?
To solve the seed CSR access problem is my initial purpose.
The other purpose is to write back the unmodified value back as the
coments in trans_csrrs/trans_csrrc:
"
* Note that if rs1 specifies a register other than x0, holding
* a zero value, the instruction will still attempt to write the
* unmodified value back to the csr and will cause side effects.
*/
"
For this purpose, we still should distinguish the two cases in
riscv_csrrw, since whether write_mask
is zero is used to distinguish read and write operation currently.
"The seed CSR must be accessed with a read-write instruction. A
read-only instruction such as CSRRS/CSRRC with rs1=x0 or CSRRSI/CSRRCI
with uimm=0 will raise an illegal instruction exception. The write
value (in rs1 or uimm) must be ignored by implementations. The purpose
of the write is to signal polling and flushing."
So trans_csr*() probably isn't the right place, but you could do the
check in helper_csr*()
helper_csrr() is only called from do_csrr(), which should only be
called on an invalid access for the seed CSR.
The truely question is that helper_csrr will call riscv_csrrw*with
write_mask = zero, new_value=zero,
which cannot distinguished from the call from helper_csrrw of which its
write_mask all can be zero
Agreed. But helper_csrrw() isn't called if rs1=x0 or uimm=0. So if
helper_csrrw() is called no matter the write mask then you should be
fine.
So a simple:
if (csr == CSR_SEED) {
riscv_raise_exception()
}
in helper_csrr() and helper_csrw() should be fine. Or am I missing something?
It's OK to do this to solve the seed csr problem. Maybe it's unnecessary
to do this in helper_csrw since the spec
seems not specify the case that rd = x0 to raise an illegal instruction
exception.
Regards,
Weiwei Li
Alistair
(origin from trans_csrrs/trans_csrrc when rs1 is not x0 and the value of
rs1 reg is zero).
Regards,
Weiwei Li
The read and write operation in trans_csr* have been truely
distinguished in original code:
all the read operation will go to the helper_csrr, write to
helper_csrrw, read/write to helper_csrrw.
However, they all go to the same progress riscv_csrrw* in the helper
with different arguments.
Regards,
Weiwei Li
Regards,
Bin
