Replace the static array returned as MachineClass::valid_cpu_types[]
by a runtime one generated by MachineClass::get_valid_cpu_types()
once the machine is created (its options being processed).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/virt.c | 59 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a3c9ffe29eb..c6ae7cc1705 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3130,36 +3130,41 @@ static int virt_hvf_get_physical_address_range(MachineState *ms)
     return requested_ipa_size;
 }
 
+static GSList *virt_get_valid_cpu_types(const MachineState *ms)
+{
+    GSList *vct = NULL;
+
+#ifdef CONFIG_TCG
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a7")));
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a15")));
+#ifdef TARGET_AARCH64
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a35")));
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a55")));
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a72")));
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a76")));
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a710")));
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("a64fx")));
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("neoverse-n1")));
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("neoverse-v1")));
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("neoverse-n2")));
+#endif /* TARGET_AARCH64 */
+#endif /* CONFIG_TCG */
+#ifdef TARGET_AARCH64
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a53")));
+        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a57")));
+#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
+            vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("host")));
+#endif /* CONFIG_KVM || CONFIG_HVF */
+#endif /* TARGET_AARCH64 */
+    vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("max")));
+
+    return vct;
+}
+
 static void virt_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
-    static const char * const valid_cpu_types[] = {
-#ifdef CONFIG_TCG
-        ARM_CPU_TYPE_NAME("cortex-a7"),
-        ARM_CPU_TYPE_NAME("cortex-a15"),
-#ifdef TARGET_AARCH64
-        ARM_CPU_TYPE_NAME("cortex-a35"),
-        ARM_CPU_TYPE_NAME("cortex-a55"),
-        ARM_CPU_TYPE_NAME("cortex-a72"),
-        ARM_CPU_TYPE_NAME("cortex-a76"),
-        ARM_CPU_TYPE_NAME("cortex-a710"),
-        ARM_CPU_TYPE_NAME("a64fx"),
-        ARM_CPU_TYPE_NAME("neoverse-n1"),
-        ARM_CPU_TYPE_NAME("neoverse-v1"),
-        ARM_CPU_TYPE_NAME("neoverse-n2"),
-#endif /* TARGET_AARCH64 */
-#endif /* CONFIG_TCG */
-#ifdef TARGET_AARCH64
-        ARM_CPU_TYPE_NAME("cortex-a53"),
-        ARM_CPU_TYPE_NAME("cortex-a57"),
-#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
-        ARM_CPU_TYPE_NAME("host"),
-#endif /* CONFIG_KVM || CONFIG_HVF */
-#endif /* TARGET_AARCH64 */
-        ARM_CPU_TYPE_NAME("max"),
-        NULL
-    };
 
     mc->init = machvirt_init;
     /* Start with max_cpus set to 512, which is the maximum supported by KVM.
@@ -3187,7 +3192,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
 #else
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("max");
 #endif
-    mc->valid_cpu_types = valid_cpu_types;
+    mc->get_valid_cpu_types = virt_get_valid_cpu_types;
     mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
     mc->kvm_type = virt_kvm_type;
     mc->hvf_get_physical_address_range = virt_hvf_get_physical_address_range;
-- 
2.47.1

On 4/22/25 07:54, Philippe Mathieu-Daudé wrote:
> +#ifdef CONFIG_TCG
> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a7")));
> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a15")));
> +#ifdef TARGET_AARCH64
> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a35")));
> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a55")));
> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a72")));
> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a76")));
> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a710")));
> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("a64fx")));
> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("neoverse-n1")));
> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("neoverse-v1")));
> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("neoverse-n2")));
> +#endif /* TARGET_AARCH64 */

Why do these need to be strdup'ed?

Do you anticipate other instances where these names cannot be constructed at compile-time?


r~

