On Thu, 11 Jan 2018 09:55:36 PST (-0800), Michael Clark wrote:
On Fri, Jan 12, 2018 at 3:37 AM, Richard Henderson < richard.hender...@linaro.org> wrote:On 01/10/2018 06:21 PM, Michael Clark wrote: > +static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, > + target_ulong *cs_base, uint32_t *flags) > +{ > + *pc = env->pc; > + *cs_base = 0; > + *flags = 0; /* necessary to avoid compiler warning */ > +} Actually, at minimum you have to put enough into flags to differentiate machine vs supervisor vs user mode. Otherwise you can wind up running a previously translated block with the wrong permissions. The patch I saw from Stefan O'Rear would do nicely.
Thanks for pointing this out. I've just hopped on board here (and I'm way oversubscribed, so I might not be that responsive), so I'm sure to have missed a lot.
OK. I didn't see the patch. I'll trawl through my email for the last couple of weeks before the next spin.
I think this is it https://github.com/riscv/riscv-qemu/commit/a038a2874a3eba27650c164f4622e47a3fe95199.patch which I've blindly applied to our repo here: https://github.com/riscv/riscv-qemu/pull/93 We'll be sure to fix it for the v3. Thanks!
So any CPU state that can effect translation should be in flags?
That's the first comment in the patch -- though that about exhausts my knowledge of it :)
+/* tb_flags must contain all information that affects execution of ordinary + * instructions (helpers can look at the CPURISCVState) */
Assuming that is the case, we could probably join mode with the lower 26 bits of misa and we probably should do a tb_flush on misa CSR writes if any bits are changed. i.e. if extensions such as MAFDC are added or removed (muldiv, atomics, fp-single, fp-double, compressed).
I agree: I think any MISA bit change would result in different translation behavior.
