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

["Nicholas Piggin" <npiggin@gmail.com>]

On Mon Mar 17, 2025 at 7:03 PM AEST, Philippe Mathieu-Daudé wrote:
> 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,
>>   {
>
> Alternatively:
>
>          int invalid_mem_mask = attrs.debug ? LOG_INVALID_MEM : 0;

Oh that's a thing? Would save a level of indent and might look
nicer. (I guess you have x : y expressions reversed)

Thanks,
Nick

>
>>       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;
>>       }