Re: [Qemu-devel] [PATCH v3 03/21] RISC-V CPU Core Definition

[Palmer Dabbelt]

On Thu, 11 Jan 2018 09:55:36 PST (-0800), Michael Clark wrote:
> On Fri, Jan 12, 2018 at 3:37 AM, Richard Henderson <
> address@hidden> 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.