On 2022/11/18 04:57, Richard Henderson wrote:
On 11/17/22 03:44, weiwei wrote:
Missing a smstateen_check. Not mentioned in the instruction
description itself, but it is within the State Enable section of JVT.
smstateen_check have been added in REQUIRE_ZCMT.
Oh. I see. That's wrong, I think.
Returning false from trans_* means "no match" and continue on to try
and match another pattern. If Zcmt is present in the cpu, but the
extension is not enabled by the OS, we have found the matching insn
and should not look for another insn.
You need to separate the check like
REQUIRE_ZCMT(ctx);
if (!smstateen_check(ctx, 0, SMTATEEN0_JVT)) {
gen_exception_illegal(ctx);
return true;
}
I see that the fpcr code that you're modifying in this patch, which is
not yet upstream, is also incorrect in this.
Looking back through your git history,
https://github.com/plctlab/plct-qemu/commit/09668167880c492f88b74d0a921053ed25fc3b5c
is incorrect:
Yeah. This patchset is not yet upstream but have been added to
riscv-to-apply.next. I also suggested similar way
in this patchset at the beginning. However, to some extent, JVT and
FCSR in statenen CSR are used to enable/disable
Zfinx and Zcmt extensions. When they are disabled, It seems reasonable
to look for another insn, just like the
processor doesn't support them at all.
From the other aspect, is it possible that we support many overlapping
extensions(such as Zcmt and Zcd or CD) in one
processor and only one work once (just disable anothers if we need
another to work)?
static inline bool smstateen_fcsr_check(DisasContext *ctx, int index)
{
CPUState *cpu = ctx->cs;
CPURISCVState *env = cpu->env_ptr;
uint64_t stateen = env->mstateen[index];
You cannot read from env during translation like this.
Everything that you use for translation must be encoded in tb->flags.
Otherwise the state will not be considered when selecting an existing
TranslationBlock to execute, and the next execution through this
instruction will not have the *current* state of env.
You probably get lucky with mstateen, because I could imagine that it
gets set once while the OS is booting and is never changed again. If
true, then mstateen chould be treated like misa and flush all
translations on write: see write_misa(). And also add a large comment
to smstateen_check() explaining why the read from env is correct.
But if that "set once" assumption is not true, and mstateen is more
like mstatus.fs, where a given extension is enabled/disabled often for
lazy migration of state, then you won't want to continually flush
translations.
Yeah. I didn't realize this question. I think we can use a specific
helper to do this check, since tb_flags may not be big enough to catch
all the information of xStateen csr.
r~
