diff mbox series

[118/147] target/arm/cpu: remove inline stubs for aarch32 emulation

Message ID 20250422192819.302784-119-richard.henderson@linaro.org
State Superseded
Headers show
Series single-binary patch queue | expand

Commit Message

Richard Henderson April 22, 2025, 7:27 p.m. UTC 
From: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Directly condition associated calls in target/arm/helper.c for now.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20250325045915.994760-23-pierrick.bouvier@linaro.org>
---
 target/arm/cpu.h    | 8 --------
 target/arm/helper.c | 6 ++++++
 2 files changed, 6 insertions(+), 8 deletions(-)

Comments

Philippe Mathieu-Daudé April 23, 2025, 10:35 a.m. UTC | #1 
On 22/4/25 21:27, Richard Henderson wrote:
> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> Directly condition associated calls in target/arm/helper.c for now.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> Message-ID: <20250325045915.994760-23-pierrick.bouvier@linaro.org>
> ---
>   target/arm/cpu.h    | 8 --------
>   target/arm/helper.c | 6 ++++++
>   2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index b1c3e46326..c1a0faed3a 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1222,7 +1222,6 @@ int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
>    */
>   void arm_emulate_firmware_reset(CPUState *cpustate, int target_el);
>   
> -#ifdef TARGET_AARCH64
>   int aarch64_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
>   int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>   void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
> @@ -1254,13 +1253,6 @@ static inline uint64_t *sve_bswap64(uint64_t *dst, uint64_t *src, int nr)
>   #endif
>   }
>   
> -#else
> -static inline void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq) { }
> -static inline void aarch64_sve_change_el(CPUARMState *env, int o,
> -                                         int n, bool a)
> -{ }
> -#endif
> -
>   void aarch64_sync_32_to_64(CPUARMState *env);
>   void aarch64_sync_64_to_32(CPUARMState *env);
>   

Should we complete squashing:

-- >8 --
diff --git a/target/arm/internals.h b/target/arm/internals.h
index cf4ab17bc08..f9353887415 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1810,7 +1810,6 @@ static inline uint64_t 
pmu_counter_mask(CPUARMState *env)
    return (1ULL << 31) | ((1ULL << pmu_num_counters(env)) - 1);
  }

-#ifdef TARGET_AARCH64
  GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu, int base_reg);
  int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg);
  int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg);
@@ -1820,7 +1819,6 @@ int aarch64_gdb_get_pauth_reg(CPUState *cs, 
GByteArray *buf, int reg);
  int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg);
  int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, GByteArray *buf, int reg);
  int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg);
-#endif
  void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
  void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
  void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
---

?

> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index becbbbd0d8..7fb6e88630 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6563,7 +6563,9 @@ static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>        */
>       new_len = sve_vqm1_for_el(env, cur_el);
>       if (new_len < old_len) {
> +#ifdef TARGET_AARCH64
>           aarch64_sve_narrow_vq(env, new_len + 1);
> +#endif
>       }
>   }
>   
> @@ -10628,7 +10630,9 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>            * Note that new_el can never be 0.  If cur_el is 0, then
>            * el0_a64 is is_a64(), else el0_a64 is ignored.
>            */
> +#ifdef TARGET_AARCH64
>           aarch64_sve_change_el(env, cur_el, new_el, is_a64(env));
> +#endif
>       }
>   
>       if (cur_el < new_el) {
> @@ -11640,7 +11644,9 @@ void aarch64_sve_change_el(CPUARMState *env, int old_el,
>   
>       /* When changing vector length, clear inaccessible state.  */
>       if (new_len < old_len) {
> +#ifdef TARGET_AARCH64
>           aarch64_sve_narrow_vq(env, new_len + 1);
> +#endif
>       }
>   }
>   #endif
Pierrick Bouvier April 23, 2025, 4:26 p.m. UTC | #2 
On 4/23/25 03:35, Philippe Mathieu-Daudé wrote:
> On 22/4/25 21:27, Richard Henderson wrote:
>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>> Directly condition associated calls in target/arm/helper.c for now.
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> Message-ID: <20250325045915.994760-23-pierrick.bouvier@linaro.org>
>> ---
>>    target/arm/cpu.h    | 8 --------
>>    target/arm/helper.c | 6 ++++++
>>    2 files changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index b1c3e46326..c1a0faed3a 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -1222,7 +1222,6 @@ int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
>>     */
>>    void arm_emulate_firmware_reset(CPUState *cpustate, int target_el);
>>    
>> -#ifdef TARGET_AARCH64
>>    int aarch64_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
>>    int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>>    void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
>> @@ -1254,13 +1253,6 @@ static inline uint64_t *sve_bswap64(uint64_t *dst, uint64_t *src, int nr)
>>    #endif
>>    }
>>    
>> -#else
>> -static inline void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq) { }
>> -static inline void aarch64_sve_change_el(CPUARMState *env, int o,
>> -                                         int n, bool a)
>> -{ }
>> -#endif
>> -
>>    void aarch64_sync_32_to_64(CPUARMState *env);
>>    void aarch64_sync_64_to_32(CPUARMState *env);
>>    
> 
> Should we complete squashing:
> 
> -- >8 --
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index cf4ab17bc08..f9353887415 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1810,7 +1810,6 @@ static inline uint64_t
> pmu_counter_mask(CPUARMState *env)
>      return (1ULL << 31) | ((1ULL << pmu_num_counters(env)) - 1);
>    }
> 
> -#ifdef TARGET_AARCH64
>    GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu, int base_reg);
>    int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg);
>    int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg);
> @@ -1820,7 +1819,6 @@ int aarch64_gdb_get_pauth_reg(CPUState *cs,
> GByteArray *buf, int reg);
>    int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg);
>    int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, GByteArray *buf, int reg);
>    int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg);
> -#endif
>    void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
>    void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
>    void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
> ---
> 
> ?
> 

