| Message ID | 20220715060740.1500628-2-yangxiaojuan@loongson.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix LoongArch coverity error and cpu name bug | expand |
On 7/15/22 11:37, Xiaojuan Yang wrote: > In loongarch_cpu_class_by_name(char *cpu_model) function, > the argument cpu_model already has the suffix '-loongarch-cpu', > so we should remove the LOONGARCH_CPU_TYPE_NAME(cpu_model) macro. > And add the assertion that 'cpu_model' resolves to a class of the > appropriate type. > > Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> This patch causes tests to fail, e.g. TEST float_convd on loongarch64 qemu-loongarch64: unable to find CPU model 'la464' make[1]: *** [/home/rth/qemu/src/tests/tcg/multiarch/Makefile.target:29: run-float_convd] Error 1 What caused you assume that all cpu models are already suffixed? r~
On 7/19/22 12:16, Richard Henderson wrote: > On 7/15/22 11:37, Xiaojuan Yang wrote: >> In loongarch_cpu_class_by_name(char *cpu_model) function, >> the argument cpu_model already has the suffix '-loongarch-cpu', >> so we should remove the LOONGARCH_CPU_TYPE_NAME(cpu_model) macro. >> And add the assertion that 'cpu_model' resolves to a class of the >> appropriate type. >> >> Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > This patch causes tests to fail, e.g. > > TEST float_convd on loongarch64 > > qemu-loongarch64: unable to find CPU model 'la464' > > make[1]: *** [/home/rth/qemu/src/tests/tcg/multiarch/Makefile.target:29: run-float_convd] > Error 1 > > > What caused you assume that all cpu models are already suffixed? Mm. I suppose the use over in hw/loongarch/loongson3.c. I will make this function match target/alpha/cpu.c, which checks cpu_model as-is, and then tries again with the suffix. r~
On Tue, 19 Jul 2022 12:22:14 +0530 Richard Henderson <richard.henderson@linaro.org> wrote: > On 7/19/22 12:16, Richard Henderson wrote: > > On 7/15/22 11:37, Xiaojuan Yang wrote: > >> In loongarch_cpu_class_by_name(char *cpu_model) function, > >> the argument cpu_model already has the suffix '-loongarch-cpu', > >> so we should remove the LOONGARCH_CPU_TYPE_NAME(cpu_model) macro. > >> And add the assertion that 'cpu_model' resolves to a class of the > >> appropriate type. > >> > >> Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn> > >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > > > This patch causes tests to fail, e.g. > > > > TEST float_convd on loongarch64 > > > > qemu-loongarch64: unable to find CPU model 'la464' > > > > make[1]: *** [/home/rth/qemu/src/tests/tcg/multiarch/Makefile.target:29: run-float_convd] > > Error 1 > > > > > > What caused you assume that all cpu models are already suffixed? > > Mm. I suppose the use over in hw/loongarch/loongson3.c. > I will make this function match target/alpha/cpu.c, which checks cpu_model as-is, and then > tries again with the suffix. if we think about adding hotplug/qmp/-device support for CPUs in future, those interface operate with type-names directly without any pre-processing whatsoever. so I'd very much prefer just as-is (i.e. direct type-name matching) (all other model parsing usually comes from legacy code) > > > r~ >
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c index e21715592a..ed26f9beed 100644 --- a/target/loongarch/cpu.c +++ b/target/loongarch/cpu.c @@ -571,11 +571,12 @@ static void loongarch_cpu_init(Object *obj) static ObjectClass *loongarch_cpu_class_by_name(const char *cpu_model) { ObjectClass *oc; - char *typename; - typename = g_strdup_printf(LOONGARCH_CPU_TYPE_NAME("%s"), cpu_model); - oc = object_class_by_name(typename); - g_free(typename); + oc = object_class_by_name(cpu_model); + if (!oc || !object_class_dynamic_cast(oc, TYPE_LOONGARCH_CPU) || + object_class_is_abstract(oc)) { + return NULL; + } return oc; }