[PATCH v3] system: Convert qemu_arch_available() to TargetInfo API

Philippe Mathieu-Daudé posted 1 patch 4 weeks, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260108163601.18676-1-philmd@linaro.org
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Pierrick Bouvier <pierrick.bouvier@linaro.org>
meson.build        |  2 --
system/arch_init.c | 30 -----------------------
target-info.c      | 60 ++++++++++++++++++++++++++++++++++++++++++++++
system/meson.build |  1 -
4 files changed, 60 insertions(+), 33 deletions(-)
delete mode 100644 system/arch_init.c
[PATCH v3] system: Convert qemu_arch_available() to TargetInfo API
Posted by Philippe Mathieu-Daudé 4 weeks, 1 day ago
Get the base arch_mask from the current SysEmuTarget,
making qemu_arch_available() target-agnostic.

We don't need the per-target QEMU_ARCH definition anymore,
remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
v3: Return directly within switch
v2: Prefer switch over array (pbo)
---
 meson.build        |  2 --
 system/arch_init.c | 30 -----------------------
 target-info.c      | 60 ++++++++++++++++++++++++++++++++++++++++++++++
 system/meson.build |  1 -
 4 files changed, 60 insertions(+), 33 deletions(-)
 delete mode 100644 system/arch_init.c

diff --git a/meson.build b/meson.build
index 734c801cc77..435dc6e3c8e 100644
--- a/meson.build
+++ b/meson.build
@@ -3419,8 +3419,6 @@ foreach target : target_dirs
       config_target_data.set(k, v)
     endif
   endforeach
-  config_target_data.set('QEMU_ARCH',
-                         'QEMU_ARCH_' + config_target['TARGET_BASE_ARCH'].to_upper())
   config_target_h += {target: configure_file(output: target + '-config-target.h',
                                                configuration: config_target_data)}
 
diff --git a/system/arch_init.c b/system/arch_init.c
deleted file mode 100644
index e85736884c9..00000000000
--- a/system/arch_init.c
+++ /dev/null
@@ -1,30 +0,0 @@
-/*
- * QEMU System Emulator
- *
- * Copyright (c) 2003-2008 Fabrice Bellard
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-#include "qemu/osdep.h"
-#include "system/arch_init.h"
-
-bool qemu_arch_available(unsigned qemu_arch_mask)
-{
-    return qemu_arch_mask & QEMU_ARCH;
-}
diff --git a/target-info.c b/target-info.c
index 24696ff4111..774fdcd2c46 100644
--- a/target-info.c
+++ b/target-info.c
@@ -11,6 +11,7 @@
 #include "qemu/target-info-qapi.h"
 #include "qemu/target-info-impl.h"
 #include "qapi/error.h"
+#include "system/arch_init.h"
 
 const char *target_name(void)
 {
@@ -33,6 +34,65 @@ SysEmuTarget target_arch(void)
     return arch;
 }
 
+bool qemu_arch_available(unsigned qemu_arch_mask)
+{
+    switch (target_arch()) {
+    case SYS_EMU_TARGET_ALPHA:
+        return qemu_arch_mask & QEMU_ARCH_ALPHA;
+    case SYS_EMU_TARGET_ARM:
+    case SYS_EMU_TARGET_AARCH64:
+        return qemu_arch_mask & QEMU_ARCH_ARM;
+    case SYS_EMU_TARGET_I386:
+    case SYS_EMU_TARGET_X86_64:
+        return qemu_arch_mask & QEMU_ARCH_I386;
+    case SYS_EMU_TARGET_M68K:
+        return qemu_arch_mask & QEMU_ARCH_M68K;
+    case SYS_EMU_TARGET_MICROBLAZE:
+    case SYS_EMU_TARGET_MICROBLAZEEL:
+        return qemu_arch_mask & QEMU_ARCH_MICROBLAZE;
+    case SYS_EMU_TARGET_MIPS:
+    case SYS_EMU_TARGET_MIPSEL:
+    case SYS_EMU_TARGET_MIPS64:
+    case SYS_EMU_TARGET_MIPS64EL:
+        return qemu_arch_mask & QEMU_ARCH_MIPS;
+    case SYS_EMU_TARGET_PPC:
+    case SYS_EMU_TARGET_PPC64:
+        return qemu_arch_mask & QEMU_ARCH_PPC;
+    case SYS_EMU_TARGET_S390X:
+        return qemu_arch_mask & QEMU_ARCH_S390X;
+    case SYS_EMU_TARGET_SH4:
+    case SYS_EMU_TARGET_SH4EB:
+        return qemu_arch_mask & QEMU_ARCH_SH4;
+    case SYS_EMU_TARGET_SPARC:
+    case SYS_EMU_TARGET_SPARC64:
+        return qemu_arch_mask & QEMU_ARCH_SPARC;
+    case SYS_EMU_TARGET_XTENSA:
+    case SYS_EMU_TARGET_XTENSAEB:
+        return qemu_arch_mask & QEMU_ARCH_XTENSA;
+    case SYS_EMU_TARGET_OR1K:
+        return qemu_arch_mask & QEMU_ARCH_OPENRISC;
+    case SYS_EMU_TARGET_TRICORE:
+        return qemu_arch_mask & QEMU_ARCH_TRICORE;
+    case SYS_EMU_TARGET_HPPA:
+        return qemu_arch_mask & QEMU_ARCH_HPPA;
+    case SYS_EMU_TARGET_RISCV32:
+    case SYS_EMU_TARGET_RISCV64:
+        return qemu_arch_mask & QEMU_ARCH_RISCV;
+    case SYS_EMU_TARGET_RX:
+        return qemu_arch_mask & QEMU_ARCH_RX;
+    case SYS_EMU_TARGET_AVR:
+        return qemu_arch_mask & QEMU_ARCH_AVR;
+    /*
+    case SYS_EMU_TARGET_HEXAGON:
+        return qemu_arch_mask & QEMU_ARCH_HEXAGON;
+    */
+    case SYS_EMU_TARGET_LOONGARCH64:
+        return qemu_arch_mask & QEMU_ARCH_LOONGARCH;
+    default:
+        g_assert_not_reached();
+    };
+}
+
 const char *target_cpu_type(void)
 {
     return target_info()->cpu_type;
diff --git a/system/meson.build b/system/meson.build
index 4b69ef0f5fb..66e16db55ce 100644
--- a/system/meson.build
+++ b/system/meson.build
@@ -1,5 +1,4 @@
 specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
-  'arch_init.c',
   'globals-target.c',
 )])
 
-- 
2.52.0


