More rigorous handling of reset

[Timmmm]

Currently the model doesn't have very clear or rigorous handling of chip reset. I think we should do the following:

1. Rename all of these init functions to reset. This makes it clear that the semantics correspond to resetting the chip, which can be done many times per simulation. As opposed to initialising the model (the generated C function model_init()) which should only be done once at the start of simulation. It's also kind of confusing that there's a C model_init() function and a Sail init_model() function that do different things.

/* initialize model state */
function init_model () -> unit = {
  init_platform (); /* devices */
  init_sys ();      /* processor */
  init_vmem ();     /* virtual memory */

  /* initialize extensions last */
  ext_init ();
  ext_init_regs ();
}

2. Go through the init_ functions and make sure they do only reset the things that they are supposed to according to the spec. The spec doesn't require many registers to be reset. For example we shouldn't be zeroing mscratch there.

3. In the distant future load reset values from riscv-config and apply them.

See also #126

[allenjbaum]

Regarding resetting things: there needs to be a hook to allow user-defined reset values to CSRs. This is probably something that will be part of the who configuration issue, but putting the hook in now, even if unused, is probably wise.

On Thu, Jan 4, 2024 at 4:20 AM Tim Hutt @.***> wrote:

Currently the model doesn't have very clear or rigorous handling of chip reset. I think we should do the following:

1. Rename all of these init functions to reset. This makes it clear that the semantics correspond to resetting the chip, which can be done many times per simulation. As opposed to initialising the model (the generated C function model_init()) which should only be done once at the start of simulation. It's also kind of confusing that there's a C model_init() function and a Sail init_model() function that do different things.

/* initialize model state / function init_model () -> unit = { init_platform (); / devices / init_sys (); / processor / init_vmem (); / virtual memory */

/* initialize extensions last */ ext_init (); ext_init_regs (); }

Go through the init_ functions and make sure they do only reset the things that they are supposed to according to the spec. The spec doesn't require many registers to be reset. For example we shouldn't be zeroing mscratch there. 2.

In the distant future load reset values from riscv-config and apply them.

See also #126 https://github.com/riscv/sail-riscv/issues/126

— Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/issues/383, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJUWVBD2R4MO44GHMWDYM2M7JAVCNFSM6AAAAABBM26BKSVHI2DSMVQWIX3LMV43ASLTON2WKOZSGA3DKNJVHE2TEOI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[martinberger]

@allenjbaum

How about something like this: For every register: (CSRs and others)

• An explicit Sail write-legalisation function, that defaults to the identity (i.e. every value is legal), except where the RV standard requires something else.
• An explicit Sail function that intialises in reset. By default it calls the associated write-legalisation function with 0, except where the RV standard requires something else.

All defaults should have hooks so users can hook in their own write-legalisations and reset values (there is a large design space how to handle hooks). All the above Sail functions should be in their own file, ordered by register id or something like this, so it's easy to check that nothing is forgotten, and also easy to auto-generate.

[allenjbaum]

The default init function should be "unknown" , not zero (unless the spec says otherwise, which it rarely does.)

The default write legalisation function is the identity - but that isn't quite enough, because of value dependencies, e.g. changing one CSR might change the legalization function of a different CSR, so that other one needs to get updated as well. I don't actually recall if changing one CSR value that causes another CSR value to disappear simply masks the value (so, state is retained but unobservable until the first CSR value is restored), or if the state goes back to its reset value when the value is restored, but maybe that must be defined in the hook. e.g. if misa.S is cleared, then the SATP register ceases to exist.

And of course now it is (or will be) legal to not trap on the access to non-existent CSRs

• so that is yet another option we have to define for every defined CSR (and maybe even undefined ones) since there is no way to know what will happen if you try. We can have Sail choose between the two likely options: trap and returning zero, similar to what we do for WARL fields

On Sat, Jan 6, 2024 at 4:48 AM Martin Berger @.***> wrote:

@allenjbaum https://github.com/allenjbaum

How about something like this: For every register: (CSRs and others)

• An explicit Sail write-legalisation function, that defaults to the identity (i.e. every value is legal), except where the RV standard requires something else.
• An explicit Sail function that intialises in reset. By default it calls the associated write-legalisation function with 0, except where the RV standard requires something else.

All defaults should have hooks so users can hook in their own write-legalisations and reset values (there is a large design space how to handle hooks). All the above Sail functions should be in their own file, ordered by register id or something like this, so it's easy to check that nothing is forgotten, and also easy to auto-generate.

— Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/issues/383#issuecomment-1879672456, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJSSEXR5VWOKV4NLF23YNFBYFAVCNFSM6AAAAABBM26BKSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZZGY3TENBVGY . You are receiving this because you were mentioned.Message ID: @.***>

[bacam]

I noticed recently that the init_ functions don't reset MIE and MPRV in mstatus, despite the prose spec saying that reset should do that. I wasn't entirely sure what to make of that at the time because nothing in the Sail spec actually says that these functions are supposed to model reset.