On 10/18/23 10:31, Akihiko Odaki wrote:
On 2023/10/18 22:01, Daniel Henrique Barboza wrote:
On 10/17/23 15:53, Akihiko Odaki wrote:
misa_mxl_max is 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>
---
I'll repeat what I said in the v1: this patch is adding an extra
class parameter,
an extra param required to each class_init, and an extra CPUClass
cast every time
we want to read misa_mxl_max, all of that because we want to assign
gdb_core_xml_file
earlier.
If my previous suggestion of assigning gdb_core directly into
riscv_cpu_class_init()
doesn't work necause we need misa_mxl_max to do it, a good
alternative is setting
gdb_core_xml_file in riscv_cpu_post_init(), which is executed after
all cpu_init()
functions where we already have env->misa_mxl_max set properly.
We want to assign gdb_core_xml_file *earlier* so assigning it after
cpu_init() is not OK. In general it should be considered unsafe to
initialize a class variable after class_init(); otherwise other
subsystems cannot know when it becomes available.
There's no difference in assigning it during .instance_init (cpu_init())
and
.instance_post_init. The callbacks are called one after the other during
init time. See qom/object.c, object_initialize_with_type().
Now, if we want to be strict with only initializing class variables
during class_init(),
which is something that I will defer to the maintainers, we want to
minimize the use
of env->misa_mxl_max in the code before converting it to a class
variable. env->misa_mxl
is always equal to env->misa_mxl_max, and they are being used
interchangeably, but if
misa_mxl_max is going to be a class variable then we want to use
env->misa_mxl instead
to avoid the RISCV_CPU_GET_CLASS() overhead.
I suggest a pre-patch to change the uses of env->misa_mxl_max to
env->misa_mxl in
riscv_is_32bit(), riscv_cpu_gdb_read_register(),
riscv_cpu_gdb_write_register() and
so on (there are quite a few places). And then apply this patch on top
of it. This would
at least minimize the amount of casts being done.