Re: [PATCH v6 19/30] qemu/target-info: Add target_base_arch()

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 21/10/25 01:15, Pierrick Bouvier wrote:
> On 2025-10-20 15:14, Philippe Mathieu-Daudé wrote:
>> When multiple QEMU targets are variants (word size, endianness)
>> of the same base architecture, target_base_arch() returns this
>> base. For example, for the Aarch64 target it will return
>> SYS_EMU_TARGET_ARM as common base.
>>
> 
> I'm not sure that reusing semantic on a subset of this enum is the best 
> idea, so many things can go wrong.
> 
> More widely, I don't know where we would need to access this, versus 
> specific functions like target_base_arm().
> If a code needs to check various base archs target_base_*, or it can use 
> a switch with all variants.

Yeah, good point. I'll just drop this single patch.

> 
> If we really want to have this target_base concept, at least it deserves 
> it's own enum, separate from SYS_EMU_TARGET.
> 
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/qemu/target-info-impl.h |  2 ++
>>   include/qemu/target-info-qapi.h |  7 +++++++
>>   target-info-stub.c              |  1 +
>>   target-info.c                   | 10 ++++++++++
>>   4 files changed, 20 insertions(+)
>>
>> diff --git a/include/qemu/target-info-impl.h b/include/qemu/target- 
>> info-impl.h
>> index e446585bf53..2c171f8359b 100644
>> --- a/include/qemu/target-info-impl.h
>> +++ b/include/qemu/target-info-impl.h
>> @@ -17,6 +17,8 @@ typedef struct TargetInfo {
>>       const char *target_name;
>>       /* related to TARGET_ARCH definition */
>>       SysEmuTarget target_arch;
>> +    /* related to TARGET_BASE_ARCH definition (target/${base_arch}/ 
>> path) */
>> +    SysEmuTarget target_base_arch;
>>       /* runtime equivalent of TARGET_LONG_BITS definition */
>>       unsigned long_bits;
>>       /* runtime equivalent of CPU_RESOLVING_TYPE definition */
>> diff --git a/include/qemu/target-info-qapi.h b/include/qemu/target- 
>> info-qapi.h
>> index d5ce0523238..65ed4ca8eea 100644
>> --- a/include/qemu/target-info-qapi.h
>> +++ b/include/qemu/target-info-qapi.h
>> @@ -19,6 +19,13 @@
>>    */
>>   SysEmuTarget target_arch(void);
>> +/**
>> + * target_base_arch:
>> + *
>> + * Returns: QAPI SysEmuTarget enum (i.e. SYS_EMU_TARGET_I386).
>> + */
>> +SysEmuTarget target_base_arch(void);
>> +
>>   /**
>>    * target_endian_mode:
>>    *
>> diff --git a/target-info-stub.c b/target-info-stub.c
>> index d96d8249c1d..d2cfca1b4c2 100644
>> --- a/target-info-stub.c
>> +++ b/target-info-stub.c
>> @@ -19,6 +19,7 @@ QEMU_BUILD_BUG_ON(offsetof(ArchCPU, env) != 
>> sizeof(CPUState));
>>   static const TargetInfo target_info_stub = {
>>       .target_name = TARGET_NAME,
>>       .target_arch = SYS_EMU_TARGET__MAX,
>> +    .target_base_arch = SYS_EMU_TARGET__MAX,
> 
> And nothing can enforce base and arch match by design, which is a 
> problem IMHO.
> 
>>       .long_bits = TARGET_LONG_BITS,
>>       .cpu_type = CPU_RESOLVING_TYPE,
>>       .machine_typename = TYPE_MACHINE,
>> diff --git a/target-info.c b/target-info.c
>> index e567cb4c40a..332198e40a2 100644
>> --- a/target-info.c
>> +++ b/target-info.c
>> @@ -33,6 +33,16 @@ SysEmuTarget target_arch(void)
>>       return arch;
>>   }
>> +SysEmuTarget target_base_arch(void)
>> +{
>> +    SysEmuTarget base_arch = target_info()->target_base_arch;
>> +
>> +    if (base_arch == SYS_EMU_TARGET__MAX) {
>> +        base_arch = target_arch();
>> +    }
>> +    return base_arch;
> 
> More confusing, we can eventually return a non base arch if base arch 
> was not correctly set above.
> 
>> +}
>> +
>>   const char *target_cpu_type(void)
>>   {
>>       return target_info()->cpu_type;