Re: [PATCH v3] system: Convert qemu_arch_available() to TargetInfo API
Posted by Pierrick Bouvier 4 weeks, 1 day ago
On 1/8/26 8:36 AM, Philippe Mathieu-Daudé wrote:
> Get the base arch_mask from the current SysEmuTarget,
> making qemu_arch_available() target-agnostic.
> 
> We don't need the per-target QEMU_ARCH definition anymore,
> remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v3: Return directly within switch
> v2: Prefer switch over array (pbo)
> ---
>   meson.build        |  2 --
>   system/arch_init.c | 30 -----------------------
>   target-info.c      | 60 ++++++++++++++++++++++++++++++++++++++++++++++
>   system/meson.build |  1 -
>   4 files changed, 60 insertions(+), 33 deletions(-)
>   delete mode 100644 system/arch_init.c
> 
> diff --git a/meson.build b/meson.build
> index 734c801cc77..435dc6e3c8e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3419,8 +3419,6 @@ foreach target : target_dirs
>         config_target_data.set(k, v)
>       endif
>     endforeach
> -  config_target_data.set('QEMU_ARCH',
> -                         'QEMU_ARCH_' + config_target['TARGET_BASE_ARCH'].to_upper())
>     config_target_h += {target: configure_file(output: target + '-config-target.h',
>                                                  configuration: config_target_data)}
>   
> diff --git a/system/arch_init.c b/system/arch_init.c
> deleted file mode 100644
> index e85736884c9..00000000000
> --- a/system/arch_init.c
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -/*
> - * QEMU System Emulator
> - *
> - * Copyright (c) 2003-2008 Fabrice Bellard
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a copy
> - * of this software and associated documentation files (the "Software"), to deal
> - * in the Software without restriction, including without limitation the rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - */
> -#include "qemu/osdep.h"
> -#include "system/arch_init.h"
> -
> -bool qemu_arch_available(unsigned qemu_arch_mask)
> -{
> -    return qemu_arch_mask & QEMU_ARCH;
> -}
> diff --git a/target-info.c b/target-info.c
> index 24696ff4111..774fdcd2c46 100644
> --- a/target-info.c
> +++ b/target-info.c
> @@ -11,6 +11,7 @@
>   #include "qemu/target-info-qapi.h"
>   #include "qemu/target-info-impl.h"
>   #include "qapi/error.h"
> +#include "system/arch_init.h"
>   
>   const char *target_name(void)
>   {
> @@ -33,6 +34,65 @@ SysEmuTarget target_arch(void)
>       return arch;
>   }
>   
> +bool qemu_arch_available(unsigned qemu_arch_mask)
> +{
> +    switch (target_arch()) {
> +    case SYS_EMU_TARGET_ALPHA:
> +        return qemu_arch_mask & QEMU_ARCH_ALPHA;
> +    case SYS_EMU_TARGET_ARM:
> +    case SYS_EMU_TARGET_AARCH64:
> +        return qemu_arch_mask & QEMU_ARCH_ARM;
> +    case SYS_EMU_TARGET_I386:
> +    case SYS_EMU_TARGET_X86_64:
> +        return qemu_arch_mask & QEMU_ARCH_I386;
> +    case SYS_EMU_TARGET_M68K:
> +        return qemu_arch_mask & QEMU_ARCH_M68K;
> +    case SYS_EMU_TARGET_MICROBLAZE:
> +    case SYS_EMU_TARGET_MICROBLAZEEL:
> +        return qemu_arch_mask & QEMU_ARCH_MICROBLAZE;
> +    case SYS_EMU_TARGET_MIPS:
> +    case SYS_EMU_TARGET_MIPSEL:
> +    case SYS_EMU_TARGET_MIPS64:
> +    case SYS_EMU_TARGET_MIPS64EL:
> +        return qemu_arch_mask & QEMU_ARCH_MIPS;
> +    case SYS_EMU_TARGET_PPC:
> +    case SYS_EMU_TARGET_PPC64:
> +        return qemu_arch_mask & QEMU_ARCH_PPC;
> +    case SYS_EMU_TARGET_S390X:
> +        return qemu_arch_mask & QEMU_ARCH_S390X;
> +    case SYS_EMU_TARGET_SH4:
> +    case SYS_EMU_TARGET_SH4EB:
> +        return qemu_arch_mask & QEMU_ARCH_SH4;
> +    case SYS_EMU_TARGET_SPARC:
> +    case SYS_EMU_TARGET_SPARC64:
> +        return qemu_arch_mask & QEMU_ARCH_SPARC;
> +    case SYS_EMU_TARGET_XTENSA:
> +    case SYS_EMU_TARGET_XTENSAEB:
> +        return qemu_arch_mask & QEMU_ARCH_XTENSA;
> +    case SYS_EMU_TARGET_OR1K:
> +        return qemu_arch_mask & QEMU_ARCH_OPENRISC;
> +    case SYS_EMU_TARGET_TRICORE:
> +        return qemu_arch_mask & QEMU_ARCH_TRICORE;
> +    case SYS_EMU_TARGET_HPPA:
> +        return qemu_arch_mask & QEMU_ARCH_HPPA;
> +    case SYS_EMU_TARGET_RISCV32:
> +    case SYS_EMU_TARGET_RISCV64:
> +        return qemu_arch_mask & QEMU_ARCH_RISCV;
> +    case SYS_EMU_TARGET_RX:
> +        return qemu_arch_mask & QEMU_ARCH_RX;
> +    case SYS_EMU_TARGET_AVR:
> +        return qemu_arch_mask & QEMU_ARCH_AVR;
> +    /*
> +    case SYS_EMU_TARGET_HEXAGON:
> +        return qemu_arch_mask & QEMU_ARCH_HEXAGON;
> +    */
> +    case SYS_EMU_TARGET_LOONGARCH64:
> +        return qemu_arch_mask & QEMU_ARCH_LOONGARCH;
> +    default:
> +        g_assert_not_reached();

Per explaination on previous version, would be more interesting to have 
SYS_EMU_TARGET__MAX instead of default here, so compilation will be 
blocked when enum is extended.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

> +    };
> +}
> +
>   const char *target_cpu_type(void)
>   {
>       return target_info()->cpu_type;
> diff --git a/system/meson.build b/system/meson.build
> index 4b69ef0f5fb..66e16db55ce 100644
> --- a/system/meson.build
> +++ b/system/meson.build
> @@ -1,5 +1,4 @@
>   specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
> -  'arch_init.c',
>     'globals-target.c',
>   )])
>   


