qemu-ppc
[Top][All Lists]
Advanced

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

Re: [PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field
Date: Wed, 11 Oct 2023 05:28:49 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.15.1

Hi Gavin,

On 25/9/23 02:24, Gavin Shan wrote:
On 9/12/23 08:40, Gavin Shan wrote:
On 9/11/23 19:43, Philippe Mathieu-Daudé wrote:
On 11/9/23 01:28, Gavin Shan wrote:
On 9/8/23 21:22, Philippe Mathieu-Daudé wrote:
Add a field to return the QOM type name of a CPU class.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---


diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 129d179937..e469efd409 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -100,6 +100,7 @@ struct SysemuCPUOps;
  /**
   * CPUClass:
+ * @cpu_resolving_type: CPU QOM type name
   * @class_by_name: Callback to map -cpu command line model name to an
   *                 instantiatable CPU type.
   * @parse_features: Callback to parse command line arguments.
@@ -148,6 +149,7 @@ struct CPUClass {
      DeviceClass parent_class;
      /*< public >*/
+    const char *cpu_resolving_type;
      ObjectClass *(*class_by_name)(const char *cpu_model);
      void (*parse_features)(const char *typename, char *str, Error **errp);

The question is why not use CPU_RESOLVING_TYPE directly? It seems CPU_RESOLVING_TYPE
is exactly what you want here.

Unfortunately CPU_RESOLVING_TYPE is target-specific, we want
hw/core/cpu-common.c to be target-agnostic (build once for all
targets). This is particularly important in the context of
heterogeneous QEMU, where a single binary will be able to create
CPUs from different targets.


CPU_RESOLVING_TYPE and CPUClass::cpu_resolving_type is duplicate to
each other. There are two options I can figure out to avoid the
duplication.

(a) move cpu_class_by_name() from hw/core/cpu-common.c to cpu.c, so that
     CPU_RESOLVING_TYPE can be seen. cpu.c::list_cpus() is the example.

(b) remove hw/core/cpu-common.c::cpu_calss_by_name() and squeeze its
     logic to cpu.c::parse_cpu_option() since there are not too much
     users for it. target/arm and target/s390 needs some tweaks so that
     hw/core/cpu-common.c::cpu_calss_by_name() can be removed.

     [gshan@gshan q]$ git grep \ cpu_class_by_name\(
     cpu.c:    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);      target/arm/arm-qmp-cmds.c:    oc = cpu_class_by_name(TYPE_ARM_CPU, model->name);      target/s390x/cpu_models_sysemu.c:    oc = cpu_class_by_name(TYPE_S390_CPU, info->name);

When option (b) is taken, this series to have the checks against @oc
in hw/core/cpu-common.c::cpu_calss_by_name() becomes non-sense. Instead,
we need the same (and complete) checks in CPUClass::class_by_name() for
individual targets. Further more, an inline helper can be provided to do
the check in CPUClass::class_by_name() for individual targets.

    include/hw/core/cpu.h

    static inline bool cpu_class_is_valid(ObjectClass *oc, const char *parent)
    {
        if (!object_class_dynamic_cast(oc, parent) ||
            object_class_is_abstract(oc)) {
            return false;
        }

        return true;
    }


Since my series to make CPU type check unified depends on this series, could
you please share your thoughts? If you don't have bandwidth for this, I can
improve the code based on your thoughts, and include your patches to my series
so that they can be reviewed at once. Please just let me know.

You seem to prove (b) is not useful, so we have to do (a).

Unfortunately at this moment I feel hopeless with this topic.

I don't want to delay your work further. If you find a way to integrate
both series, please go ahead. Otherwise let's drop my approach and
continue with your previous work.

I apologize I kept you waiting that long.

Regards,

Phil.




reply via email to

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