This part of the series focus on hw/arm, so it was not needed to clean 
target/arm/internals.h as part of it.
That's why I "pushed" the TARGET_AARCH64 #ifdef to target/arm/helper.c, 
allowing to do it later.
I tried to cleanup that completely at the time, as requested by Peter, 
but it was pulling too many things, so I just dropped it.

So I think we should not squash it here.

>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index becbbbd0d8..7fb6e88630 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -6563,7 +6563,9 @@ static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>>         */
>>        new_len = sve_vqm1_for_el(env, cur_el);
>>        if (new_len < old_len) {
>> +#ifdef TARGET_AARCH64
>>            aarch64_sve_narrow_vq(env, new_len + 1);
>> +#endif
>>        }
>>    }
>>    
>> @@ -10628,7 +10630,9 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>>             * Note that new_el can never be 0.  If cur_el is 0, then
>>             * el0_a64 is is_a64(), else el0_a64 is ignored.
>>             */
>> +#ifdef TARGET_AARCH64
>>            aarch64_sve_change_el(env, cur_el, new_el, is_a64(env));
>> +#endif
>>        }
>>    
>>        if (cur_el < new_el) {
>> @@ -11640,7 +11644,9 @@ void aarch64_sve_change_el(CPUARMState *env, int old_el,
>>    
>>        /* When changing vector length, clear inaccessible state.  */
>>        if (new_len < old_len) {
>> +#ifdef TARGET_AARCH64
>>            aarch64_sve_narrow_vq(env, new_len + 1);
>> +#endif
>>        }
>>    }
>>    #endif
>
Philippe Mathieu-Daudé April 23, 2025, 4:38 p.m. UTC | #3 
On 23/4/25 18:26, Pierrick Bouvier wrote:
> On 4/23/25 03:35, Philippe Mathieu-Daudé wrote:
>> On 22/4/25 21:27, Richard Henderson wrote:
>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>
>>> Directly condition associated calls in target/arm/helper.c for now.
>>>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> Message-ID: <20250325045915.994760-23-pierrick.bouvier@linaro.org>
>>> ---
>>>    target/arm/cpu.h    | 8 --------
>>>    target/arm/helper.c | 6 ++++++
>>>    2 files changed, 6 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>>> index b1c3e46326..c1a0faed3a 100644
>>> --- a/target/arm/cpu.h
>>> +++ b/target/arm/cpu.h
>>> @@ -1222,7 +1222,6 @@ int 
>>> arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
>>>     */
>>>    void arm_emulate_firmware_reset(CPUState *cpustate, int target_el);
>>> -#ifdef TARGET_AARCH64
>>>    int aarch64_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, 
>>> int reg);
>>>    int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, 
>>> int reg);
>>>    void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
>>> @@ -1254,13 +1253,6 @@ static inline uint64_t *sve_bswap64(uint64_t 
>>> *dst, uint64_t *src, int nr)
>>>    #endif
>>>    }
>>> -#else
>>> -static inline void aarch64_sve_narrow_vq(CPUARMState *env, unsigned 
>>> vq) { }
>>> -static inline void aarch64_sve_change_el(CPUARMState *env, int o,
>>> -                                         int n, bool a)
>>> -{ }
>>> -#endif
>>> -
>>>    void aarch64_sync_32_to_64(CPUARMState *env);
>>>    void aarch64_sync_64_to_32(CPUARMState *env);
>>
>> Should we complete squashing:
>>
>> -- >8 --
>> diff --git a/target/arm/internals.h b/target/arm/internals.h
>> index cf4ab17bc08..f9353887415 100644
>> --- a/target/arm/internals.h
>> +++ b/target/arm/internals.h
>> @@ -1810,7 +1810,6 @@ static inline uint64_t
>> pmu_counter_mask(CPUARMState *env)
>>      return (1ULL << 31) | ((1ULL << pmu_num_counters(env)) - 1);
>>    }
>>
>> -#ifdef TARGET_AARCH64
>>    GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu, int 
>> base_reg);
>>    int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg);
>>    int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg);
>> @@ -1820,7 +1819,6 @@ int aarch64_gdb_get_pauth_reg(CPUState *cs,
>> GByteArray *buf, int reg);
>>    int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg);
>>    int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, GByteArray *buf, int 
>> reg);
>>    int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg);
>> -#endif
>>    void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
>>    void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
>>    void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
>> ---
>>
>> ?
>>
> 
> This part of the series focus on hw/arm, so it was not needed to clean 
> target/arm/internals.h as part of it.
> That's why I "pushed" the TARGET_AARCH64 #ifdef to target/arm/helper.c, 
> allowing to do it later.
> I tried to cleanup that completely at the time, as requested by Peter, 
> but it was pulling too many things, so I just dropped it.
> 
> So I think we should not squash it here.