Re: [PATCH v3] system: Convert qemu_arch_available() to TargetInfo API
Posted by Philippe Mathieu-Daudé 4 weeks, 1 day ago
On 8/1/26 19:57, Pierrick Bouvier wrote:
> On 1/8/26 8:36 AM, Philippe Mathieu-Daudé wrote:
>> Get the base arch_mask from the current SysEmuTarget,
>> making qemu_arch_available() target-agnostic.
>>
>> We don't need the per-target QEMU_ARCH definition anymore,
>> remove it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> v3: Return directly within switch
>> v2: Prefer switch over array (pbo)
>> ---
>>   meson.build        |  2 --
>>   system/arch_init.c | 30 -----------------------
>>   target-info.c      | 60 ++++++++++++++++++++++++++++++++++++++++++++++
>>   system/meson.build |  1 -
>>   4 files changed, 60 insertions(+), 33 deletions(-)
>>   delete mode 100644 system/arch_init.c
>>
>> diff --git a/meson.build b/meson.build
>> index 734c801cc77..435dc6e3c8e 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -3419,8 +3419,6 @@ foreach target : target_dirs
>>         config_target_data.set(k, v)
>>       endif
>>     endforeach
>> -  config_target_data.set('QEMU_ARCH',
>> -                         'QEMU_ARCH_' + 
>> config_target['TARGET_BASE_ARCH'].to_upper())
>>     config_target_h += {target: configure_file(output: target + '- 
>> config-target.h',
>>                                                  configuration: 
>> config_target_data)}
>> diff --git a/system/arch_init.c b/system/arch_init.c
>> deleted file mode 100644
>> index e85736884c9..00000000000
>> --- a/system/arch_init.c
>> +++ /dev/null
>> @@ -1,30 +0,0 @@
>> -/*
>> - * QEMU System Emulator
>> - *
>> - * Copyright (c) 2003-2008 Fabrice Bellard
>> - *
>> - * Permission is hereby granted, free of charge, to any person 
>> obtaining a copy
>> - * of this software and associated documentation files (the 
>> "Software"), to deal
>> - * in the Software without restriction, including without limitation 
>> the rights
>> - * to use, copy, modify, merge, publish, distribute, sublicense, and/ 
>> or sell
>> - * copies of the Software, and to permit persons to whom the Software is
>> - * furnished to do so, subject to the following conditions:
>> - *
>> - * The above copyright notice and this permission notice shall be 
>> included in
>> - * all copies or substantial portions of the Software.
>> - *
>> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT 
>> SHALL
>> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING FROM,
>> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
>> DEALINGS IN
>> - * THE SOFTWARE.
>> - */
>> -#include "qemu/osdep.h"
>> -#include "system/arch_init.h"
>> -
>> -bool qemu_arch_available(unsigned qemu_arch_mask)
>> -{
>> -    return qemu_arch_mask & QEMU_ARCH;
>> -}
>> diff --git a/target-info.c b/target-info.c
>> index 24696ff4111..774fdcd2c46 100644
>> --- a/target-info.c
>> +++ b/target-info.c
>> @@ -11,6 +11,7 @@
>>   #include "qemu/target-info-qapi.h"
>>   #include "qemu/target-info-impl.h"
>>   #include "qapi/error.h"
>> +#include "system/arch_init.h"
>>   const char *target_name(void)
>>   {
>> @@ -33,6 +34,65 @@ SysEmuTarget target_arch(void)
>>       return arch;
>>   }
>> +bool qemu_arch_available(unsigned qemu_arch_mask)
>> +{
>> +    switch (target_arch()) {
>> +    case SYS_EMU_TARGET_ALPHA:
>> +        return qemu_arch_mask & QEMU_ARCH_ALPHA;
>> +    case SYS_EMU_TARGET_ARM:
>> +    case SYS_EMU_TARGET_AARCH64:
>> +        return qemu_arch_mask & QEMU_ARCH_ARM;
>> +    case SYS_EMU_TARGET_I386:
>> +    case SYS_EMU_TARGET_X86_64:
>> +        return qemu_arch_mask & QEMU_ARCH_I386;
>> +    case SYS_EMU_TARGET_M68K:
>> +        return qemu_arch_mask & QEMU_ARCH_M68K;
>> +    case SYS_EMU_TARGET_MICROBLAZE:
>> +    case SYS_EMU_TARGET_MICROBLAZEEL:
>> +        return qemu_arch_mask & QEMU_ARCH_MICROBLAZE;
>> +    case SYS_EMU_TARGET_MIPS:
>> +    case SYS_EMU_TARGET_MIPSEL:
>> +    case SYS_EMU_TARGET_MIPS64:
>> +    case SYS_EMU_TARGET_MIPS64EL:
>> +        return qemu_arch_mask & QEMU_ARCH_MIPS;
>> +    case SYS_EMU_TARGET_PPC:
>> +    case SYS_EMU_TARGET_PPC64:
>> +        return qemu_arch_mask & QEMU_ARCH_PPC;
>> +    case SYS_EMU_TARGET_S390X:
>> +        return qemu_arch_mask & QEMU_ARCH_S390X;
>> +    case SYS_EMU_TARGET_SH4:
>> +    case SYS_EMU_TARGET_SH4EB:
>> +        return qemu_arch_mask & QEMU_ARCH_SH4;
>> +    case SYS_EMU_TARGET_SPARC:
>> +    case SYS_EMU_TARGET_SPARC64:
>> +        return qemu_arch_mask & QEMU_ARCH_SPARC;
>> +    case SYS_EMU_TARGET_XTENSA:
>> +    case SYS_EMU_TARGET_XTENSAEB:
>> +        return qemu_arch_mask & QEMU_ARCH_XTENSA;
>> +    case SYS_EMU_TARGET_OR1K:
>> +        return qemu_arch_mask & QEMU_ARCH_OPENRISC;
>> +    case SYS_EMU_TARGET_TRICORE:
>> +        return qemu_arch_mask & QEMU_ARCH_TRICORE;
>> +    case SYS_EMU_TARGET_HPPA:
>> +        return qemu_arch_mask & QEMU_ARCH_HPPA;
>> +    case SYS_EMU_TARGET_RISCV32:
>> +    case SYS_EMU_TARGET_RISCV64:
>> +        return qemu_arch_mask & QEMU_ARCH_RISCV;
>> +    case SYS_EMU_TARGET_RX:
>> +        return qemu_arch_mask & QEMU_ARCH_RX;
>> +    case SYS_EMU_TARGET_AVR:
>> +        return qemu_arch_mask & QEMU_ARCH_AVR;
>> +    /*
>> +    case SYS_EMU_TARGET_HEXAGON:
>> +        return qemu_arch_mask & QEMU_ARCH_HEXAGON;
>> +    */
>> +    case SYS_EMU_TARGET_LOONGARCH64:
>> +        return qemu_arch_mask & QEMU_ARCH_LOONGARCH;
>> +    default:
>> +        g_assert_not_reached();
> 
> Per explaination on previous version, would be more interesting to have 
> SYS_EMU_TARGET__MAX instead of default here, so compilation will be 
> blocked when enum is extended.

How compilation can be blocked without using -Wswitch?

> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
>> +    };
>> +}
>> +
>>   const char *target_cpu_type(void)
>>   {
>>       return target_info()->cpu_type;
>> diff --git a/system/meson.build b/system/meson.build
>> index 4b69ef0f5fb..66e16db55ce 100644
>> --- a/system/meson.build
>> +++ b/system/meson.build
>> @@ -1,5 +1,4 @@
>>   specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
>> -  'arch_init.c',
>>     'globals-target.c',
>>   )])
> 


