On 4/25/25 15:20, Daniel Henrique Barboza wrote:
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index c52c87faae..992ec8ebff 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -2111,10 +2111,13 @@ static RISCVException write_misa(CPURISCVState *env,
int csrno,
val &= env->misa_ext_mask;
/*
- * Suppress 'C' if next instruction is not aligned
- * TODO: this should check next_pc
+ * Suppress 'C' if next instruction is not aligned.
+ * Outside of the context of a running cpu, env->pc contains next_pc.
+ * Within the context of a running cpu, env->pc contains the pc of
+ * the csrw/csrrw instruction. But since all such instructions are
+ * exactly 4 bytes, next_pc has the same alignment mod 4.
By "outside of the context of a running CPU" you mean a halted CPU, correct?
Correct, e.g. from gdbstub.
And now, for a running CPU, env->pc has the pc of csrw/csrrw because of patch 1.
Correct.
Otherwise it would contain a pc that was synchronized via the last
synchronize_from_tb, or any other insn that happened to sync env->pc in
the same TB via gen_update_pc(). We're not keeping env->pc up to date all
the time because it would be extra work that wouldn't be needed most of the
time. Am I too off the mark?
Correct.
The reason I'm asking is because I see at least one place
(get_physical_address())
where it's stated that "the env->pc at this point is incorrect".
Correct, since get_physical_address is called from riscv_cpu_tlb_fill, which can be called
during the processing of any guest memory operation.
And I see env->pc
being either the current PC or the next insn PC depending on the situation.
Reading these 2 patches clarified it a bit (assuming I'm not completely
incorrect,
of course).
I would expect env->pc to be more or less random in get_physical_address, with a bias
toward the PC of the first instruction of the TB.
I'm not sure why get_physical_address has that comment. The current pc isn't relevant to
resolving a virtual address. It only becomes relevant when there is a fault, and the pc
is restored via unwinding along the fault path.
r~