OK, then this patch can be queued on top:
https://lore.kernel.org/qemu-devel/20250403235821.9909-37-philmd@linaro.org/
Richard Henderson April 23, 2025, 9:23 p.m. UTC | #4 
On 4/23/25 09:38, Philippe Mathieu-Daudé wrote:
>> So I think we should not squash it here.
> 
> OK, then this patch can be queued on top:
> https://lore.kernel.org/qemu-devel/20250403235821.9909-37-philmd@linaro.org/

Ok, I queued that immediately afterward.
Incidentally "unconditionally" not "indistinctly"; fixed locally.


r~
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index b1c3e46326..c1a0faed3a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1222,7 +1222,6 @@  int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
  */
 void arm_emulate_firmware_reset(CPUState *cpustate, int target_el);
 
-#ifdef TARGET_AARCH64
 int aarch64_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
@@ -1254,13 +1253,6 @@  static inline uint64_t *sve_bswap64(uint64_t *dst, uint64_t *src, int nr)
 #endif
 }
 
-#else
-static inline void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq) { }
-static inline void aarch64_sve_change_el(CPUARMState *env, int o,
-                                         int n, bool a)
-{ }
-#endif
-
 void aarch64_sync_32_to_64(CPUARMState *env);
 void aarch64_sync_64_to_32(CPUARMState *env);
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index becbbbd0d8..7fb6e88630 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6563,7 +6563,9 @@  static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
      */
     new_len = sve_vqm1_for_el(env, cur_el);
     if (new_len < old_len) {
+#ifdef TARGET_AARCH64
         aarch64_sve_narrow_vq(env, new_len + 1);
+#endif
     }
 }
 
@@ -10628,7 +10630,9 @@  static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
          * Note that new_el can never be 0.  If cur_el is 0, then
          * el0_a64 is is_a64(), else el0_a64 is ignored.
          */
+#ifdef TARGET_AARCH64
         aarch64_sve_change_el(env, cur_el, new_el, is_a64(env));
+#endif
     }
 
     if (cur_el < new_el) {
@@ -11640,7 +11644,9 @@  void aarch64_sve_change_el(CPUARMState *env, int old_el,
 
     /* When changing vector length, clear inaccessible state.  */
     if (new_len < old_len) {
+#ifdef TARGET_AARCH64
         aarch64_sve_narrow_vq(env, new_len + 1);
+#endif
     }
 }
 #endif