On Thu, Feb 16, 2023 at 5:29 PM Andrew Jones <[email protected]> wrote: > > On Wed, Feb 15, 2023 at 03:57:18PM -0300, Daniel Henrique Barboza wrote: > > At this moment, and apparently since ever, we have no way of enabling > > RISCV_FEATURE_MISA. This means that all the code from write_misa(), all > > the nuts and bolts that handles how to properly write this CSR, has > > always been a no-op as well because write_misa() will always exit > > earlier. > > > > This seems to be benign in the majority of cases. Booting an Ubuntu > > 'virt' guest and logging all the calls to 'write_misa' shows that no > > writes to MISA CSR was attempted. Writing MISA, i.e. enabling/disabling > > RISC-V extensions after the machine is powered on, seems to be a niche > > use. > > > > Regardless, the spec says that MISA is a WARL read-write CSR, and gating > > the writes in the register doesn't make sense. OS and applications > > should be wary of the consequences when writing it, but the write itself > > must always be allowed. > > The write is already allowed, i.e. no exception is raised when writing it. > The spec only says that the fields may/can be writable. So we can > correctly implement the spec with just > > write_misa() > { > return RISCV_EXCP_NONE; > }
Agree. Such change is still spec compliant without worrying about the bugs. > > as it has effectively been implemented to this point. > > Based on Weiwei Li's pointing out of known bugs, and the fact that > this function has likely never been tested, then maybe we should just > implement it as above for now. Once a better solution to extension > sanity checking exists and a use (or at least test) case arises, then > the function could be expanded with some actually writable bits. (Also, > I think that when/if we do the expansion, then something like the misa_w > config proposed in the previous version of this series may still be > needed in order to allow opting-in/out of the behavior change.) In QEMU RISC-V we have some examples of implementing optional spec features without exposing a config parameter. Do we need to add config parameters for those cases too? If yes, I am afraid the parameters will grow a lot. Regards, Bin
