diff mbox series

[RFC,PATCH-for-10.1,35/39] hw/arm/virt: Replace TARGET_AARCH64 -> target_long_bits()

Message ID 20250403235821.9909-36-philmd@linaro.org
State New
Headers show
Series single-binary: Make hw/arm/ common | expand

Commit Message

Philippe Mathieu-Daudé April 3, 2025, 11:58 p.m. UTC
Replace the target-specific TARGET_AARCH64 definition
by a call to the generic target_long_bits() helper.

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

Comments

Pierrick Bouvier April 4, 2025, 6:28 p.m. UTC | #1
On 4/3/25 16:58, Philippe Mathieu-Daudé wrote:
> Replace the target-specific TARGET_AARCH64 definition
> by a call to the generic target_long_bits() helper.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/virt.c | 32 ++++++++++++++++----------------
>   1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index e241e71e1c3..a020f1bd581 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -3133,25 +3133,25 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>   #ifdef CONFIG_TCG
>       machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a7"));
>       machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a15"));
> -#ifdef TARGET_AARCH64
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a35"));
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a55"));
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a72"));
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a76"));
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a710"));
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("a64fx"));
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-n1"));
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-v1"));
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-n2"));
> -#endif /* TARGET_AARCH64 */
> +    if (target_long_bits() == 64) {

I would prefer if we introduce a true target_aarch64() function, and 
probably the same for other architectures when it will be needed.

If we start using target_long_bits(), we might enable something in 
common code that we are not supposed to do. And it will be much harder 
to find it later when we debug heterogenenous emulation.

> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a35"));
> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a55"));
> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a72"));
> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a76"));
> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a710"));
> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("a64fx"));
> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-n1"));
> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-v1"));
> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-n2"));
> +    }
>   #endif /* CONFIG_TCG */
> -#ifdef TARGET_AARCH64
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a53"));
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a57"));
> +    if (target_long_bits() == 64) {
> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a53"));
> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a57"));
>   #if defined(CONFIG_KVM) || defined(CONFIG_HVF)
> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("host"));
> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("host"));
>   #endif /* CONFIG_KVM || CONFIG_HVF */
> -#endif /* TARGET_AARCH64 */
> +    }
>       machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("max"));
>   
>       mc->init = machvirt_init;
Philippe Mathieu-Daudé April 4, 2025, 10:05 p.m. UTC | #2
On 4/4/25 20:28, Pierrick Bouvier wrote:
> On 4/3/25 16:58, Philippe Mathieu-Daudé wrote:
>> Replace the target-specific TARGET_AARCH64 definition
>> by a call to the generic target_long_bits() helper.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/arm/virt.c | 32 ++++++++++++++++----------------
>>   1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index e241e71e1c3..a020f1bd581 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -3133,25 +3133,25 @@ static void 
>> virt_machine_class_init(ObjectClass *oc, void *data)
>>   #ifdef CONFIG_TCG
>>       machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex- 
>> a7"));
>>       machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex- 
>> a15"));
>> -#ifdef TARGET_AARCH64
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex- 
>> a35"));
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex- 
>> a55"));
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex- 
>> a72"));
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex- 
>> a76"));
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex- 
>> a710"));
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("a64fx"));
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse- 
>> n1"));
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse- 
>> v1"));
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse- 
>> n2"));
>> -#endif /* TARGET_AARCH64 */
>> +    if (target_long_bits() == 64) {
> 
> I would prefer if we introduce a true target_aarch64() function, and 
> probably the same for other architectures when it will be needed.
> 
> If we start using target_long_bits(), we might enable something in 
> common code that we are not supposed to do. And it will be much harder 
> to find it later when we debug heterogenenous emulation.

I get your point. Maybe we can register valid aa64 CPUs regardless,
and filter for registered QOM CPUs? OTOH your suggestion of
TARGET_AARCH64 -> target_aarch64() could be easier to review.
I''ll give it a try.

> 
>> +        machine_class_add_valid_cpu_type(mc, 
>> ARM_CPU_TYPE_NAME("cortex-a35"));
>> +        machine_class_add_valid_cpu_type(mc, 
>> ARM_CPU_TYPE_NAME("cortex-a55"));
>> +        machine_class_add_valid_cpu_type(mc, 
>> ARM_CPU_TYPE_NAME("cortex-a72"));
>> +        machine_class_add_valid_cpu_type(mc, 
>> ARM_CPU_TYPE_NAME("cortex-a76"));
>> +        machine_class_add_valid_cpu_type(mc, 
>> ARM_CPU_TYPE_NAME("cortex-a710"));
>> +        machine_class_add_valid_cpu_type(mc, 
>> ARM_CPU_TYPE_NAME("a64fx"));
>> +        machine_class_add_valid_cpu_type(mc, 
>> ARM_CPU_TYPE_NAME("neoverse-n1"));
>> +        machine_class_add_valid_cpu_type(mc, 
>> ARM_CPU_TYPE_NAME("neoverse-v1"));
>> +        machine_class_add_valid_cpu_type(mc, 
>> ARM_CPU_TYPE_NAME("neoverse-n2"));
>> +    }
>>   #endif /* CONFIG_TCG */
>> -#ifdef TARGET_AARCH64
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex- 
>> a53"));
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex- 
>> a57"));
>> +    if (target_long_bits() == 64) {
>> +        machine_class_add_valid_cpu_type(mc, 
>> ARM_CPU_TYPE_NAME("cortex-a53"));
>> +        machine_class_add_valid_cpu_type(mc, 
>> ARM_CPU_TYPE_NAME("cortex-a57"));
>>   #if defined(CONFIG_KVM) || defined(CONFIG_HVF)
>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("host"));
>> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("host"));
>>   #endif /* CONFIG_KVM || CONFIG_HVF */
>> -#endif /* TARGET_AARCH64 */
>> +    }
>>       machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("max"));
>>       mc->init = machvirt_init;
>
Pierrick Bouvier April 5, 2025, 1:10 a.m. UTC | #3
On 4/4/25 15:05, Philippe Mathieu-Daudé wrote:
> On 4/4/25 20:28, Pierrick Bouvier wrote:
>> On 4/3/25 16:58, Philippe Mathieu-Daudé wrote:
>>> Replace the target-specific TARGET_AARCH64 definition
>>> by a call to the generic target_long_bits() helper.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    hw/arm/virt.c | 32 ++++++++++++++++----------------
>>>    1 file changed, 16 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index e241e71e1c3..a020f1bd581 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -3133,25 +3133,25 @@ static void
>>> virt_machine_class_init(ObjectClass *oc, void *data)
>>>    #ifdef CONFIG_TCG
>>>        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-
>>> a7"));
>>>        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-
>>> a15"));
>>> -#ifdef TARGET_AARCH64
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-
>>> a35"));
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-
>>> a55"));
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-
>>> a72"));
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-
>>> a76"));
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-
>>> a710"));
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("a64fx"));
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-
>>> n1"));
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-
>>> v1"));
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-
>>> n2"));
>>> -#endif /* TARGET_AARCH64 */
>>> +    if (target_long_bits() == 64) {
>>
>> I would prefer if we introduce a true target_aarch64() function, and
>> probably the same for other architectures when it will be needed.
>>
>> If we start using target_long_bits(), we might enable something in
>> common code that we are not supposed to do. And it will be much harder
>> to find it later when we debug heterogenenous emulation.
> 
> I get your point. Maybe we can register valid aa64 CPUs regardless,
> and filter for registered QOM CPUs? OTOH your suggestion of
> TARGET_AARCH64 -> target_aarch64() could be easier to review.
> I''ll give it a try.
> 

I think it's better to customize the list creation directly, instead of 
adding another hook to filter things afterward.
As well, it's obvious when reading the code, the same way current ifdef 
are obvious when reading them.

>>
>>> +        machine_class_add_valid_cpu_type(mc,
>>> ARM_CPU_TYPE_NAME("cortex-a35"));
>>> +        machine_class_add_valid_cpu_type(mc,
>>> ARM_CPU_TYPE_NAME("cortex-a55"));
>>> +        machine_class_add_valid_cpu_type(mc,
>>> ARM_CPU_TYPE_NAME("cortex-a72"));
>>> +        machine_class_add_valid_cpu_type(mc,
>>> ARM_CPU_TYPE_NAME("cortex-a76"));
>>> +        machine_class_add_valid_cpu_type(mc,
>>> ARM_CPU_TYPE_NAME("cortex-a710"));
>>> +        machine_class_add_valid_cpu_type(mc,
>>> ARM_CPU_TYPE_NAME("a64fx"));
>>> +        machine_class_add_valid_cpu_type(mc,
>>> ARM_CPU_TYPE_NAME("neoverse-n1"));
>>> +        machine_class_add_valid_cpu_type(mc,
>>> ARM_CPU_TYPE_NAME("neoverse-v1"));
>>> +        machine_class_add_valid_cpu_type(mc,
>>> ARM_CPU_TYPE_NAME("neoverse-n2"));
>>> +    }
>>>    #endif /* CONFIG_TCG */
>>> -#ifdef TARGET_AARCH64
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-
>>> a53"));
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-
>>> a57"));
>>> +    if (target_long_bits() == 64) {
>>> +        machine_class_add_valid_cpu_type(mc,
>>> ARM_CPU_TYPE_NAME("cortex-a53"));
>>> +        machine_class_add_valid_cpu_type(mc,
>>> ARM_CPU_TYPE_NAME("cortex-a57"));
>>>    #if defined(CONFIG_KVM) || defined(CONFIG_HVF)
>>> -    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("host"));
>>> +        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("host"));
>>>    #endif /* CONFIG_KVM || CONFIG_HVF */
>>> -#endif /* TARGET_AARCH64 */
>>> +    }
>>>        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("max"));
>>>        mc->init = machvirt_init;
>>
>
diff mbox series