Re: [PATCH v3] system: Convert qemu_arch_available() to TargetInfo API
Posted by Pierrick Bouvier 4 weeks, 1 day ago
On 1/8/26 9:53 PM, Philippe Mathieu-Daudé wrote:
> On 8/1/26 19:57, Pierrick Bouvier wrote:
>> On 1/8/26 8:36 AM, Philippe Mathieu-Daudé wrote:
>>> Get the base arch_mask from the current SysEmuTarget,
>>> making qemu_arch_available() target-agnostic.
>>>
>>> We don't need the per-target QEMU_ARCH definition anymore,
>>> remove it.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> v3: Return directly within switch
>>> v2: Prefer switch over array (pbo)
>>> ---
>>>    meson.build        |  2 --
>>>    system/arch_init.c | 30 -----------------------
>>>    target-info.c      | 60 ++++++++++++++++++++++++++++++++++++++++++++++
>>>    system/meson.build |  1 -
>>>    4 files changed, 60 insertions(+), 33 deletions(-)
>>>    delete mode 100644 system/arch_init.c
>>>
>>> diff --git a/meson.build b/meson.build
>>> index 734c801cc77..435dc6e3c8e 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -3419,8 +3419,6 @@ foreach target : target_dirs
>>>          config_target_data.set(k, v)
>>>        endif
>>>      endforeach
>>> -  config_target_data.set('QEMU_ARCH',
>>> -                         'QEMU_ARCH_' +
>>> config_target['TARGET_BASE_ARCH'].to_upper())
>>>      config_target_h += {target: configure_file(output: target + '-
>>> config-target.h',
>>>                                                   configuration:
>>> config_target_data)}
>>> diff --git a/system/arch_init.c b/system/arch_init.c
>>> deleted file mode 100644
>>> index e85736884c9..00000000000
>>> --- a/system/arch_init.c
>>> +++ /dev/null
>>> @@ -1,30 +0,0 @@
>>> -/*
>>> - * QEMU System Emulator
>>> - *
>>> - * Copyright (c) 2003-2008 Fabrice Bellard
>>> - *
>>> - * Permission is hereby granted, free of charge, to any person
>>> obtaining a copy
>>> - * of this software and associated documentation files (the
>>> "Software"), to deal
>>> - * in the Software without restriction, including without limitation
>>> the rights
>>> - * to use, copy, modify, merge, publish, distribute, sublicense, and/
>>> or sell
>>> - * copies of the Software, and to permit persons to whom the Software is
>>> - * furnished to do so, subject to the following conditions:
>>> - *
>>> - * The above copyright notice and this permission notice shall be
>>> included in
>>> - * all copies or substantial portions of the Software.
>>> - *
>>> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>> EXPRESS OR
>>> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>> MERCHANTABILITY,
>>> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>>> SHALL
>>> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
>>> OR OTHER
>>> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>>> ARISING FROM,
>>> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>>> DEALINGS IN
>>> - * THE SOFTWARE.
>>> - */
>>> -#include "qemu/osdep.h"
>>> -#include "system/arch_init.h"
>>> -
>>> -bool qemu_arch_available(unsigned qemu_arch_mask)
>>> -{
>>> -    return qemu_arch_mask & QEMU_ARCH;
>>> -}
>>> diff --git a/target-info.c b/target-info.c
>>> index 24696ff4111..774fdcd2c46 100644
>>> --- a/target-info.c
>>> +++ b/target-info.c
>>> @@ -11,6 +11,7 @@
>>>    #include "qemu/target-info-qapi.h"
>>>    #include "qemu/target-info-impl.h"
>>>    #include "qapi/error.h"
>>> +#include "system/arch_init.h"
>>>    const char *target_name(void)
>>>    {
>>> @@ -33,6 +34,65 @@ SysEmuTarget target_arch(void)
>>>        return arch;
>>>    }
>>> +bool qemu_arch_available(unsigned qemu_arch_mask)
>>> +{
>>> +    switch (target_arch()) {
>>> +    case SYS_EMU_TARGET_ALPHA:
>>> +        return qemu_arch_mask & QEMU_ARCH_ALPHA;
>>> +    case SYS_EMU_TARGET_ARM:
>>> +    case SYS_EMU_TARGET_AARCH64:
>>> +        return qemu_arch_mask & QEMU_ARCH_ARM;
>>> +    case SYS_EMU_TARGET_I386:
>>> +    case SYS_EMU_TARGET_X86_64:
>>> +        return qemu_arch_mask & QEMU_ARCH_I386;
>>> +    case SYS_EMU_TARGET_M68K:
>>> +        return qemu_arch_mask & QEMU_ARCH_M68K;
>>> +    case SYS_EMU_TARGET_MICROBLAZE:
>>> +    case SYS_EMU_TARGET_MICROBLAZEEL:
>>> +        return qemu_arch_mask & QEMU_ARCH_MICROBLAZE;
>>> +    case SYS_EMU_TARGET_MIPS:
>>> +    case SYS_EMU_TARGET_MIPSEL:
>>> +    case SYS_EMU_TARGET_MIPS64:
>>> +    case SYS_EMU_TARGET_MIPS64EL:
>>> +        return qemu_arch_mask & QEMU_ARCH_MIPS;
>>> +    case SYS_EMU_TARGET_PPC:
>>> +    case SYS_EMU_TARGET_PPC64:
>>> +        return qemu_arch_mask & QEMU_ARCH_PPC;
>>> +    case SYS_EMU_TARGET_S390X:
>>> +        return qemu_arch_mask & QEMU_ARCH_S390X;
>>> +    case SYS_EMU_TARGET_SH4:
>>> +    case SYS_EMU_TARGET_SH4EB:
>>> +        return qemu_arch_mask & QEMU_ARCH_SH4;
>>> +    case SYS_EMU_TARGET_SPARC:
>>> +    case SYS_EMU_TARGET_SPARC64:
>>> +        return qemu_arch_mask & QEMU_ARCH_SPARC;
>>> +    case SYS_EMU_TARGET_XTENSA:
>>> +    case SYS_EMU_TARGET_XTENSAEB:
>>> +        return qemu_arch_mask & QEMU_ARCH_XTENSA;
>>> +    case SYS_EMU_TARGET_OR1K:
>>> +        return qemu_arch_mask & QEMU_ARCH_OPENRISC;
>>> +    case SYS_EMU_TARGET_TRICORE:
>>> +        return qemu_arch_mask & QEMU_ARCH_TRICORE;
>>> +    case SYS_EMU_TARGET_HPPA:
>>> +        return qemu_arch_mask & QEMU_ARCH_HPPA;
>>> +    case SYS_EMU_TARGET_RISCV32:
>>> +    case SYS_EMU_TARGET_RISCV64:
>>> +        return qemu_arch_mask & QEMU_ARCH_RISCV;
>>> +    case SYS_EMU_TARGET_RX:
>>> +        return qemu_arch_mask & QEMU_ARCH_RX;
>>> +    case SYS_EMU_TARGET_AVR:
>>> +        return qemu_arch_mask & QEMU_ARCH_AVR;
>>> +    /*
>>> +    case SYS_EMU_TARGET_HEXAGON:
>>> +        return qemu_arch_mask & QEMU_ARCH_HEXAGON;
>>> +    */
>>> +    case SYS_EMU_TARGET_LOONGARCH64:
>>> +        return qemu_arch_mask & QEMU_ARCH_LOONGARCH;
>>> +    default:
>>> +        g_assert_not_reached();
>>
>> Per explaination on previous version, would be more interesting to have
>> SYS_EMU_TARGET__MAX instead of default here, so compilation will be
>> blocked when enum is extended.
> 
> How compilation can be blocked without using -Wswitch?
>

See my previous answer on v1, -Wswitch is used:

Reading the thread above, the only mention I find is the 3rd commit that
precisely change definition because -Wswitch is enabled with clang.
And it's not only a clang thing, gcc has it in Wall also [1].

I don't mind the array approach, but maybe just add a *static* assert
making sure (SYS_EMU_TARGET__MAX-1 == SYS_EMU_TARGET_XTENSAEB) so this
file will break as soon as there is a new target added. It's simple and
the next developer who won't have to debug this will thank you.

[1] https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Warning-Options.html

>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>>> +    };
>>> +}
>>> +
>>>    const char *target_cpu_type(void)
>>>    {
>>>        return target_info()->cpu_type;
>>> diff --git a/system/meson.build b/system/meson.build
>>> index 4b69ef0f5fb..66e16db55ce 100644
>>> --- a/system/meson.build
>>> +++ b/system/meson.build
>>> @@ -1,5 +1,4 @@
>>>    specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
>>> -  'arch_init.c',
>>>      'globals-target.c',
>>>    )])
>>
> 


