Re: [PATCH v6 01/11] sysemu/tcg: Introduce tcg_builtin() helper

[Philippe Mathieu-Daudé]

On 1/31/21 3:18 PM, Claudio Fontana wrote:
> On 1/31/21 12:50 PM, Philippe Mathieu-Daudé wrote:
>> Modules are registered early with type_register_static().
>>
>> We would like to call tcg_enabled() when registering QOM types,
> 
> 
> Hi Philippe,
> 
> could this not be controlled by meson at this stage?
> On X86, I register the tcg-specific types in tcg/* in modules that are only 
> built for TCG.
> 
> Maybe tcg_builtin() is useful anyway, thinking long term at loadable modules,
> but there we are interested in whether tcg code is available or not, 
> regardless of whether it's builtin,
> or needs to be loaded via a .so plugin..
> 
> maybe tcg_available()?

The alternatives I found:

- reorder things in vl.c?

- use ugly #ifdef'ry, see this patch:
  https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg08037.html

- this earlier approach I previously discarded:

-- >8 --
diff --git a/include/qom/object.h b/include/qom/object.h
index d378f13a116..30590c6fac3 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -403,9 +403,12 @@ struct Object
  *   parent class initialization has occurred, but before the class itself
  *   is initialized.  This is the function to use to undo the effects of
  *   memcpy from the parent class to the descendants.
- * @class_data: Data to pass to the @class_init,
+ * @class_data: Data to pass to the @class_registerable, @class_init,
  *   @class_base_init. This can be useful when building dynamic
  *   classes.
+ * @registerable: This function is called when modules are registered,
+ *   prior to any class initialization. When present and returning %false,
+ *   the type is not registered, the class is not present (not usable).
  * @interfaces: The list of interfaces associated with this type.  This
  *   should point to a static array that's terminated with a zero filled
  *   element.
@@ -428,6 +431,7 @@ struct TypeInfo
     void (*class_base_init)(ObjectClass *klass, void *data);
     void *class_data;

+    bool (*registerable)(void *data);
     InterfaceInfo *interfaces;
 };

diff --git a/qom/object.c b/qom/object.c
index 2fa0119647c..0febaffa12e 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -138,6 +138,10 @@ static TypeImpl *type_new(const TypeInfo *info)
 static TypeImpl *type_register_internal(const TypeInfo *info)
 {
     TypeImpl *ti;
+
+    if (info->registerable && !info->registerable(info->class_data)) {
+        return NULL;
+    }
     ti = type_new(info);

     type_table_add(ti);

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 990509d3852..1a2b1889da4 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -24,6 +24,7 @@
 #include "hw/loader.h"
 #include "hw/arm/boot.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/tcg.h"
 #include "qom/object.h"

 #define SMPBOOT_ADDR    0x300 /* this should leave enough space for
ATAGS */
@@ -368,18 +369,26 @@ static void raspi3b_machine_class_init(ObjectClass
*oc, void *data)
 };
 #endif /* TARGET_AARCH64 */

+static bool raspi_machine_requiring_tcg_accel(void *data)
+{
+    return tcg_builtin();
+}
+
 static const TypeInfo raspi_machine_types[] = {
     {
         .name           = MACHINE_TYPE_NAME("raspi0"),
         .parent         = TYPE_RASPI_MACHINE,
+        .registerable   = raspi_machine_requiring_tcg_accel,
         .class_init     = raspi0_machine_class_init,
     }, {
         .name           = MACHINE_TYPE_NAME("raspi1ap"),
         .parent         = TYPE_RASPI_MACHINE,
+        .registerable   = raspi_machine_requiring_tcg_accel,
         .class_init     = raspi1ap_machine_class_init,
     }, {
         .name           = MACHINE_TYPE_NAME("raspi2b"),
         .parent         = TYPE_RASPI_MACHINE,
+        .registerable   = raspi_machine_requiring_tcg_accel,
         .class_init     = raspi2b_machine_class_init,
 #ifdef TARGET_AARCH64
     }, {
---

> 
> Ciao,
> 
> Claudio
> 
>> but tcg_enabled() returns tcg_allowed which is a runtime property
>> initialized later (See commit 2f181fbd5a9 which introduced the
>> MachineInitPhase in "hw/qdev-core.h" representing the different
>> phases of machine initialization and commit 0427b6257e2 which
>> document the initialization order).
>>
>> As we are only interested if the TCG accelerator is builtin,
>> regardless of being enabled, introduce the tcg_builtin() helper.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> Cc: Markus Armbruster <armbru@redhat.com>
>> ---
>>  include/sysemu/tcg.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
>> index 00349fb18a7..6ac5c2ca89d 100644
>> --- a/include/sysemu/tcg.h
>> +++ b/include/sysemu/tcg.h
>> @@ -13,8 +13,10 @@ void tcg_exec_init(unsigned long tb_size, int splitwx);
>>  #ifdef CONFIG_TCG
>>  extern bool tcg_allowed;
>>  #define tcg_enabled() (tcg_allowed)
>> +#define tcg_builtin() 1
>>  #else
>>  #define tcg_enabled() 0
>> +#define tcg_builtin() 0
>>  #endif
>>  
>>  #endif
>>
>