On 22/4/25 19:56, Richard Henderson wrote:
> On 4/22/25 07:54, Philippe Mathieu-Daudé wrote:
>> +#ifdef CONFIG_TCG
>> +        vct = g_slist_prepend(vct, 
>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a7")));
>> +        vct = g_slist_prepend(vct, 
>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a15")));
>> +#ifdef TARGET_AARCH64
>> +        vct = g_slist_prepend(vct, 
>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a35")));
>> +        vct = g_slist_prepend(vct, 
>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a55")));
>> +        vct = g_slist_prepend(vct, 
>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a72")));
>> +        vct = g_slist_prepend(vct, 
>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a76")));
>> +        vct = g_slist_prepend(vct, 
>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a710")));
>> +        vct = g_slist_prepend(vct, 
>> g_strdup(ARM_CPU_TYPE_NAME("a64fx")));
>> +        vct = g_slist_prepend(vct, 
>> g_strdup(ARM_CPU_TYPE_NAME("neoverse-n1")));
>> +        vct = g_slist_prepend(vct, 
>> g_strdup(ARM_CPU_TYPE_NAME("neoverse-v1")));
>> +        vct = g_slist_prepend(vct, 
>> g_strdup(ARM_CPU_TYPE_NAME("neoverse-n2")));
>> +#endif /* TARGET_AARCH64 */
> 
> Why do these need to be strdup'ed?

g_slist_prepend() expects non-const.

> 
> Do you anticipate other instances where these names cannot be 
> constructed at compile-time?
> 
> 
> r~

On 22/4/25 20:18, Philippe Mathieu-Daudé wrote:
> On 22/4/25 19:56, Richard Henderson wrote:
>> On 4/22/25 07:54, Philippe Mathieu-Daudé wrote:
>>> +#ifdef CONFIG_TCG
>>> +        vct = g_slist_prepend(vct, 
>>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a7")));
>>> +        vct = g_slist_prepend(vct, 
>>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a15")));
>>> +#ifdef TARGET_AARCH64
>>> +        vct = g_slist_prepend(vct, 
>>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a35")));
>>> +        vct = g_slist_prepend(vct, 
>>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a55")));
>>> +        vct = g_slist_prepend(vct, 
>>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a72")));
>>> +        vct = g_slist_prepend(vct, 
>>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a76")));
>>> +        vct = g_slist_prepend(vct, 
>>> g_strdup(ARM_CPU_TYPE_NAME("cortex-a710")));
>>> +        vct = g_slist_prepend(vct, 
>>> g_strdup(ARM_CPU_TYPE_NAME("a64fx")));
>>> +        vct = g_slist_prepend(vct, 
>>> g_strdup(ARM_CPU_TYPE_NAME("neoverse-n1")));
>>> +        vct = g_slist_prepend(vct, 
>>> g_strdup(ARM_CPU_TYPE_NAME("neoverse-v1")));
>>> +        vct = g_slist_prepend(vct, 
>>> g_strdup(ARM_CPU_TYPE_NAME("neoverse-n2")));
>>> +#endif /* TARGET_AARCH64 */
>>
>> Why do these need to be strdup'ed?
> 
> g_slist_prepend() expects non-const.
> 
>>
>> Do you anticipate other instances where these names cannot be 
>> constructed at compile-time?

In a few patches this become a run-time check:

   if (target_aarch64()) {
     ...
   }

I'll keep as it for now but am opened to simplify on a following up
patch.

On 4/22/25 10:56, Richard Henderson wrote:
> On 4/22/25 07:54, Philippe Mathieu-Daudé wrote:
>> +#ifdef CONFIG_TCG
>> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a7")));
>> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a15")));
>> +#ifdef TARGET_AARCH64
>> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a35")));
>> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a55")));
>> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a72")));
>> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a76")));
>> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("cortex-a710")));
>> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("a64fx")));
>> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("neoverse-n1")));
>> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("neoverse-v1")));
>> +        vct = g_slist_prepend(vct, g_strdup(ARM_CPU_TYPE_NAME("neoverse-n2")));
>> +#endif /* TARGET_AARCH64 */
> 
> Why do these need to be strdup'ed?
> 
> Do you anticipate other instances where these names cannot be constructed at compile-time?
> 

It seems it would be more easy if get_valid_cpu_types simply return a 
const* char* const (same as existing valid_cpu_types), and caller does 
not have the responsibility to free it.

This way, the list can be built either with a static array initializer, 
or with a dynamic GPtrArray, that we keep under a local static variable, 
so it has to be built only once. We can debate the leak introduced but I 
don't think it's really a problem.

> 
> r~