Re: [PATCH v3] system: Convert qemu_arch_available() to TargetInfo API
Posted by Philippe Mathieu-Daudé 4 weeks ago
On 9/1/26 07:58, Pierrick Bouvier wrote:
> On 1/8/26 9:53 PM, Philippe Mathieu-Daudé wrote:
>> On 8/1/26 19:57, Pierrick Bouvier wrote:
>>> On 1/8/26 8:36 AM, Philippe Mathieu-Daudé wrote:
>>>> Get the base arch_mask from the current SysEmuTarget,
>>>> making qemu_arch_available() target-agnostic.
>>>>
>>>> We don't need the per-target QEMU_ARCH definition anymore,
>>>> remove it.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>> v3: Return directly within switch
>>>> v2: Prefer switch over array (pbo)
>>>> ---
>>>>    meson.build        |  2 --
>>>>    system/arch_init.c | 30 -----------------------
>>>>    target-info.c      | 60 +++++++++++++++++++++++++++++++++++++++++ 
>>>> +++++
>>>>    system/meson.build |  1 -
>>>>    4 files changed, 60 insertions(+), 33 deletions(-)
>>>>    delete mode 100644 system/arch_init.c
>>>>
>>>> diff --git a/meson.build b/meson.build
>>>> index 734c801cc77..435dc6e3c8e 100644
>>>> --- a/meson.build
>>>> +++ b/meson.build
>>>> @@ -3419,8 +3419,6 @@ foreach target : target_dirs
>>>>          config_target_data.set(k, v)
>>>>        endif
>>>>      endforeach
>>>> -  config_target_data.set('QEMU_ARCH',
>>>> -                         'QEMU_ARCH_' +
>>>> config_target['TARGET_BASE_ARCH'].to_upper())
>>>>      config_target_h += {target: configure_file(output: target + '-
>>>> config-target.h',
>>>>                                                   configuration:
>>>> config_target_data)}
>>>> diff --git a/system/arch_init.c b/system/arch_init.c
>>>> deleted file mode 100644
>>>> index e85736884c9..00000000000
>>>> --- a/system/arch_init.c
>>>> +++ /dev/null
>>>> @@ -1,30 +0,0 @@
>>>> -/*
>>>> - * QEMU System Emulator
>>>> - *
>>>> - * Copyright (c) 2003-2008 Fabrice Bellard
>>>> - *
>>>> - * Permission is hereby granted, free of charge, to any person
>>>> obtaining a copy
>>>> - * of this software and associated documentation files (the
>>>> "Software"), to deal
>>>> - * in the Software without restriction, including without limitation
>>>> the rights
>>>> - * to use, copy, modify, merge, publish, distribute, sublicense, and/
>>>> or sell
>>>> - * copies of the Software, and to permit persons to whom the 
>>>> Software is
>>>> - * furnished to do so, subject to the following conditions:
>>>> - *
>>>> - * The above copyright notice and this permission notice shall be
>>>> included in
>>>> - * all copies or substantial portions of the Software.
>>>> - *
>>>> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>>>> EXPRESS OR
>>>> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>>> MERCHANTABILITY,
>>>> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>>>> SHALL
>>>> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
>>>> OR OTHER
>>>> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>>>> ARISING FROM,
>>>> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>>>> DEALINGS IN
>>>> - * THE SOFTWARE.
>>>> - */
>>>> -#include "qemu/osdep.h"
>>>> -#include "system/arch_init.h"
>>>> -
>>>> -bool qemu_arch_available(unsigned qemu_arch_mask)
>>>> -{
>>>> -    return qemu_arch_mask & QEMU_ARCH;
>>>> -}
>>>> diff --git a/target-info.c b/target-info.c
>>>> index 24696ff4111..774fdcd2c46 100644
>>>> --- a/target-info.c
>>>> +++ b/target-info.c
>>>> @@ -11,6 +11,7 @@
>>>>    #include "qemu/target-info-qapi.h"
>>>>    #include "qemu/target-info-impl.h"
>>>>    #include "qapi/error.h"
>>>> +#include "system/arch_init.h"
>>>>    const char *target_name(void)
>>>>    {
>>>> @@ -33,6 +34,65 @@ SysEmuTarget target_arch(void)
>>>>        return arch;
>>>>    }
>>>> +bool qemu_arch_available(unsigned qemu_arch_mask)
>>>> +{
>>>> +    switch (target_arch()) {
>>>> +    case SYS_EMU_TARGET_ALPHA:
>>>> +        return qemu_arch_mask & QEMU_ARCH_ALPHA;
>>>> +    case SYS_EMU_TARGET_ARM:
>>>> +    case SYS_EMU_TARGET_AARCH64:
>>>> +        return qemu_arch_mask & QEMU_ARCH_ARM;
>>>> +    case SYS_EMU_TARGET_I386:
>>>> +    case SYS_EMU_TARGET_X86_64:
>>>> +        return qemu_arch_mask & QEMU_ARCH_I386;
>>>> +    case SYS_EMU_TARGET_M68K:
>>>> +        return qemu_arch_mask & QEMU_ARCH_M68K;
>>>> +    case SYS_EMU_TARGET_MICROBLAZE:
>>>> +    case SYS_EMU_TARGET_MICROBLAZEEL:
>>>> +        return qemu_arch_mask & QEMU_ARCH_MICROBLAZE;
>>>> +    case SYS_EMU_TARGET_MIPS:
>>>> +    case SYS_EMU_TARGET_MIPSEL:
>>>> +    case SYS_EMU_TARGET_MIPS64:
>>>> +    case SYS_EMU_TARGET_MIPS64EL:
>>>> +        return qemu_arch_mask & QEMU_ARCH_MIPS;
>>>> +    case SYS_EMU_TARGET_PPC:
>>>> +    case SYS_EMU_TARGET_PPC64:
>>>> +        return qemu_arch_mask & QEMU_ARCH_PPC;
>>>> +    case SYS_EMU_TARGET_S390X:
>>>> +        return qemu_arch_mask & QEMU_ARCH_S390X;
>>>> +    case SYS_EMU_TARGET_SH4:
>>>> +    case SYS_EMU_TARGET_SH4EB:
>>>> +        return qemu_arch_mask & QEMU_ARCH_SH4;
>>>> +    case SYS_EMU_TARGET_SPARC:
>>>> +    case SYS_EMU_TARGET_SPARC64:
>>>> +        return qemu_arch_mask & QEMU_ARCH_SPARC;
>>>> +    case SYS_EMU_TARGET_XTENSA:
>>>> +    case SYS_EMU_TARGET_XTENSAEB:
>>>> +        return qemu_arch_mask & QEMU_ARCH_XTENSA;
>>>> +    case SYS_EMU_TARGET_OR1K:
>>>> +        return qemu_arch_mask & QEMU_ARCH_OPENRISC;
>>>> +    case SYS_EMU_TARGET_TRICORE:
>>>> +        return qemu_arch_mask & QEMU_ARCH_TRICORE;
>>>> +    case SYS_EMU_TARGET_HPPA:
>>>> +        return qemu_arch_mask & QEMU_ARCH_HPPA;
>>>> +    case SYS_EMU_TARGET_RISCV32:
>>>> +    case SYS_EMU_TARGET_RISCV64:
>>>> +        return qemu_arch_mask & QEMU_ARCH_RISCV;
>>>> +    case SYS_EMU_TARGET_RX:
>>>> +        return qemu_arch_mask & QEMU_ARCH_RX;
>>>> +    case SYS_EMU_TARGET_AVR:
>>>> +        return qemu_arch_mask & QEMU_ARCH_AVR;
>>>> +    /*
>>>> +    case SYS_EMU_TARGET_HEXAGON:
>>>> +        return qemu_arch_mask & QEMU_ARCH_HEXAGON;
>>>> +    */
>>>> +    case SYS_EMU_TARGET_LOONGARCH64:
>>>> +        return qemu_arch_mask & QEMU_ARCH_LOONGARCH;
>>>> +    default:
>>>> +        g_assert_not_reached();
>>>
>>> Per explaination on previous version, would be more interesting to have
>>> SYS_EMU_TARGET__MAX instead of default here, so compilation will be
>>> blocked when enum is extended.
>>
>> How compilation can be blocked without using -Wswitch?
>>
> 
> See my previous answer on v1, -Wswitch is used:
> 
> Reading the thread above, the only mention I find is the 3rd commit that
> precisely change definition because -Wswitch is enabled with clang.
> And it's not only a clang thing, gcc has it in Wall also [1].

Yes I read that, I'd really like we use -Wswitch but IIUC we can not,
so with that in mind I don't understand your request. Is that for the
hypothetical case we can use -Wswitch in the future? Sorry I'm not
trying to be picky here, I just fail to see the problem you raised :(

> I don't mind the array approach, but maybe just add a *static* assert
> making sure (SYS_EMU_TARGET__MAX-1 == SYS_EMU_TARGET_XTENSAEB) so this
> file will break as soon as there is a new target added. It's simple and
> the next developer who won't have to debug this will thank you.
> 
> [1] https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Warning-Options.html
> 
>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>
>>>> +    };
>>>> +}
>>>> +
>>>>    const char *target_cpu_type(void)
>>>>    {
>>>>        return target_info()->cpu_type;
>>>> diff --git a/system/meson.build b/system/meson.build
>>>> index 4b69ef0f5fb..66e16db55ce 100644
>>>> --- a/system/meson.build
>>>> +++ b/system/meson.build
>>>> @@ -1,5 +1,4 @@
>>>>    specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
>>>> -  'arch_init.c',
>>>>      'globals-target.c',
>>>>    )])
>>>
>>
> 


