target-info-stub.c | 11 ++++++++++- target-info.c | 9 +-------- 2 files changed, 11 insertions(+), 9 deletions(-)
target_arch() function will reparse target_name() every time if it was
not set to a proper SYS_EMU_TARGET_* value (when using
target-info-stub.c), which is not efficient.
Since we want to preserve the constness of TargetInfo but C doesn't give
us flexible compile time expressions, we simply set target_arch using a
static constructor once instead.
This was found when doing changes to virtio_access_is_big_endian()
function, having an overhead of 50% after switching to runtime checks.
With this, overhead left is around 3%, due to indirect function
calls.
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
target-info-stub.c | 11 ++++++++++-
target-info.c | 9 +--------
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/target-info-stub.c b/target-info-stub.c
index 8392d81e8f8..ff86a02247a 100644
--- a/target-info-stub.c
+++ b/target-info-stub.c
@@ -10,13 +10,14 @@
#include "qemu/target-info.h"
#include "qemu/target-info-impl.h"
#include "hw/core/boards.h"
+#include "qapi/error.h"
#include "cpu.h"
/* Validate correct placement of CPUArchState. */
QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
-static const TargetInfo target_info_stub = {
+static TargetInfo target_info_stub = {
.target_name = TARGET_NAME,
.target_arch = SYS_EMU_TARGET__MAX,
.long_bits = TARGET_LONG_BITS,
@@ -29,3 +30,11 @@ const TargetInfo *target_info(void)
{
return &target_info_stub;
}
+
+__attribute__((constructor))
+static void init_target_arch(void)
+{
+ target_info_stub.target_arch = qapi_enum_parse(&SysEmuTarget_lookup,
+ target_name(), -1,
+ &error_abort);
+}
diff --git a/target-info.c b/target-info.c
index 24696ff4111..c3c0856d01a 100644
--- a/target-info.c
+++ b/target-info.c
@@ -10,7 +10,6 @@
#include "qemu/target-info.h"
#include "qemu/target-info-qapi.h"
#include "qemu/target-info-impl.h"
-#include "qapi/error.h"
const char *target_name(void)
{
@@ -24,13 +23,7 @@ unsigned target_long_bits(void)
SysEmuTarget target_arch(void)
{
- SysEmuTarget arch = target_info()->target_arch;
-
- if (arch == SYS_EMU_TARGET__MAX) {
- arch = qapi_enum_parse(&SysEmuTarget_lookup, target_name(), -1,
- &error_abort);
- }
- return arch;
+ return target_info()->target_arch;
}
const char *target_cpu_type(void)
--
2.47.3Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> target_arch() function will reparse target_name() every time if it was
> not set to a proper SYS_EMU_TARGET_* value (when using
> target-info-stub.c), which is not efficient.
>
> Since we want to preserve the constness of TargetInfo but C doesn't give
> us flexible compile time expressions, we simply set target_arch using a
> static constructor once instead.
>
> This was found when doing changes to virtio_access_is_big_endian()
> function, having an overhead of 50% after switching to runtime checks.
> With this, overhead left is around 3%, due to indirect function
> calls.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
> target-info-stub.c | 11 ++++++++++-
> target-info.c | 9 +--------
> 2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/target-info-stub.c b/target-info-stub.c
> index 8392d81e8f8..ff86a02247a 100644
> --- a/target-info-stub.c
> +++ b/target-info-stub.c
> @@ -10,13 +10,14 @@
> #include "qemu/target-info.h"
> #include "qemu/target-info-impl.h"
> #include "hw/core/boards.h"
> +#include "qapi/error.h"
> #include "cpu.h"
>
> /* Validate correct placement of CPUArchState. */
> QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
> QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
>
> -static const TargetInfo target_info_stub = {
> +static TargetInfo target_info_stub = {
> .target_name = TARGET_NAME,
> .target_arch = SYS_EMU_TARGET__MAX,
> .long_bits = TARGET_LONG_BITS,
> @@ -29,3 +30,11 @@ const TargetInfo *target_info(void)
> {
> return &target_info_stub;
> }
> +
> +__attribute__((constructor))
> +static void init_target_arch(void)
> +{
> + target_info_stub.target_arch = qapi_enum_parse(&SysEmuTarget_lookup,
> + target_name(), -1,
> + &error_abort);
> +}
This is slightly unclean. Constructors run before main(). If
qapi_enum_parse(..., &error_abort) fails here, error_handle() will
report the unexpected error with error_report() before main() calls
error_init().
See also
From: Markus Armbruster <armbru@redhat.com>
Subject: Re: [PATCH v3 02/20] monitor: initialize global data from a constructor
Date: Thu, 18 Sep 2025 08:30:23 +0200
Message-ID: <87frck1dds.fsf@pond.sub.org>
Observation, not objection.
> diff --git a/target-info.c b/target-info.c
> index 24696ff4111..c3c0856d01a 100644
> --- a/target-info.c
> +++ b/target-info.c
> @@ -10,7 +10,6 @@
> #include "qemu/target-info.h"
> #include "qemu/target-info-qapi.h"
> #include "qemu/target-info-impl.h"
> -#include "qapi/error.h"
>
> const char *target_name(void)
> {
> @@ -24,13 +23,7 @@ unsigned target_long_bits(void)
>
> SysEmuTarget target_arch(void)
> {
> - SysEmuTarget arch = target_info()->target_arch;
> -
> - if (arch == SYS_EMU_TARGET__MAX) {
> - arch = qapi_enum_parse(&SysEmuTarget_lookup, target_name(), -1,
> - &error_abort);
> - }
> - return arch;
> + return target_info()->target_arch;
> }
>
> const char *target_cpu_type(void)
On 2/5/26 1:29 AM, Markus Armbruster wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>
>> target_arch() function will reparse target_name() every time if it was
>> not set to a proper SYS_EMU_TARGET_* value (when using
>> target-info-stub.c), which is not efficient.
>>
>> Since we want to preserve the constness of TargetInfo but C doesn't give
>> us flexible compile time expressions, we simply set target_arch using a
>> static constructor once instead.
>>
>> This was found when doing changes to virtio_access_is_big_endian()
>> function, having an overhead of 50% after switching to runtime checks.
>> With this, overhead left is around 3%, due to indirect function
>> calls.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>> target-info-stub.c | 11 ++++++++++-
>> target-info.c | 9 +--------
>> 2 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/target-info-stub.c b/target-info-stub.c
>> index 8392d81e8f8..ff86a02247a 100644
>> --- a/target-info-stub.c
>> +++ b/target-info-stub.c
>> @@ -10,13 +10,14 @@
>> #include "qemu/target-info.h"
>> #include "qemu/target-info-impl.h"
>> #include "hw/core/boards.h"
>> +#include "qapi/error.h"
>> #include "cpu.h"
>>
>> /* Validate correct placement of CPUArchState. */
>> QEMU_BUILD_BUG_ON(offsetof(ArchCPU, parent_obj) != 0);
>> QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != sizeof(CPUState));
>>
>> -static const TargetInfo target_info_stub = {
>> +static TargetInfo target_info_stub = {
>> .target_name = TARGET_NAME,
>> .target_arch = SYS_EMU_TARGET__MAX,
>> .long_bits = TARGET_LONG_BITS,
>> @@ -29,3 +30,11 @@ const TargetInfo *target_info(void)
>> {
>> return &target_info_stub;
>> }
>> +
>> +__attribute__((constructor))
>> +static void init_target_arch(void)
>> +{
>> + target_info_stub.target_arch = qapi_enum_parse(&SysEmuTarget_lookup,
>> + target_name(), -1,
>> + &error_abort);
>> +}
>
> This is slightly unclean. Constructors run before main(). If
> qapi_enum_parse(..., &error_abort) fails here, error_handle() will
> report the unexpected error with error_report() before main() calls
> error_init().
>
> See also
>
> From: Markus Armbruster <armbru@redhat.com>
> Subject: Re: [PATCH v3 02/20] monitor: initialize global data from a constructor
> Date: Thu, 18 Sep 2025 08:30:23 +0200
> Message-ID: <87frck1dds.fsf@pond.sub.org>
>
> Observation, not objection.
>
Good point.
Richard posted a v2 tooking the build system approach here:
https://lore.kernel.org/qemu-devel/20260205030617.266625-1-richard.henderson@linaro.org/
So it doesn't suffer from this issue.
As well, it's cleaner and more efficient overall, so that's the best way
to do it.
Regards,
Pierrick
On 03/02/26, Pierrick Bouvier wrote: > target_arch() function will reparse target_name() every time if it was > not set to a proper SYS_EMU_TARGET_* value (when using > target-info-stub.c), which is not efficient. > > Since we want to preserve the constness of TargetInfo but C doesn't give > us flexible compile time expressions, we simply set target_arch using a > static constructor once instead. > > This was found when doing changes to virtio_access_is_big_endian() > function, having an overhead of 50% after switching to runtime checks. > With this, overhead left is around 3%, due to indirect function > calls. > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > target-info-stub.c | 11 ++++++++++- > target-info.c | 9 +-------- > 2 files changed, 11 insertions(+), 9 deletions(-) Good find! Reviewed-by: Anton Johansson <anjo@rev.ng>
On Tue, Feb 03, 2026 at 11:41:13AM -0800, Pierrick Bouvier wrote: > target_arch() function will reparse target_name() every time if it was > not set to a proper SYS_EMU_TARGET_* value (when using > target-info-stub.c), which is not efficient. > > Since we want to preserve the constness of TargetInfo but C doesn't give > us flexible compile time expressions, we simply set target_arch using a > static constructor once instead. > > This was found when doing changes to virtio_access_is_big_endian() > function, having an overhead of 50% after switching to runtime checks. > With this, overhead left is around 3%, due to indirect function > calls. Hmm, this presumably needs to be backported to 10.1.0 / 10.2.0 stable branches, given that performance hit. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 2/4/26 1:10 AM, Daniel P. Berrangé wrote: > On Tue, Feb 03, 2026 at 11:41:13AM -0800, Pierrick Bouvier wrote: >> target_arch() function will reparse target_name() every time if it was >> not set to a proper SYS_EMU_TARGET_* value (when using >> target-info-stub.c), which is not efficient. >> >> Since we want to preserve the constness of TargetInfo but C doesn't give >> us flexible compile time expressions, we simply set target_arch using a >> static constructor once instead. >> >> This was found when doing changes to virtio_access_is_big_endian() >> function, having an overhead of 50% after switching to runtime checks. >> With this, overhead left is around 3%, due to indirect function >> calls. > > Hmm, this presumably needs to be backported to 10.1.0 / 10.2.0 > stable branches, given that performance hit. > Indeed, I'll let Richard fix it. > > With regards, > Daniel Regards, Pierrick
On 2/4/26 05:41, Pierrick Bouvier wrote: > target_arch() function will reparse target_name() every time if it was > not set to a proper SYS_EMU_TARGET_* value (when using > target-info-stub.c), which is not efficient. > > Since we want to preserve the constness of TargetInfo but C doesn't give > us flexible compile time expressions, we simply set target_arch using a > static constructor once instead. A static constructor isn't static initialization. That said, we can do better with some extra help from meson; see attached. I'm mildly annoyed with openrisc vs or1k. We really ought to fix that, but I haven't looked into what API breakage we get from selecting one or the other. r~
On 2/3/26 5:36 PM, Richard Henderson wrote: > On 2/4/26 05:41, Pierrick Bouvier wrote: >> target_arch() function will reparse target_name() every time if it was >> not set to a proper SYS_EMU_TARGET_* value (when using >> target-info-stub.c), which is not efficient. >> >> Since we want to preserve the constness of TargetInfo but C doesn't give >> us flexible compile time expressions, we simply set target_arch using a >> static constructor once instead. > > A static constructor isn't static initialization. > That said, we can do better with some extra help from meson; see attached. > > I'm mildly annoyed with openrisc vs or1k. We really ought to fix that, but I haven't > looked into what API breakage we get from selecting one or the other. > This was my first approach, and I noticed the or1k issue + missing hexagon in SYS_EMU_TARGET enum. Having a hack for target name in meson.build is *really* ugly IMHO. At the moment, hexagon is only linux-user, so I thought it didn't make sense to add it to SYS_EMU_TARGET, and I didn't want to go down the rabbit hole to rename or extend this qapi definition, as the initial goal is just to define a field before main, which I consider to be static initialization, even though you might prefer to call it differently. So with all that, having a function with constructor attribute seemed like being the best compromise between maintainability and efficiency. I'll let you upstream whatever changes you prefer, and I drop this patch. Regards, Pierrick
On 2/5/26 02:36, Pierrick Bouvier wrote: > On 2/3/26 5:36 PM, Richard Henderson wrote: >> On 2/4/26 05:41, Pierrick Bouvier wrote: >>> target_arch() function will reparse target_name() every time if it was >>> not set to a proper SYS_EMU_TARGET_* value (when using >>> target-info-stub.c), which is not efficient. >>> >>> Since we want to preserve the constness of TargetInfo but C doesn't give >>> us flexible compile time expressions, we simply set target_arch using a >>> static constructor once instead. >> >> A static constructor isn't static initialization. >> That said, we can do better with some extra help from meson; see attached. >> >> I'm mildly annoyed with openrisc vs or1k. We really ought to fix that, but I haven't >> looked into what API breakage we get from selecting one or the other. >> > > This was my first approach, and I noticed the or1k issue + missing hexagon in > SYS_EMU_TARGET enum. Having a hack for target name in meson.build is *really* ugly IMHO. I agree. I assume the qapi string is the one that should take precedence; everything else appears to be merely qemu source level strings. Marcus, can you confirm? > At the moment, hexagon is only linux-user, so I thought it didn't make sense to add it to > SYS_EMU_TARGET, and I didn't want to go down the rabbit hole to rename or extend this qapi > definition, as the initial goal is just to define a field before main, which I consider to > be static initialization, even though you might prefer to call it differently. I was only going to ask you to change "statically" to "at startup" there. On the other hand, system-mode patches for hexagon have been on the list for a while, so I don't think it's jumping the gun too much to include it in SYS_EMU_TARGET at this time. > I'll let you upstream whatever changes you prefer, and I drop this patch. Ok, will do. r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 2/5/26 02:36, Pierrick Bouvier wrote: >> On 2/3/26 5:36 PM, Richard Henderson wrote: >>> On 2/4/26 05:41, Pierrick Bouvier wrote: >>>> target_arch() function will reparse target_name() every time if it was >>>> not set to a proper SYS_EMU_TARGET_* value (when using >>>> target-info-stub.c), which is not efficient. >>>> >>>> Since we want to preserve the constness of TargetInfo but C doesn't give >>>> us flexible compile time expressions, we simply set target_arch using a >>>> static constructor once instead. >>> >>> A static constructor isn't static initialization. >>> That said, we can do better with some extra help from meson; see attached. >>> >>> I'm mildly annoyed with openrisc vs or1k. We really ought to fix that, but I haven't >>> looked into what API breakage we get from selecting one or the other. >>> >> This was my first approach, and I noticed the or1k issue + missing hexagon in SYS_EMU_TARGET enum. Having a hack for target name in meson.build is *really* ugly IMHO. > > I agree. > > I assume the qapi string is the one that should take precedence; everything else appears to be merely qemu source level strings. Marcus, can you confirm? What exactly would you like me to confirm? I *guess* it's about the messy part of the patch you posted upthread. There, you have to normalize TARGET_ARCH value 'openrisc' to 'or1k'. >> At the moment, hexagon is only linux-user, so I thought it didn't make sense to add it to SYS_EMU_TARGET, and I didn't want to go down the rabbit hole to rename or extend this qapi definition, as the initial goal is just to define a field before main, which I consider to be static initialization, even though you might prefer to call it differently. I figure this is about adding 'hexagon' to SysEmuTarget even though it's not actually a system emulator target now. The fact that adding it there helps indicates SysEmuTarget has leaked into user emulators, and its name has become misleading. Is this true? > I was only going to ask you to change "statically" to "at startup" there. > > On the other hand, system-mode patches for hexagon have been on the list for a while, so I don't think it's jumping the gun too much to include it in SYS_EMU_TARGET at this time. > >> I'll let you upstream whatever changes you prefer, and I drop this patch. > > Ok, will do. > > > r~
On 2/5/26 19:52, Markus Armbruster wrote: >> I assume the qapi string is the one that should take precedence; everything else appears to be merely qemu source level strings. Marcus, can you confirm? > > What exactly would you like me to confirm? > > I *guess* it's about the messy part of the patch you posted upthread. > There, you have to normalize TARGET_ARCH value 'openrisc' to 'or1k'. Yes. I wanted you to confirm that changing the string in qapi is a non-starter, therefore 'or1k' is the string we should standardize on. > I figure this is about adding 'hexagon' to SysEmuTarget even though it's > not actually a system emulator target now. > > The fact that adding it there helps indicates SysEmuTarget has leaked > into user emulators, and its name has become misleading. Is this true? Yes. The target_info structure is used for both user and system, and one of these fields is the SysEmuTarget entry. r~
Richard Henderson <richard.henderson@linaro.org> writes:
> On 2/5/26 19:52, Markus Armbruster wrote:
>>> I assume the qapi string is the one that should take precedence; everything else appears to be merely qemu source level strings. Marcus, can you confirm?
>>
>> What exactly would you like me to confirm?
>>
>> I *guess* it's about the messy part of the patch you posted upthread.
>> There, you have to normalize TARGET_ARCH value 'openrisc' to 'or1k'.
>
> Yes. I wanted you to confirm that changing the string in qapi is a non-starter, therefore
> 'or1k' is the string we should standardize on.
Yes, changing enum value @or1k is problematic.
Enum SysEmuTarget is visible in QMP: query-target return member @arch,
query-cpus-fast return member @target. These are stable interfaces.
I can see two ways to change the enum value to @openrisc:
1. We create a new qemu-system-openrisc, identical to qemu-system-or1k
except for the target name. We deprecate qemu-system-or1k and
SysEmuTarget member @or1k, and remove them after the grace period.
I doubt it's worth the bother.
2. We cheat: we rename with the excuse that the target is bottom support
tier, and our compatibility promise doesn't apply there.
I don't think that's a good idea. If we want to qualify our
compatibility promise with tiers, we better define the tiers and what
they mean for the promise *first*. Related:
From: Daniel P. Berrangé <berrange@redhat.com>
Subject: [PATCH] docs/about: propose OS platform/arch support tiers
Date: Thu, 15 Jan 2026 18:01:23 +0000
Message-ID: <20260115180123.848640-1-berrange@redhat.com>
https://lore.kernel.org/qemu-devel/20260115180123.848640-1-berrange@redhat.com/
>> I figure this is about adding 'hexagon' to SysEmuTarget even though it's
>> not actually a system emulator target now.
>>
>> The fact that adding it there helps indicates SysEmuTarget has leaked
>> into user emulators, and its name has become misleading. Is this true?
>
> Yes. The target_info structure is used for both user and system, and one of these fields
> is the SysEmuTarget entry.
Then SysEmuTarget has become misleading.
We *can* fix that: QAPI type names are not part of the external
interface by design.
On 2/5/26 10:58 PM, Markus Armbruster wrote: > Richard Henderson <richard.henderson@linaro.org> writes: > >> On 2/5/26 19:52, Markus Armbruster wrote: >>>> I assume the qapi string is the one that should take precedence; everything else appears to be merely qemu source level strings. Marcus, can you confirm? >>> >>> What exactly would you like me to confirm? >>> >>> I *guess* it's about the messy part of the patch you posted upthread. >>> There, you have to normalize TARGET_ARCH value 'openrisc' to 'or1k'. >> >> Yes. I wanted you to confirm that changing the string in qapi is a non-starter, therefore >> 'or1k' is the string we should standardize on. > > Yes, changing enum value @or1k is problematic. > > Enum SysEmuTarget is visible in QMP: query-target return member @arch, > query-cpus-fast return member @target. These are stable interfaces. > > I can see two ways to change the enum value to @openrisc: > > 1. We create a new qemu-system-openrisc, identical to qemu-system-or1k > except for the target name. We deprecate qemu-system-or1k and > SysEmuTarget member @or1k, and remove them after the grace period. > > I doubt it's worth the bother. > > 2. We cheat: we rename with the excuse that the target is bottom support > tier, and our compatibility promise doesn't apply there. > > I don't think that's a good idea. If we want to qualify our > compatibility promise with tiers, we better define the tiers and what > they mean for the promise *first*. Related: > > From: Daniel P. Berrangé <berrange@redhat.com> > Subject: [PATCH] docs/about: propose OS platform/arch support tiers > Date: Thu, 15 Jan 2026 18:01:23 +0000 > Message-ID: <20260115180123.848640-1-berrange@redhat.com> > https://lore.kernel.org/qemu-devel/20260115180123.848640-1-berrange@redhat.com/ > >>> I figure this is about adding 'hexagon' to SysEmuTarget even though it's >>> not actually a system emulator target now. >>> >>> The fact that adding it there helps indicates SysEmuTarget has leaked >>> into user emulators, and its name has become misleading. Is this true? >> >> Yes. The target_info structure is used for both user and system, and one of these fields >> is the SysEmuTarget entry. > > Then SysEmuTarget has become misleading. > > We *can* fix that: QAPI type names are not part of the external > interface by design. > Do you mean it could be renamed to EmuTarget without any consequence? It seemed to me that everything exported from the json files could potentially be consumed by another entity (libvirt for instance). Regards, Pierrick
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> On 2/5/26 10:58 PM, Markus Armbruster wrote:
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> On 2/5/26 19:52, Markus Armbruster wrote:
[...]
>> Then SysEmuTarget has become misleading.
>>
>> We *can* fix that: QAPI type names are not part of the external
>> interface by design.
>
> Do you mean it could be renamed to EmuTarget without any consequence?
Yes!
> It seemed to me that everything exported from the json files could
> potentially be consumed by another entity (libvirt for instance).
No. The QAPI schema ("the json files") is not meant for external
consumption, and certainly not a stable interface. The stable external
interface is QMP introspection with query-qmp-schema. I designed that
command not to expose type names. One of my smarter moves ;)
On 2/5/26 11:38 PM, Markus Armbruster wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>
>> On 2/5/26 10:58 PM, Markus Armbruster wrote:
>>> Richard Henderson <richard.henderson@linaro.org> writes:
>>>
>>>> On 2/5/26 19:52, Markus Armbruster wrote:
>
> [...]
>
>>> Then SysEmuTarget has become misleading.
>>>
>>> We *can* fix that: QAPI type names are not part of the external
>>> interface by design.
>>
>> Do you mean it could be renamed to EmuTarget without any consequence?
>
> Yes!
>
That's pretty good news. So yeah, it makes sense to extend it to
represent all targets, including user ones.
>> It seemed to me that everything exported from the json files could
>> potentially be consumed by another entity (libvirt for instance).
>
> No. The QAPI schema ("the json files") is not meant for external
> consumption, and certainly not a stable interface. The stable external
> interface is QMP introspection with query-qmp-schema. I designed that
> command not to expose type names. One of my smarter moves ;)
>
😎
We can thank you, and you can thank yourself :)
Thanks,
Pierrick
© 2016 - 2026 Red Hat, Inc.