Patch

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e241e71e1c3..a020f1bd581 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3133,25 +3133,25 @@  static void virt_machine_class_init(ObjectClass *oc, void *data)
 #ifdef CONFIG_TCG
     machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a7"));
     machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a15"));
-#ifdef TARGET_AARCH64
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a35"));
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a55"));
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a72"));
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a76"));
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a710"));
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("a64fx"));
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-n1"));
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-v1"));
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-n2"));
-#endif /* TARGET_AARCH64 */
+    if (target_long_bits() == 64) {
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a35"));
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a55"));
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a72"));
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a76"));
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a710"));
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("a64fx"));
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-n1"));
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-v1"));
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("neoverse-n2"));
+    }
 #endif /* CONFIG_TCG */
-#ifdef TARGET_AARCH64
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a53"));
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a57"));
+    if (target_long_bits() == 64) {
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a53"));
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("cortex-a57"));
 #if defined(CONFIG_KVM) || defined(CONFIG_HVF)
-    machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("host"));
+        machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("host"));
 #endif /* CONFIG_KVM || CONFIG_HVF */
-#endif /* TARGET_AARCH64 */
+    }
     machine_class_add_valid_cpu_type(mc, ARM_CPU_TYPE_NAME("max"));
 
     mc->init = machvirt_init;