Re: [PATCH v3] system: Convert qemu_arch_available() to TargetInfo API
Posted by Markus Armbruster 4 weeks ago
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 9/1/26 07:58, Pierrick Bouvier wrote:

[...]

>> See my previous answer on v1, -Wswitch is used:
>> Reading the thread above, the only mention I find is the 3rd commit that
>> precisely change definition because -Wswitch is enabled with clang.
>> And it's not only a clang thing, gcc has it in Wall also [1].
>
> Yes I read that, I'd really like we use -Wswitch but IIUC we can not,
> so with that in mind I don't understand your request. Is that for the
> hypothetical case we can use -Wswitch in the future? Sorry I'm not
> trying to be picky here, I just fail to see the problem you raised :(

We *are* using -Wswitch.

To see that, delete the default: case in to_json()'s outer switch (patch
appended), and compile:

    ../qobject/qjson.c: In function ‘to_json’:
    ../qobject/qjson.c:154:5: warning: enumeration value ‘QTYPE_NONE’ not handled in switch [-Wswitch]
      154 |     switch (qobject_type(obj)) {
          |     ^~~~~~
    ../qobject/qjson.c:154:5: warning: enumeration value ‘QTYPE__MAX’ not handled in switch [-Wswitch]

We run gcc -Wall, which implies -Wswitch.

[...]


diff --git a/qobject/qjson.c b/qobject/qjson.c
index c858dafb5e..6287c93c67 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -213,8 +213,6 @@ static void to_json(JSONWriter *writer, const char *name,
         json_writer_bool(writer, name, qbool_get_bool(val));
         break;
     }
-    default:
-        abort();
     }
 }
 
Re: [PATCH v3] system: Convert qemu_arch_available() to TargetInfo API
Posted by Philippe Mathieu-Daudé 4 weeks ago
On 9/1/26 09:18, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> On 9/1/26 07:58, Pierrick Bouvier wrote:
> 
> [...]
> 
>>> See my previous answer on v1, -Wswitch is used:
>>> Reading the thread above, the only mention I find is the 3rd commit that
>>> precisely change definition because -Wswitch is enabled with clang.
>>> And it's not only a clang thing, gcc has it in Wall also [1].
>>
>> Yes I read that, I'd really like we use -Wswitch but IIUC we can not,
>> so with that in mind I don't understand your request. Is that for the
>> hypothetical case we can use -Wswitch in the future? Sorry I'm not
>> trying to be picky here, I just fail to see the problem you raised :(
> 
> We *are* using -Wswitch.

Oh... I guess I meant -Wswitch-enum then (which forces to handle the
-- meaningless in that case -- QAPI foo__MAX enum values).

Anyway time to stop bikeshedding and call it a day (originally I just
wanted to remove the duplicated qemu_arch_available symbol :) ).

> To see that, delete the default: case in to_json()'s outer switch (patch
> appended), and compile:
> 
>      ../qobject/qjson.c: In function ‘to_json’:
>      ../qobject/qjson.c:154:5: warning: enumeration value ‘QTYPE_NONE’ not handled in switch [-Wswitch]
>        154 |     switch (qobject_type(obj)) {
>            |     ^~~~~~
>      ../qobject/qjson.c:154:5: warning: enumeration value ‘QTYPE__MAX’ not handled in switch [-Wswitch]
> 
> We run gcc -Wall, which implies -Wswitch.
> 
> [...]
> 
> 
> diff --git a/qobject/qjson.c b/qobject/qjson.c
> index c858dafb5e..6287c93c67 100644
> --- a/qobject/qjson.c
> +++ b/qobject/qjson.c
> @@ -213,8 +213,6 @@ static void to_json(JSONWriter *writer, const char *name,
>           json_writer_bool(writer, name, qbool_get_bool(val));
>           break;
>       }
> -    default:
> -        abort();
>       }
>   }
>   
> 


Re: [PATCH v3] system: Convert qemu_arch_available() to TargetInfo API
Posted by Pierrick Bouvier 4 weeks ago
On 1/9/26 12:32 AM, Philippe Mathieu-Daudé wrote:
> On 9/1/26 09:18, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> On 9/1/26 07:58, Pierrick Bouvier wrote:
>>
>> [...]
>>
>>>> See my previous answer on v1, -Wswitch is used:
>>>> Reading the thread above, the only mention I find is the 3rd commit that
>>>> precisely change definition because -Wswitch is enabled with clang.
>>>> And it's not only a clang thing, gcc has it in Wall also [1].
>>>
>>> Yes I read that, I'd really like we use -Wswitch but IIUC we can not,
>>> so with that in mind I don't understand your request. Is that for the
>>> hypothetical case we can use -Wswitch in the future? Sorry I'm not
>>> trying to be picky here, I just fail to see the problem you raised :(
>>
>> We *are* using -Wswitch.
> 
> Oh... I guess I meant -Wswitch-enum then (which forces to handle the
> -- meaningless in that case -- QAPI foo__MAX enum values).
> 

Yes :)

> Anyway time to stop bikeshedding and call it a day (originally I just
> wanted to remove the duplicated qemu_arch_available symbol :) ).
>

I agree, and sorry for the discussion that has been triggered.

My original point was that a switch allows to catch missing cases, but 
*ONLY* if it does not include a default:.
So either stick to the array, or change the switch to remove the default 
case. Even though I prefer the switch (without default), I don't mind if 
you prefer the array at this point. You had my review on first version 
anyway.

