Re: [PATCH 1/4] target/arm: Make cpu_register() and set_feature() available for other files

From: Auger Eric <eric.auger@redhat.com>
To: Thomas Huth <thuth@redhat.com>,
	qemu-devel@nongnu.org, Peter Maydell <peter.maydell@linaro.org>
Cc: "Richard Henderson" <richard.henderson@linaro.org>,
	qemu-arm@nongnu.org, "Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH 1/4] target/arm: Make cpu_register() and set_feature() available for other files
Date: Mon, 23 Sep 2019 16:28:42 +0200	[thread overview]
Message-ID: <9457e42b-8550-561a-d2d0-7dce076af200@redhat.com> (raw)
In-Reply-To: <20190921150420.30743-2-thuth@redhat.com>

Hi Thomas,

On 9/21/19 5:04 PM, Thomas Huth wrote:
> Move the common set_feature() and unset_feature() functions from cpu.c and
> cpu64.c to internals.h, and make cpu_register() (renamed to arm_cpu_register())
> available from there, too, so we can register CPUs also from other files
> in the future.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  target/arm/cpu.c       | 20 ++------------------
>  target/arm/cpu64.c     | 17 +----------------
>  target/arm/internals.h | 18 ++++++++++++++++++
>  3 files changed, 21 insertions(+), 34 deletions(-)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 2399c14471..f1f9eecdc8 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -614,16 +614,6 @@ static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
>  
>  #endif
>  
> -static inline void set_feature(CPUARMState *env, int feature)
> -{
> -    env->features |= 1ULL << feature;
> -}
> -
> -static inline void unset_feature(CPUARMState *env, int feature)
> -{
> -    env->features &= ~(1ULL << feature);
> -}
> -
>  static int
>  print_insn_thumb1(bfd_vma pc, disassemble_info *info)
>  {
> @@ -2515,12 +2505,6 @@ static void arm_max_initfn(Object *obj)
>  
>  #endif /* !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64) */
>  
> -struct ARMCPUInfo {
> -    const char *name;
> -    void (*initfn)(Object *obj);
> -    void (*class_init)(ObjectClass *oc, void *data);
> -};
> -
>  static const ARMCPUInfo arm_cpus[] = {
>  #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
>      { .name = "arm926",      .initfn = arm926_initfn },
> @@ -2681,7 +2665,7 @@ static void cpu_register_class_init(ObjectClass *oc, void *data)
>      acc->info = data;
>  }
>  
> -static void cpu_register(const ARMCPUInfo *info)
> +void arm_cpu_register(const ARMCPUInfo *info)
>  {
>      TypeInfo type_info = {
>          .parent = TYPE_ARM_CPU,
> @@ -2722,7 +2706,7 @@ static void arm_cpu_register_types(void)
>      type_register_static(&idau_interface_type_info);
>  
>      while (info->name) {
> -        cpu_register(info);
> +        arm_cpu_register(info);
>          info++;
>      }
>  
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index d7f5bf610a..5239ba5529 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -21,6 +21,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "cpu.h"
> +#include "internals.h"
>  #include "qemu/module.h"
>  #if !defined(CONFIG_USER_ONLY)
>  #include "hw/loader.h"
> @@ -29,16 +30,6 @@
>  #include "kvm_arm.h"
>  #include "qapi/visitor.h"
>  
> -static inline void set_feature(CPUARMState *env, int feature)
> -{
> -    env->features |= 1ULL << feature;
> -}
> -
> -static inline void unset_feature(CPUARMState *env, int feature)
> -{
> -    env->features &= ~(1ULL << feature);
> -}
> -
>  #ifndef CONFIG_USER_ONLY
>  static uint64_t a57_a53_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
> @@ -396,12 +387,6 @@ static void aarch64_max_initfn(Object *obj)
>      }
>  }
>  
> -struct ARMCPUInfo {
> -    const char *name;
> -    void (*initfn)(Object *obj);
> -    void (*class_init)(ObjectClass *oc, void *data);
> -};
> -
>  static const ARMCPUInfo aarch64_cpus[] = {
>      { .name = "cortex-a57",         .initfn = aarch64_a57_initfn },
>      { .name = "cortex-a53",         .initfn = aarch64_a53_initfn },
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 232d963875..e71196ed5f 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1046,4 +1046,22 @@ void arm_log_exception(int idx);
>  
>  #endif /* !CONFIG_USER_ONLY */
>  
> +static inline void set_feature(CPUARMState *env, int feature)
> +{
> +    env->features |= 1ULL << feature;
> +}
> +
> +static inline void unset_feature(CPUARMState *env, int feature)
> +{
> +    env->features &= ~(1ULL << feature);
> +}
> +
> +struct ARMCPUInfo {
> +    const char *name;
> +    void (*initfn)(Object *obj);
> +    void (*class_init)(ObjectClass *oc, void *data);
> +};
> +
> +void arm_cpu_register(const ARMCPUInfo *info);
> +
>  #endif
> 