On Tue, 8 Mar 2022 at 07:20, Richard Henderson <richard.hender...@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. > > Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c > index f2813d3b47..189adf111c 100644 > --- a/target/nios2/cpu.c > +++ b/target/nios2/cpu.c > @@ -88,6 +88,55 @@ static void nios2_cpu_initfn(Object *obj) > > cpu_set_cpustate_pointers(cpu); > > + /* Begin with all fields of all registers are reserved. */ > + memset(cpu->cr_state, 0, sizeof(cpu->cr_state)); > + > + /* > + * The combination of writable and readonly is the set of all > + * non-reserved fields. We apply writable as a mask to bits, > + * and merge in existing readonly bits, before storing. > + */ > +#define WR_REG(C) cpu->cr_state[C].writable = -1 > +#define RO_REG(C) cpu->cr_state[C].readonly = -1 > +#define WR_FIELD(C, F) cpu->cr_state[C].writable |= R_##C##_##F##_MASK > +#define RO_FIELD(C, F) cpu->cr_state[C].readonly |= R_##C##_##F##_MASK > + > + WR_FIELD(CR_STATUS, PIE); I think you need to claim (CR_STATUS, RSIE) is a RO bit, because without EIC it's should-be-one. > + WR_REG(CR_ESTATUS); > + WR_REG(CR_BSTATUS); > + RO_REG(CR_CPUID); > + WR_FIELD(CR_EXCEPTION, CAUSE); > + WR_REG(CR_BADADDR); > + > + /* TODO: These control registers are not present with the EIC. */ > + WR_REG(CR_IENABLE); > + RO_REG(CR_IPENDING); Missing CR_CONFIG register ? > + > + if (cpu->mmu_present) { > + WR_FIELD(CR_STATUS, U); > + WR_FIELD(CR_STATUS, EH); True by the documentation, but we don't seem to prevent EH from being set to 1 when we take an exception on the no-MMU config... > + > + WR_FIELD(CR_PTEADDR, VPN); > + WR_FIELD(CR_PTEADDR, PTBASE); > + > + RO_FIELD(CR_TLBMISC, D); > + RO_FIELD(CR_TLBMISC, PERM); > + RO_FIELD(CR_TLBMISC, BAD); > + RO_FIELD(CR_TLBMISC, DBL); > + WR_FIELD(CR_TLBMISC, WR); (the docs call this field 'WE', incidentally) > + WR_FIELD(CR_TLBMISC, RD); If you claim this bit to be writable you'll allow the gdbstub to set it, which is probably not what you want. (Actual writes to this register are handled via the helper function.) > + WR_FIELD(CR_TLBMISC, WAY); Missing PID field ? > + > + WR_REG(CR_TLBACC); > + } You don't enforce the reserved/readonly bits on status when we copy it from estatus during eret. (That change appears later, in patch 26.) The same *ought* to apply for bret, except that we have a bug in our implementation of it, where we fail to copy bstatus into status... The machinery itself looks OK. thanks -- PMM