>> To see that, delete the default: case in to_json()'s outer switch (patch
>> appended), and compile:
>>
>>       ../qobject/qjson.c: In function ‘to_json’:
>>       ../qobject/qjson.c:154:5: warning: enumeration value ‘QTYPE_NONE’ not handled in switch [-Wswitch]
>>         154 |     switch (qobject_type(obj)) {
>>             |     ^~~~~~
>>       ../qobject/qjson.c:154:5: warning: enumeration value ‘QTYPE__MAX’ not handled in switch [-Wswitch]
>>
>> We run gcc -Wall, which implies -Wswitch.
>>

Thanks Markus, that was my point indeed.

>> [...]
>>
>>
>> diff --git a/qobject/qjson.c b/qobject/qjson.c
>> index c858dafb5e..6287c93c67 100644
>> --- a/qobject/qjson.c
>> +++ b/qobject/qjson.c
>> @@ -213,8 +213,6 @@ static void to_json(JSONWriter *writer, const char *name,
>>            json_writer_bool(writer, name, qbool_get_bool(val));
>>            break;
>>        }
>> -    default:
>> -        abort();
>>        }
>>    }
>>    
>>
> 


Re: [PATCH v3] system: Convert qemu_arch_available() to TargetInfo API
Posted by Alex Bennée 4 weeks, 1 day ago
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Get the base arch_mask from the current SysEmuTarget,
> making qemu_arch_available() target-agnostic.
>
> We don't need the per-target QEMU_ARCH definition anymore,
> remove it.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v3: Return directly within switch
> v2: Prefer switch over array (pbo)
> ---
>  meson.build        |  2 --
>  system/arch_init.c | 30 -----------------------
>  target-info.c      | 60 ++++++++++++++++++++++++++++++++++++++++++++++
>  system/meson.build |  1 -
>  4 files changed, 60 insertions(+), 33 deletions(-)
>  delete mode 100644 system/arch_init.c
>
> diff --git a/meson.build b/meson.build
> index 734c801cc77..435dc6e3c8e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3419,8 +3419,6 @@ foreach target : target_dirs
>        config_target_data.set(k, v)
>      endif
>    endforeach
> -  config_target_data.set('QEMU_ARCH',
> -                         'QEMU_ARCH_' + config_target['TARGET_BASE_ARCH'].to_upper())
>    config_target_h += {target: configure_file(output: target + '-config-target.h',
>                                                 configuration: config_target_data)}
>  
> diff --git a/system/arch_init.c b/system/arch_init.c
> deleted file mode 100644
> index e85736884c9..00000000000
> --- a/system/arch_init.c
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -/*
> - * QEMU System Emulator
> - *
> - * Copyright (c) 2003-2008 Fabrice Bellard
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a copy
> - * of this software and associated documentation files (the "Software"), to deal
> - * in the Software without restriction, including without limitation the rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - */
> -#include "qemu/osdep.h"
> -#include "system/arch_init.h"
> -
> -bool qemu_arch_available(unsigned qemu_arch_mask)
> -{
> -    return qemu_arch_mask & QEMU_ARCH;
> -}
> diff --git a/target-info.c b/target-info.c
> index 24696ff4111..774fdcd2c46 100644
> --- a/target-info.c
> +++ b/target-info.c
> @@ -11,6 +11,7 @@
>  #include "qemu/target-info-qapi.h"
>  #include "qemu/target-info-impl.h"
>  #include "qapi/error.h"
> +#include "system/arch_init.h"
>  
>  const char *target_name(void)
>  {
> @@ -33,6 +34,65 @@ SysEmuTarget target_arch(void)
>      return arch;
>  }
>

This looks fine to me but I note the API takes a mask which implies you
could potentially pass multiple arches in the mask. Not an issue now but
we should probably add a kdoc to the declaration to make it clear you
can only test one arch at a time for now.

> +bool qemu_arch_available(unsigned qemu_arch_mask)
> +{
> +    switch (target_arch()) {
> +    case SYS_EMU_TARGET_ALPHA:
> +        return qemu_arch_mask & QEMU_ARCH_ALPHA;
> +    case SYS_EMU_TARGET_ARM:
> +    case SYS_EMU_TARGET_AARCH64:
> +        return qemu_arch_mask & QEMU_ARCH_ARM;
> +    case SYS_EMU_TARGET_I386:
> +    case SYS_EMU_TARGET_X86_64:
> +        return qemu_arch_mask & QEMU_ARCH_I386;
> +    case SYS_EMU_TARGET_M68K:
> +        return qemu_arch_mask & QEMU_ARCH_M68K;
> +    case SYS_EMU_TARGET_MICROBLAZE:
> +    case SYS_EMU_TARGET_MICROBLAZEEL:
> +        return qemu_arch_mask & QEMU_ARCH_MICROBLAZE;
> +    case SYS_EMU_TARGET_MIPS:
> +    case SYS_EMU_TARGET_MIPSEL:
> +    case SYS_EMU_TARGET_MIPS64:
> +    case SYS_EMU_TARGET_MIPS64EL:
> +        return qemu_arch_mask & QEMU_ARCH_MIPS;
> +    case SYS_EMU_TARGET_PPC:
> +    case SYS_EMU_TARGET_PPC64:
> +        return qemu_arch_mask & QEMU_ARCH_PPC;
> +    case SYS_EMU_TARGET_S390X:
> +        return qemu_arch_mask & QEMU_ARCH_S390X;
> +    case SYS_EMU_TARGET_SH4:
> +    case SYS_EMU_TARGET_SH4EB:
> +        return qemu_arch_mask & QEMU_ARCH_SH4;
> +    case SYS_EMU_TARGET_SPARC:
> +    case SYS_EMU_TARGET_SPARC64:
> +        return qemu_arch_mask & QEMU_ARCH_SPARC;
> +    case SYS_EMU_TARGET_XTENSA:
> +    case SYS_EMU_TARGET_XTENSAEB:
> +        return qemu_arch_mask & QEMU_ARCH_XTENSA;
> +    case SYS_EMU_TARGET_OR1K:
> +        return qemu_arch_mask & QEMU_ARCH_OPENRISC;
> +    case SYS_EMU_TARGET_TRICORE:
> +        return qemu_arch_mask & QEMU_ARCH_TRICORE;
> +    case SYS_EMU_TARGET_HPPA:
> +        return qemu_arch_mask & QEMU_ARCH_HPPA;
> +    case SYS_EMU_TARGET_RISCV32:
> +    case SYS_EMU_TARGET_RISCV64:
> +        return qemu_arch_mask & QEMU_ARCH_RISCV;
> +    case SYS_EMU_TARGET_RX:
> +        return qemu_arch_mask & QEMU_ARCH_RX;
> +    case SYS_EMU_TARGET_AVR:
> +        return qemu_arch_mask & QEMU_ARCH_AVR;
> +    /*
> +    case SYS_EMU_TARGET_HEXAGON:
> +        return qemu_arch_mask & QEMU_ARCH_HEXAGON;
> +    */
> +    case SYS_EMU_TARGET_LOONGARCH64:
> +        return qemu_arch_mask & QEMU_ARCH_LOONGARCH;
> +    default:
> +        g_assert_not_reached();
> +    };
> +}
> +
>  const char *target_cpu_type(void)
>  {
>      return target_info()->cpu_type;
> diff --git a/system/meson.build b/system/meson.build
> index 4b69ef0f5fb..66e16db55ce 100644
> --- a/system/meson.build
> +++ b/system/meson.build
> @@ -1,5 +1,4 @@
>  specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
> -  'arch_init.c',
>    'globals-target.c',
>  )])

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v3] system: Convert qemu_arch_available() to TargetInfo API
Posted by Philippe Mathieu-Daudé 4 weeks ago
On 8/1/26 18:06, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Get the base arch_mask from the current SysEmuTarget,
>> making qemu_arch_available() target-agnostic.
>>
>> We don't need the per-target QEMU_ARCH definition anymore,
>> remove it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> v3: Return directly within switch
>> v2: Prefer switch over array (pbo)
>> ---
>>   meson.build        |  2 --
>>   system/arch_init.c | 30 -----------------------
>>   target-info.c      | 60 ++++++++++++++++++++++++++++++++++++++++++++++
>>   system/meson.build |  1 -
>>   4 files changed, 60 insertions(+), 33 deletions(-)
>>   delete mode 100644 system/arch_init.c
>>
>> diff --git a/meson.build b/meson.build
>> index 734c801cc77..435dc6e3c8e 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -3419,8 +3419,6 @@ foreach target : target_dirs
>>         config_target_data.set(k, v)
>>       endif
>>     endforeach
>> -  config_target_data.set('QEMU_ARCH',
>> -                         'QEMU_ARCH_' + config_target['TARGET_BASE_ARCH'].to_upper())
>>     config_target_h += {target: configure_file(output: target + '-config-target.h',
>>                                                  configuration: config_target_data)}
>>   
>> diff --git a/system/arch_init.c b/system/arch_init.c
>> deleted file mode 100644
>> index e85736884c9..00000000000
>> --- a/system/arch_init.c
>> +++ /dev/null
>> @@ -1,30 +0,0 @@
>> -/*
>> - * QEMU System Emulator
>> - *
>> - * Copyright (c) 2003-2008 Fabrice Bellard
>> - *
>> - * Permission is hereby granted, free of charge, to any person obtaining a copy
>> - * of this software and associated documentation files (the "Software"), to deal
>> - * in the Software without restriction, including without limitation the rights
>> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> - * copies of the Software, and to permit persons to whom the Software is
>> - * furnished to do so, subject to the following conditions:
>> - *
>> - * The above copyright notice and this permission notice shall be included in
>> - * all copies or substantial portions of the Software.
>> - *
>> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> - * THE SOFTWARE.
>> - */
>> -#include "qemu/osdep.h"
>> -#include "system/arch_init.h"
>> -
>> -bool qemu_arch_available(unsigned qemu_arch_mask)
>> -{
>> -    return qemu_arch_mask & QEMU_ARCH;
>> -}
>> diff --git a/target-info.c b/target-info.c
>> index 24696ff4111..774fdcd2c46 100644
>> --- a/target-info.c
>> +++ b/target-info.c
>> @@ -11,6 +11,7 @@
>>   #include "qemu/target-info-qapi.h"
>>   #include "qemu/target-info-impl.h"
>>   #include "qapi/error.h"
>> +#include "system/arch_init.h"
>>   
>>   const char *target_name(void)
>>   {
>> @@ -33,6 +34,65 @@ SysEmuTarget target_arch(void)
>>       return arch;
>>   }
>>
> 
> This looks fine to me but I note the API takes a mask which implies you
> could potentially pass multiple arches in the mask. Not an issue now but
> we should probably add a kdoc to the declaration to make it clear you
> can only test one arch at a time for now.

