Re: [PATCH] memory: Log access direction for invalid accesses

[David Hildenbrand]

On 11.10.21 19:32, BALATON Zoltan wrote:
> In memory_region_access_valid() invalid accesses are logged to help
> debugging but the log message does not say if it was a read or write.
> Log that too to better identify the access causing the problem.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   softmmu/memory.c | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index db182e5d3d..e5826faa0c 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -1378,17 +1378,17 @@ bool memory_region_access_valid(MemoryRegion *mr,
>   {
>       if (mr->ops->valid.accepts
>           && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, attrs)) {
> -        qemu_log_mask(LOG_GUEST_ERROR, "Invalid access at addr "
> -                                       "0x%" HWADDR_PRIX ", size %u, "
> -                                       "region '%s', reason: rejected\n",
> +        qemu_log_mask(LOG_GUEST_ERROR, "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_GUEST_ERROR, "Invalid access at addr "
> -                                       "0x%" HWADDR_PRIX ", size %u, "
> -                                       "region '%s', reason: unaligned\n",
> +        qemu_log_mask(LOG_GUEST_ERROR, "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;
>       }
> @@ -1400,10 +1400,10 @@ 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_GUEST_ERROR, "Invalid access at addr "
> -                                       "0x%" HWADDR_PRIX ", size %u, "
> -                                       "region '%s', reason: invalid size "
> -                                       "(min:%u max:%u)\n",
> +        qemu_log_mask(LOG_GUEST_ERROR, "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);

Reviewed-by: David Hildenbrand <david@redhat.com>

--
Thanks,

David / dhildenb