[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 02/10] target/riscv: always allow write_misa() to write MI
|
From: |
Bin Meng |
|
Subject: |
Re: [PATCH v3 02/10] target/riscv: always allow write_misa() to write MISA |
|
Date: |
Thu, 16 Feb 2023 17:33:55 +0800 |
On Thu, Feb 16, 2023 at 5:29 PM Andrew Jones <ajones@ventanamicro.com> 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
[PATCH v3 03/10] target/riscv: introduce riscv_cpu_cfg(), Daniel Henrique Barboza, 2023/02/15
[PATCH v3 05/10] target/riscv/cpu.c: error out if EPMP is enabled without PMP, Daniel Henrique Barboza, 2023/02/15
[PATCH v3 06/10] target/riscv: remove RISCV_FEATURE_EPMP, Daniel Henrique Barboza, 2023/02/15
[PATCH v3 04/10] target/riscv: remove RISCV_FEATURE_DEBUG, Daniel Henrique Barboza, 2023/02/15
[PATCH v3 07/10] target/riscv: remove RISCV_FEATURE_PMP, Daniel Henrique Barboza, 2023/02/15
[PATCH v3 08/10] hw/riscv/virt.c: do not use RISCV_FEATURE_MMU in create_fdt_socket_cpus(), Daniel Henrique Barboza, 2023/02/15
[PATCH v3 09/10] target/riscv: remove RISCV_FEATURE_MMU, Daniel Henrique Barboza, 2023/02/15