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;
>>
>