[PATCH 07/24] system/watchpoint: Include missing 'exec/cpu-all.h' header

Philippe Mathieu-Daudé posted 24 patches 1 week, 6 days ago
[PATCH 07/24] system/watchpoint: Include missing 'exec/cpu-all.h' header
Posted by Philippe Mathieu-Daudé 1 week, 6 days ago
TARGET_PAGE_MASK is defined in "exec/cpu-all.h".
Include it in order to avoid when refactoring:

  system/watchpoint.c:52:24: error: use of undeclared identifier 'TARGET_PAGE_MASK'
     52 |     in_page = -(addr | TARGET_PAGE_MASK);
        |                        ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 system/watchpoint.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/system/watchpoint.c b/system/watchpoint.c
index 2aa2a9ea63..f7366574a3 100644
--- a/system/watchpoint.c
+++ b/system/watchpoint.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
 #include "exec/exec-all.h"
+#include "exec/cpu-all.h"
 #include "hw/core/cpu.h"
 
 /* Add a watchpoint.  */
-- 
2.45.2


Re: [PATCH 07/24] system/watchpoint: Include missing 'exec/cpu-all.h' header
Posted by Richard Henderson 1 week, 5 days ago
On 11/13/24 17:12, Philippe Mathieu-Daudé wrote:
> TARGET_PAGE_MASK is defined in "exec/cpu-all.h".
> Include it in order to avoid when refactoring:
> 
>    system/watchpoint.c:52:24: error: use of undeclared identifier 'TARGET_PAGE_MASK'
>       52 |     in_page = -(addr | TARGET_PAGE_MASK);
>          |                        ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   system/watchpoint.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/system/watchpoint.c b/system/watchpoint.c
> index 2aa2a9ea63..f7366574a3 100644
> --- a/system/watchpoint.c
> +++ b/system/watchpoint.c
> @@ -20,6 +20,7 @@
>   #include "qemu/osdep.h"
>   #include "qemu/error-report.h"
>   #include "exec/exec-all.h"
> +#include "exec/cpu-all.h"

This uses TARGET_PAGE_MASK to choose between flushing one page or the entire tlb.  Better 
would be to use tlb_flush_range_by_mmuidx(..., ALL_MMUIDX_BITS) to flush exactly one or 
two pages.

That said, I've just noticed that tlb_flush_range_by_mmuidx does not properly handle 
addr+len that are not already page aligned.  That can be fixed fairly easily.


r~

Re: [PATCH 07/24] system/watchpoint: Include missing 'exec/cpu-all.h' header
Posted by Pierrick Bouvier 1 week, 6 days ago

On 11/13/24 17:12, Philippe Mathieu-Daudé wrote:
> TARGET_PAGE_MASK is defined in "exec/cpu-all.h".
> Include it in order to avoid when refactoring:
> 
>    system/watchpoint.c:52:24: error: use of undeclared identifier 'TARGET_PAGE_MASK'
>       52 |     in_page = -(addr | TARGET_PAGE_MASK);
>          |                        ^
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   system/watchpoint.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/system/watchpoint.c b/system/watchpoint.c
> index 2aa2a9ea63..f7366574a3 100644
> --- a/system/watchpoint.c
> +++ b/system/watchpoint.c
> @@ -20,6 +20,7 @@
>   #include "qemu/osdep.h"
>   #include "qemu/error-report.h"
>   #include "exec/exec-all.h"
> +#include "exec/cpu-all.h"
>   #include "hw/core/cpu.h"
>   
>   /* Add a watchpoint.  */

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