There is no change in this API, your description is what is intended:
compile qemu-options once and display (-help) the relevant options for
the current (read in the future, "selected") target architecture.

If we want to add a @docstring to qemu_arch_available() this can be
done in parallel of this patch.

> 
>> +bool qemu_arch_available(unsigned qemu_arch_mask)
>> +{
>> +    switch (target_arch()) {
>> +    case SYS_EMU_TARGET_ALPHA:
>> +        return qemu_arch_mask & QEMU_ARCH_ALPHA;
>> +    case SYS_EMU_TARGET_ARM:
>> +    case SYS_EMU_TARGET_AARCH64:
>> +        return qemu_arch_mask & QEMU_ARCH_ARM;
>> +    case SYS_EMU_TARGET_I386:
>> +    case SYS_EMU_TARGET_X86_64:
>> +        return qemu_arch_mask & QEMU_ARCH_I386;
>> +    case SYS_EMU_TARGET_M68K:
>> +        return qemu_arch_mask & QEMU_ARCH_M68K;
>> +    case SYS_EMU_TARGET_MICROBLAZE:
>> +    case SYS_EMU_TARGET_MICROBLAZEEL:
>> +        return qemu_arch_mask & QEMU_ARCH_MICROBLAZE;
>> +    case SYS_EMU_TARGET_MIPS:
>> +    case SYS_EMU_TARGET_MIPSEL:
>> +    case SYS_EMU_TARGET_MIPS64:
>> +    case SYS_EMU_TARGET_MIPS64EL:
>> +        return qemu_arch_mask & QEMU_ARCH_MIPS;
>> +    case SYS_EMU_TARGET_PPC:
>> +    case SYS_EMU_TARGET_PPC64:
>> +        return qemu_arch_mask & QEMU_ARCH_PPC;
>> +    case SYS_EMU_TARGET_S390X:
>> +        return qemu_arch_mask & QEMU_ARCH_S390X;
>> +    case SYS_EMU_TARGET_SH4:
>> +    case SYS_EMU_TARGET_SH4EB:
>> +        return qemu_arch_mask & QEMU_ARCH_SH4;
>> +    case SYS_EMU_TARGET_SPARC:
>> +    case SYS_EMU_TARGET_SPARC64:
>> +        return qemu_arch_mask & QEMU_ARCH_SPARC;
>> +    case SYS_EMU_TARGET_XTENSA:
>> +    case SYS_EMU_TARGET_XTENSAEB:
>> +        return qemu_arch_mask & QEMU_ARCH_XTENSA;
>> +    case SYS_EMU_TARGET_OR1K:
>> +        return qemu_arch_mask & QEMU_ARCH_OPENRISC;
>> +    case SYS_EMU_TARGET_TRICORE:
>> +        return qemu_arch_mask & QEMU_ARCH_TRICORE;
>> +    case SYS_EMU_TARGET_HPPA:
>> +        return qemu_arch_mask & QEMU_ARCH_HPPA;
>> +    case SYS_EMU_TARGET_RISCV32:
>> +    case SYS_EMU_TARGET_RISCV64:
>> +        return qemu_arch_mask & QEMU_ARCH_RISCV;
>> +    case SYS_EMU_TARGET_RX:
>> +        return qemu_arch_mask & QEMU_ARCH_RX;
>> +    case SYS_EMU_TARGET_AVR:
>> +        return qemu_arch_mask & QEMU_ARCH_AVR;
>> +    /*
>> +    case SYS_EMU_TARGET_HEXAGON:
>> +        return qemu_arch_mask & QEMU_ARCH_HEXAGON;
>> +    */
>> +    case SYS_EMU_TARGET_LOONGARCH64:
>> +        return qemu_arch_mask & QEMU_ARCH_LOONGARCH;
>> +    default:
>> +        g_assert_not_reached();
>> +    };
>> +}
>> +
>>   const char *target_cpu_type(void)
>>   {
>>       return target_info()->cpu_type;
>> diff --git a/system/meson.build b/system/meson.build
>> index 4b69ef0f5fb..66e16db55ce 100644
>> --- a/system/meson.build
>> +++ b/system/meson.build
>> @@ -1,5 +1,4 @@
>>   specific_ss.add(when: 'CONFIG_SYSTEM_ONLY', if_true: [files(
>> -  'arch_init.c',
>>     'globals-target.c',
>>   )])
> 
> Otherwise:
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Thanks!