qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v9 01/23] target/riscv: Move MISA limits to class


From: Akihiko Odaki
Subject: Re: [PATCH v9 01/23] target/riscv: Move MISA limits to class
Date: Thu, 12 Oct 2023 15:01:45 +0900
User-agent: Mozilla Thunderbird

On 2023/10/12 0:23, Alex Bennée wrote:

Akihiko Odaki <akihiko.odaki@daynix.com> writes:

MISA limits are common for all instances of a RISC-V CPU class so they
are better put into class.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
  target/riscv/cpu-qom.h   |   2 +
  target/riscv/cpu.h       |   2 -
  hw/riscv/boot.c          |   2 +-
  target/riscv/cpu.c       | 212 +++++++++++++++++++++++++++------------
  target/riscv/csr.c       |   3 +-
  target/riscv/gdbstub.c   |  12 ++-
  target/riscv/machine.c   |  11 +-
  target/riscv/translate.c |   3 +-
  8 files changed, 167 insertions(+), 80 deletions(-)

diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h
index 04af50983e..266a07f5be 100644
--- a/target/riscv/cpu-qom.h
+++ b/target/riscv/cpu-qom.h
@@ -67,5 +67,7 @@ struct RISCVCPUClass {
      /*< public >*/
      DeviceRealize parent_realize;
      ResettablePhases parent_phases;
+    uint32_t misa_mxl_max;  /* max mxl for this cpu */
+    uint32_t misa_ext_mask; /* max ext for this cpu */
  };
  #endif /* RISCV_CPU_QOM_H */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index ef9cf21c0c..9f9cb6cd2a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -155,9 +155,7 @@ struct CPUArchState {
/* RISCVMXL, but uint32_t for vmstate migration */
      uint32_t misa_mxl;      /* current mxl */
-    uint32_t misa_mxl_max;  /* max mxl for this cpu */
      uint32_t misa_ext;      /* current extensions */
-    uint32_t misa_ext_mask; /* max ext for this cpu */
      uint32_t xl;            /* current xlen */
/* 128-bit helpers upper part return value */
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 52bf8e67de..b7cf08f479 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -36,7 +36,7 @@
bool riscv_is_32bit(RISCVHartArrayState *harts)
  {
-    return harts->harts[0].env.misa_mxl_max == MXL_RV32;
+    return RISCV_CPU_GET_CLASS(&harts->harts[0])->misa_mxl_max ==
  MXL_RV32;

I'm going to defer to the RISCV maintainers here. While I agree the
class is a good place for these parameters that are shared across
multiple vCPUS there is a cost to RISCV_CPU_GET_CLASS() casting.

You might notice we have this comment in include/hw/core/cpu.h:

   /*
    * The class checkers bring in CPU_GET_CLASS() which is potentially
    * expensive given the eventual call to
    * object_class_dynamic_cast_assert(). Because of this the CPUState
    * has a cached value for the class in cs->cc which is set up in
    * cpu_exec_realizefn() for use in hot code paths.
    */
   typedef struct CPUClass CPUClass;
   DECLARE_CLASS_CHECKERS(CPUClass, CPU,
                          TYPE_CPU)

It is fine since these variables are cached into DisasContext during TCG translation as far as I can tell.


However I think you need to check the assumption that you will never see
multiple cores with different RISCV properties.

I missed misa_ext_mask can be different so I changed to only convert misa_mxl_max into a class member in "[PATCH 0/4] gdbstub and TCG plugin improvements".



reply via email to

[Prev in Thread] Current Thread [Next in Thread]