Re: [PATCH for-7.1 v6 26/51] target/nios2: Prevent writes to read-only or reserved control fields

[Richard Henderson]

On 3/17/22 08:49, Peter Maydell wrote:
> On Thu, 17 Mar 2022 at 05:53, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> > Create an array of masks which detail the writable and readonly
> > bits for each control register.  Apply them when writing to
> > control registers, including the write to status during eret.
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
> > @@ -34,6 +34,15 @@ void helper_raise_exception(CPUNios2State *env, uint32_t 
> > index)
> >   #ifndef CONFIG_USER_ONLY
> >   void helper_eret(CPUNios2State *env, uint32_t new_status, uint32_t new_pc)
> >   {
> > +    Nios2CPU *cpu = env_archcpu(env);
> > +
> > +    /*
> > +     * Both estatus and bstatus have no constraints on write;
> > +     * do not allow reserved fields in status to be set.
> > +     */
> > +    new_status &= (cpu->cr_state[CR_STATUS].writable |
> > +                   cpu->cr_state[CR_STATUS].readonly);
> > +
> >       env->ctrl[CR_STATUS] = new_status;
>
> Isn't this allowing the guest to write to readonly bits ?

Well, CPS is certainly required to be set by eret -- that's a difference between eret and 
wrctl.  However, I've just noticed a comment on page 3-58:

> Do not set status.PIE in a nonmaskable ISR. If status.PIE is set, a maskable 
> interrupt can pre-
> empt an NMI, and the processor exits NMI mode. It cannot be returned to NMI 
> mode until the
> next nonmaskable interrupt.

which suggests that eret does not restore NMI from estatus, as saved by normal 
interrupt.

So I guess this should be just writable | CPS_MASK.


r~