Re: [PATCH v6 01/11] sysemu/tcg: Introduce tcg_builtin() helper
On 1/31/21 4:23 PM, Philippe Mathieu-Daudé wrote:
> 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
Not sure it's that ugly,
if it is followed (or replaced by) exporting those pieces to separate files,
which are only built by meson on CONFIG_TCG.
I did not try to do it, so you know best of course.
Ciao,
Claudio
>
> - 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_ADDR0x300 /* 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é
>>> ---
>>> Cc: Markus Armbruster
>>> ---
>>> include/sysemu/tcg.h | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/include/sysemu/tcg.h
Re: [PATCH v6 01/11] sysemu/tcg: Introduce tcg_builtin() helper
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_ADDR0x300 /* 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é
>> ---
>> Cc: Markus Armbruster
>> ---
>> 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
>>
>
Re: [PATCH v6 01/11] sysemu/tcg: Introduce tcg_builtin() helper
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()? 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é > --- > Cc: Markus Armbruster > --- > 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 >
[PATCH v6 01/11] sysemu/tcg: Introduce tcg_builtin() helper
Modules are registered early with type_register_static(). We would like to call tcg_enabled() when registering QOM types, 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é --- Cc: Markus Armbruster --- 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 -- 2.26.2
