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.od...@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.
All this considered, there's still one extra class cast that we will have to
deal with
in riscv_tr_init_disas_context(). I am not sure how hot this function is but,
taking a
look at other .init_disas_context implementations from other archs and not
finding class
casts in them, I'm worried about the overhead it'll add. It seems like we can
just do
"ctx->misa_mxl_max = env->misa_mxl" to avoid it.
Thanks,
Daniel