Re: [PATCH v7 00/10] make write_misa a no-op and FEATURE_* cleanups

[Palmer Dabbelt]

On Wed, 22 Feb 2023 10:51:55 PST (-0800), dbarboza@ventanamicro.com wrote:
> Hi,
>
> In this version we gave up removing all the write_misa() body and,
> instead, we went back to something closer to what we were doing in v2.
> write_misa() is now gated behind an experimental x-misa-w cfg option,
> defaulted to false.
>
> The idea is that x-misa-w allow us to keep experimenting and testing the
> code. Marking it as experimental will (hopefully) make users wary of the
> fact that this feature is unstable. The expectation is that the flag will
> be removed once write_misa() is ready to always write MISA.
>
> Changes from v6:
> - patches without reviews/acks: patch 3
> - patch 2: taken from version 3, acks and r-bs preserved
> - patch 3:
>   - rename 'misa-w' to 'x-misa-w' to be clearer about our intents with
>     the cfg option
> - v6 link: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg05047.html
>
> Daniel Henrique Barboza (10):
>   target/riscv: introduce riscv_cpu_cfg()
>   target/riscv: do not mask unsupported QEMU extensions in write_misa()
>   target/riscv: allow MISA writes as experimental
>   target/riscv: remove RISCV_FEATURE_DEBUG
>   target/riscv/cpu.c: error out if EPMP is enabled without PMP
>   target/riscv: remove RISCV_FEATURE_EPMP
>   target/riscv: remove RISCV_FEATURE_PMP
>   hw/riscv/virt.c: do not use RISCV_FEATURE_MMU in
>     create_fdt_socket_cpus()
>   target/riscv: remove RISCV_FEATURE_MMU
>   target/riscv/cpu: remove CPUArchState::features and friends
>
>  hw/riscv/virt.c           |  7 ++++---
>  target/riscv/cpu.c        | 25 ++++++++++---------------
>  target/riscv/cpu.h        | 29 ++++++-----------------------
>  target/riscv/cpu_helper.c |  6 +++---
>  target/riscv/csr.c        | 15 ++++++---------
>  target/riscv/machine.c    | 11 ++++-------
>  target/riscv/monitor.c    |  2 +-
>  target/riscv/op_helper.c  |  2 +-
>  target/riscv/pmp.c        |  8 ++++----
>  9 files changed, 39 insertions(+), 66 deletions(-)

I just queued this up, using the text from the v1 as that's more of a 
description of the patch set.  I think that text is still sufficiently 
accurate, but let me know if I missed anything.  Here's what I ended up 
with

   Merge patch series "make write_misa a no-op and FEATURE_* cleanups"
   
   Daniel Henrique Barboza <dbarboza@ventanamicro.com> says:
   
   The RISCV_FEATURES_* enum and the CPUArchState::features attribute were
   introduced 4+ years ago, as a way to retrieve the enabled hart features
   that aren't represented via MISA CSR bits. Time passed on, and
   RISCVCPUConfig was introduced. With it, we now have a centralized way of
   reading all hart features that are enabled/disabled by the user and the
   board. All recent features are reading their correspondent cpu->cfg.X
   flag.
   
   All but the 5 features in the RISCV_FEATURE_* enum. These features are
   still operating in the same way: set it during riscv_cpu_realize() using
   their cpu->cfg value, read it using riscv_feature() when needed. There
   is nothing special about them in comparison with all the other features
   and extensions to justify this special handling.
   
   This series then is doing two things: first we're actually allowing
   users to write the MISA CSR if they so choose. Then we're deprecate each
   RISC_FEATURE_* usage until, in patch 11, we remove everything related to
   it. All 5 existing RISCV_FEATURE_* features will be handled as everyone
   else.

Thanks!