target/arm/internals: Replace some target_ulong uses

[RFC PATCH 2/3] target/arm: Replace target_ulong -> uint64_t for HWBreakpoint

Posted by Philippe Mathieu-Daudé 8 months, 1 week ago

CPUARMState::pc is of type uint64_t.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/internals.h   | 6 +++---
 target/arm/hyp_gdbstub.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 175fb792375..05ceb9bcf5d 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1942,9 +1942,9 @@ extern GArray *hw_breakpoints, *hw_watchpoints;
 #define get_hw_bp(i)    (&g_array_index(hw_breakpoints, HWBreakpoint, i))
 #define get_hw_wp(i)    (&g_array_index(hw_watchpoints, HWWatchpoint, i))
 
-bool find_hw_breakpoint(CPUState *cpu, target_ulong pc);
-int insert_hw_breakpoint(target_ulong pc);
-int delete_hw_breakpoint(target_ulong pc);
+bool find_hw_breakpoint(CPUState *cpu, uint64_t pc);
+int insert_hw_breakpoint(uint64_t pc);
+int delete_hw_breakpoint(uint64_t pc);
 
 bool check_watchpoint_in_range(int i, target_ulong addr);
 CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr);
diff --git a/target/arm/hyp_gdbstub.c b/target/arm/hyp_gdbstub.c
index 1e861263b3d..4e52d37ed80 100644
--- a/target/arm/hyp_gdbstub.c
+++ b/target/arm/hyp_gdbstub.c
@@ -54,7 +54,7 @@ GArray *hw_breakpoints, *hw_watchpoints;
  * here so future PC comparisons will work properly.
  */
 
-int insert_hw_breakpoint(target_ulong addr)
+int insert_hw_breakpoint(uint64_t addr)
 {
     HWBreakpoint brk = {
         .bcr = 0x1,                             /* BCR E=1, enable */
@@ -80,7 +80,7 @@ int insert_hw_breakpoint(target_ulong addr)
  * Delete a breakpoint and shuffle any above down
  */
 
-int delete_hw_breakpoint(target_ulong pc)
+int delete_hw_breakpoint(uint64_t pc)
 {
     int i;
     for (i = 0; i < hw_breakpoints->len; i++) {
@@ -226,7 +226,7 @@ int delete_hw_watchpoint(target_ulong addr, target_ulong len, int type)
     return -ENOENT;
 }
 
-bool find_hw_breakpoint(CPUState *cpu, target_ulong pc)
+bool find_hw_breakpoint(CPUState *cpu, uint64_t pc)
 {
     int i;
 
-- 
2.47.1

Re: [RFC PATCH 2/3] target/arm: Replace target_ulong -> uint64_t for HWBreakpoint

Posted by Richard Henderson 8 months ago

On 4/15/25 10:22, Philippe Mathieu-Daudé wrote:
> CPUARMState::pc is of type uint64_t.

That's not a good argument.  It's a guest virtual address, and using vaddr would 
self-document that fact.


r~

Re: [RFC PATCH 2/3] target/arm: Replace target_ulong -> uint64_t for HWBreakpoint

Posted by Pierrick Bouvier 8 months ago

On 4/16/25 10:05, Richard Henderson wrote:
> On 4/15/25 10:22, Philippe Mathieu-Daudé wrote:
>> CPUARMState::pc is of type uint64_t.
> 
> That's not a good argument.  It's a guest virtual address, and using vaddr would
> self-document that fact.
>

Ideally, should we update CPUARMState::pc also?

> 
> r~

Re: [RFC PATCH 2/3] target/arm: Replace target_ulong -> uint64_t for HWBreakpoint

Posted by Richard Henderson 8 months ago

On 4/16/25 10:13, Pierrick Bouvier wrote:
> On 4/16/25 10:05, Richard Henderson wrote:
>> On 4/15/25 10:22, Philippe Mathieu-Daudé wrote:
>>> CPUARMState::pc is of type uint64_t.
>>
>> That's not a good argument.  It's a guest virtual address, and using vaddr would
>> self-document that fact.
>>
> 
> Ideally, should we update CPUARMState::pc also?

I don't think so, no.

r~

Re: [RFC PATCH 2/3] target/arm: Replace target_ulong -> uint64_t for HWBreakpoint

Posted by Pierrick Bouvier 8 months, 1 week ago

On 4/15/25 10:22, Philippe Mathieu-Daudé wrote:
> CPUARMState::pc is of type uint64_t.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/internals.h   | 6 +++---
>   target/arm/hyp_gdbstub.c | 6 +++---
>   2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 175fb792375..05ceb9bcf5d 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1942,9 +1942,9 @@ extern GArray *hw_breakpoints, *hw_watchpoints;
>   #define get_hw_bp(i)    (&g_array_index(hw_breakpoints, HWBreakpoint, i))
>   #define get_hw_wp(i)    (&g_array_index(hw_watchpoints, HWWatchpoint, i))
>   
> -bool find_hw_breakpoint(CPUState *cpu, target_ulong pc);
> -int insert_hw_breakpoint(target_ulong pc);
> -int delete_hw_breakpoint(target_ulong pc);
> +bool find_hw_breakpoint(CPUState *cpu, uint64_t pc);
> +int insert_hw_breakpoint(uint64_t pc);
> +int delete_hw_breakpoint(uint64_t pc);
>   
>   bool check_watchpoint_in_range(int i, target_ulong addr);
>   CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr);
> diff --git a/target/arm/hyp_gdbstub.c b/target/arm/hyp_gdbstub.c
> index 1e861263b3d..4e52d37ed80 100644
> --- a/target/arm/hyp_gdbstub.c
> +++ b/target/arm/hyp_gdbstub.c
> @@ -54,7 +54,7 @@ GArray *hw_breakpoints, *hw_watchpoints;
>    * here so future PC comparisons will work properly.
>    */
>   
> -int insert_hw_breakpoint(target_ulong addr)
> +int insert_hw_breakpoint(uint64_t addr)
>   {
>       HWBreakpoint brk = {
>           .bcr = 0x1,                             /* BCR E=1, enable */
> @@ -80,7 +80,7 @@ int insert_hw_breakpoint(target_ulong addr)
>    * Delete a breakpoint and shuffle any above down
>    */
>   
> -int delete_hw_breakpoint(target_ulong pc)
> +int delete_hw_breakpoint(uint64_t pc)
>   {
>       int i;
>       for (i = 0; i < hw_breakpoints->len; i++) {
> @@ -226,7 +226,7 @@ int delete_hw_watchpoint(target_ulong addr, target_ulong len, int type)
>       return -ENOENT;
>   }
>   
> -bool find_hw_breakpoint(CPUState *cpu, target_ulong pc)
> +bool find_hw_breakpoint(CPUState *cpu, uint64_t pc)
>   {
>       int i;
>   

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>