Re: [PATCH 2/2] memory: suppress INVALID_MEM logs caused by debug access

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 14/3/25 08:41, Nicholas Piggin wrote:
> Debugger-driven invalid memory accesses are not guest errors, so should
> not cause these error logs.
> 
> Debuggers can access memory wildly, including access to addresses not
> specified by the user (e.g., gdb it might try to walk the stack or load
> target addresses to display disassembly). Failure is reported
> synchronously by the GDB protcol so the user can be notified via the
> debugger client.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   system/memory.c | 37 ++++++++++++++++++++++---------------
>   1 file changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/system/memory.c b/system/memory.c
> index 4c829793a0a..960f66e8d7e 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -1412,18 +1412,23 @@ bool memory_region_access_valid(MemoryRegion *mr,
>   {

Should we instead consider debug accesses as always valid? i.e.:

         if (attrs.debug) {
             return true;
         }

>       if (mr->ops->valid.accepts
>           && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, attrs)) {
> -        qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
> -                      ", size %u, region '%s', reason: rejected\n",
> -                      is_write ? "write" : "read",
> -                      addr, size, memory_region_name(mr));
> +        if (attrs.debug) {
> +            /* Don't log memory errors due to debugger accesses */
> +            qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
> +                          ", size %u, region '%s', reason: rejected\n",
> +                          is_write ? "write" : "read",
> +                          addr, size, memory_region_name(mr));
> +        }
>           return false;
>       }
>   
>       if (!mr->ops->valid.unaligned && (addr & (size - 1))) {
> -        qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
> -                      ", size %u, region '%s', reason: unaligned\n",
> -                      is_write ? "write" : "read",
> -                      addr, size, memory_region_name(mr));
> +        if (attrs.debug) {
> +            qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
> +                          ", size %u, region '%s', reason: unaligned\n",
> +                          is_write ? "write" : "read",
> +                          addr, size, memory_region_name(mr));
> +        }
>           return false;
>       }
>   
> @@ -1434,13 +1439,15 @@ bool memory_region_access_valid(MemoryRegion *mr,
>   
>       if (size > mr->ops->valid.max_access_size
>           || size < mr->ops->valid.min_access_size) {
> -        qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
> -                      ", size %u, region '%s', reason: invalid size "
> -                      "(min:%u max:%u)\n",
> -                      is_write ? "write" : "read",
> -                      addr, size, memory_region_name(mr),
> -                      mr->ops->valid.min_access_size,
> -                      mr->ops->valid.max_access_size);
> +        if (attrs.debug) {
> +            qemu_log_mask(LOG_INVALID_MEM, "Invalid %s at addr 0x%" HWADDR_PRIX
> +                          ", size %u, region '%s', reason: invalid size "
> +                          "(min:%u max:%u)\n",
> +                          is_write ? "write" : "read",
> +                          addr, size, memory_region_name(mr),
> +                          mr->ops->valid.min_access_size,
> +                          mr->ops->valid.max_access_size);
> +        }
>           return false;
>       }
>       return true;