diff mbox series

[RFC,3/4] hw/arm/aspeed: Replace TARGET_AARCH64 by legacy_binary_is_64bit()

Message ID 20250305161248.54901-4-philmd@linaro.org
State New
Headers show
Series hw/arm: Register target-specific QOM types at runtime | expand

Commit Message

Philippe Mathieu-Daudé March 5, 2025, 4:12 p.m. UTC
For legacy ARM binaries, legacy_binary_is_64bit() is
equivalent of the compile time TARGET_AARCH64 definition.

Use it as TypeInfo::registerable() callback to dynamically
add Aarch64 specific types in qemu-system-aarch64 binary,
removing the need of TARGET_AARCH64 #ifdef'ry.

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

Comments

Cédric Le Goater March 5, 2025, 4:33 p.m. UTC | #1
On 3/5/25 17:12, Philippe Mathieu-Daudé wrote:
> For legacy ARM binaries, legacy_binary_is_64bit() is
> equivalent of the compile time TARGET_AARCH64 definition.
> 
> Use it as TypeInfo::registerable() callback to dynamically
> add Aarch64 specific types in qemu-system-aarch64 binary,
> removing the need of TARGET_AARCH64 #ifdef'ry.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/aspeed.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 98bf071139b..3f18a4501e0 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -28,6 +28,7 @@
>   #include "hw/loader.h"
>   #include "qemu/error-report.h"
>   #include "qemu/units.h"
> +#include "qemu/legacy_binary_info.h"
>   #include "hw/qdev-clock.h"
>   #include "system/system.h"
>   
> @@ -179,11 +180,9 @@ struct AspeedMachineState {
>   #define AST2600_EVB_HW_STRAP1 0x000000C0
>   #define AST2600_EVB_HW_STRAP2 0x00000003
>   
> -#ifdef TARGET_AARCH64
>   /* AST2700 evb hardware value */
>   #define AST2700_EVB_HW_STRAP1 0x000000C0
>   #define AST2700_EVB_HW_STRAP2 0x00000003
> -#endif
>   
>   /* Rainier hardware value: (QEMU prototype) */
>   #define RAINIER_BMC_HW_STRAP1 (0x00422016 | SCU_AST2600_HW_STRAP_BOOT_SRC_EMMC)
> @@ -1661,7 +1660,6 @@ static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
>       aspeed_machine_class_init_cpus_defaults(mc);
>   }
>   
> -#ifdef TARGET_AARCH64
>   static void ast2700_evb_i2c_init(AspeedMachineState *bmc)
>   {
>       AspeedSoCState *soc = bmc->soc;
> @@ -1690,7 +1688,6 @@ static void aspeed_machine_ast2700_evb_class_init(ObjectClass *oc, void *data)
>       mc->default_ram_size = 1 * GiB;
>       aspeed_machine_class_init_cpus_defaults(mc);
>   }
> -#endif
>   
>   static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>                                                        void *data)
> @@ -1813,12 +1810,11 @@ static const TypeInfo aspeed_machine_types[] = {
>           .name           = MACHINE_TYPE_NAME("ast1030-evb"),
>           .parent         = TYPE_ASPEED_MACHINE,
>           .class_init     = aspeed_minibmc_machine_ast1030_evb_class_init,
> -#ifdef TARGET_AARCH64
>       }, {
>           .name          = MACHINE_TYPE_NAME("ast2700-evb"),
>           .parent        = TYPE_ASPEED_MACHINE,
> +        .registerable   = legacy_binary_is_64bit,

where is this routine implemented ?

Thanks,

C.


>           .class_init    = aspeed_machine_ast2700_evb_class_init,
> -#endif
>       }, {
>           .name          = TYPE_ASPEED_MACHINE,
>           .parent        = TYPE_MACHINE,
Philippe Mathieu-Daudé March 5, 2025, 5:07 p.m. UTC | #2
On 5/3/25 17:33, Cédric Le Goater wrote:
> On 3/5/25 17:12, Philippe Mathieu-Daudé wrote:
>> For legacy ARM binaries, legacy_binary_is_64bit() is
>> equivalent of the compile time TARGET_AARCH64 definition.
>>
>> Use it as TypeInfo::registerable() callback to dynamically
>> add Aarch64 specific types in qemu-system-aarch64 binary,
>> removing the need of TARGET_AARCH64 #ifdef'ry.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/arm/aspeed.c | 8 ++------
>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index 98bf071139b..3f18a4501e0 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -28,6 +28,7 @@
>>   #include "hw/loader.h"
>>   #include "qemu/error-report.h"
>>   #include "qemu/units.h"
>> +#include "qemu/legacy_binary_info.h"
>>   #include "hw/qdev-clock.h"
>>   #include "system/system.h"
>> @@ -179,11 +180,9 @@ struct AspeedMachineState {
>>   #define AST2600_EVB_HW_STRAP1 0x000000C0
>>   #define AST2600_EVB_HW_STRAP2 0x00000003
>> -#ifdef TARGET_AARCH64
>>   /* AST2700 evb hardware value */
>>   #define AST2700_EVB_HW_STRAP1 0x000000C0
>>   #define AST2700_EVB_HW_STRAP2 0x00000003
>> -#endif
>>   /* Rainier hardware value: (QEMU prototype) */
>>   #define RAINIER_BMC_HW_STRAP1 (0x00422016 | 
>> SCU_AST2600_HW_STRAP_BOOT_SRC_EMMC)
>> @@ -1661,7 +1660,6 @@ static void 
>> aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
>>       aspeed_machine_class_init_cpus_defaults(mc);
>>   }
>> -#ifdef TARGET_AARCH64
>>   static void ast2700_evb_i2c_init(AspeedMachineState *bmc)
>>   {
>>       AspeedSoCState *soc = bmc->soc;
>> @@ -1690,7 +1688,6 @@ static void 
>> aspeed_machine_ast2700_evb_class_init(ObjectClass *oc, void *data)
>>       mc->default_ram_size = 1 * GiB;
>>       aspeed_machine_class_init_cpus_defaults(mc);
>>   }
>> -#endif
>>   static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>>                                                        void *data)
>> @@ -1813,12 +1810,11 @@ static const TypeInfo aspeed_machine_types[] = {
>>           .name           = MACHINE_TYPE_NAME("ast1030-evb"),
>>           .parent         = TYPE_ASPEED_MACHINE,
>>           .class_init     = 
>> aspeed_minibmc_machine_ast1030_evb_class_init,
>> -#ifdef TARGET_AARCH64
>>       }, {
>>           .name          = MACHINE_TYPE_NAME("ast2700-evb"),
>>           .parent        = TYPE_ASPEED_MACHINE,
>> +        .registerable   = legacy_binary_is_64bit,
> 
> where is this routine implemented ?

Based-on series mentioned in cover:
https://lore.kernel.org/qemu-devel/20250305153929.43687-6-philmd@linaro.org/
Thomas Huth March 5, 2025, 5:43 p.m. UTC | #3
On 05/03/2025 17.12, Philippe Mathieu-Daudé wrote:
> For legacy ARM binaries, legacy_binary_is_64bit() is
> equivalent of the compile time TARGET_AARCH64 definition.
> 
> Use it as TypeInfo::registerable() callback to dynamically
> add Aarch64 specific types in qemu-system-aarch64 binary,
> removing the need of TARGET_AARCH64 #ifdef'ry.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/aspeed.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 98bf071139b..3f18a4501e0 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -28,6 +28,7 @@
>   #include "hw/loader.h"
>   #include "qemu/error-report.h"
>   #include "qemu/units.h"
> +#include "qemu/legacy_binary_info.h"
>   #include "hw/qdev-clock.h"
>   #include "system/system.h"
>   
> @@ -179,11 +180,9 @@ struct AspeedMachineState {
>   #define AST2600_EVB_HW_STRAP1 0x000000C0
>   #define AST2600_EVB_HW_STRAP2 0x00000003
>   
> -#ifdef TARGET_AARCH64
>   /* AST2700 evb hardware value */
>   #define AST2700_EVB_HW_STRAP1 0x000000C0
>   #define AST2700_EVB_HW_STRAP2 0x00000003
> -#endif
>   
>   /* Rainier hardware value: (QEMU prototype) */
>   #define RAINIER_BMC_HW_STRAP1 (0x00422016 | SCU_AST2600_HW_STRAP_BOOT_SRC_EMMC)
> @@ -1661,7 +1660,6 @@ static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
>       aspeed_machine_class_init_cpus_defaults(mc);
>   }
>   
> -#ifdef TARGET_AARCH64
>   static void ast2700_evb_i2c_init(AspeedMachineState *bmc)
>   {
>       AspeedSoCState *soc = bmc->soc;
> @@ -1690,7 +1688,6 @@ static void aspeed_machine_ast2700_evb_class_init(ObjectClass *oc, void *data)
>       mc->default_ram_size = 1 * GiB;
>       aspeed_machine_class_init_cpus_defaults(mc);
>   }
> -#endif
>   
>   static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
>                                                        void *data)
> @@ -1813,12 +1810,11 @@ static const TypeInfo aspeed_machine_types[] = {
>           .name           = MACHINE_TYPE_NAME("ast1030-evb"),
>           .parent         = TYPE_ASPEED_MACHINE,
>           .class_init     = aspeed_minibmc_machine_ast1030_evb_class_init,
> -#ifdef TARGET_AARCH64
>       }, {
>           .name          = MACHINE_TYPE_NAME("ast2700-evb"),
>           .parent        = TYPE_ASPEED_MACHINE,
> +        .registerable   = legacy_binary_is_64bit,
>           .class_init    = aspeed_machine_ast2700_evb_class_init,
> -#endif
>       }, {
>           .name          = TYPE_ASPEED_MACHINE,
>           .parent        = TYPE_MACHINE,

Same as with previous patch, I'd prefer to split the array, and replace the 
DEFINE_TYPES() with a dynamic type_init() function.

  Thomas
diff mbox series

Patch

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 98bf071139b..3f18a4501e0 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -28,6 +28,7 @@ 
 #include "hw/loader.h"
 #include "qemu/error-report.h"
 #include "qemu/units.h"
+#include "qemu/legacy_binary_info.h"
 #include "hw/qdev-clock.h"
 #include "system/system.h"
 
@@ -179,11 +180,9 @@  struct AspeedMachineState {
 #define AST2600_EVB_HW_STRAP1 0x000000C0
 #define AST2600_EVB_HW_STRAP2 0x00000003
 
-#ifdef TARGET_AARCH64
 /* AST2700 evb hardware value */
 #define AST2700_EVB_HW_STRAP1 0x000000C0
 #define AST2700_EVB_HW_STRAP2 0x00000003
-#endif
 
 /* Rainier hardware value: (QEMU prototype) */
 #define RAINIER_BMC_HW_STRAP1 (0x00422016 | SCU_AST2600_HW_STRAP_BOOT_SRC_EMMC)
@@ -1661,7 +1660,6 @@  static void aspeed_minibmc_machine_ast1030_evb_class_init(ObjectClass *oc,
     aspeed_machine_class_init_cpus_defaults(mc);
 }
 
-#ifdef TARGET_AARCH64
 static void ast2700_evb_i2c_init(AspeedMachineState *bmc)
 {
     AspeedSoCState *soc = bmc->soc;
@@ -1690,7 +1688,6 @@  static void aspeed_machine_ast2700_evb_class_init(ObjectClass *oc, void *data)
     mc->default_ram_size = 1 * GiB;
     aspeed_machine_class_init_cpus_defaults(mc);
 }
-#endif
 
 static void aspeed_machine_qcom_dc_scm_v1_class_init(ObjectClass *oc,
                                                      void *data)
@@ -1813,12 +1810,11 @@  static const TypeInfo aspeed_machine_types[] = {
         .name           = MACHINE_TYPE_NAME("ast1030-evb"),
         .parent         = TYPE_ASPEED_MACHINE,
         .class_init     = aspeed_minibmc_machine_ast1030_evb_class_init,
-#ifdef TARGET_AARCH64
     }, {
         .name          = MACHINE_TYPE_NAME("ast2700-evb"),
         .parent        = TYPE_ASPEED_MACHINE,
+        .registerable   = legacy_binary_is_64bit,
         .class_init    = aspeed_machine_ast2700_evb_class_init,
-#endif
     }, {
         .name          = TYPE_ASPEED_MACHINE,
         .parent        = TYPE_MACHINE,