[PATCH 0/2] memory.c: make memory_region_dispatch_* report ACCESS_ERROR

Daniel Henrique Barboza posted 2 patches 5 days, 9 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260602190857.1506939-1-daniel.barboza@oss.qualcomm.com
Maintainers: Matthew Rosato <mjrosato@linux.ibm.com>, Farhan Ali <alifm@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Cornelia Huck <cohuck@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>, Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>, "Philippe Mathieu-Daudé" <philmd@mailo.com>
hw/s390x/s390-pci-inst.c |  2 +-
include/system/memory.h  |  2 +-
system/memory.c          | 25 ++++++++++++++++++++-----
system/physmem.c         |  3 ++-
4 files changed, 24 insertions(+), 8 deletions(-)
[PATCH 0/2] memory.c: make memory_region_dispatch_* report ACCESS_ERROR
Posted by Daniel Henrique Barboza 5 days, 9 hours ago
Hello,

I decided to propose this change after working in a RISC-V gitlab bug
[1].  In my proposed fix [2] I had to make assumptions about
address_space_ld* error return, which is always MEMTX_DECODE_ERROR, to
identify that in that particular scenario this is a permission error.
In RISC-V (and from what I can see other archs like x86) an ACCESS_ERROR
due to mem page permission issues will fire a different fault than other
errors like page not found.

The core logic is on memory_region_access_valid(), which already makes a
distinction between error reasons when we look at the qemu_log_mask()
errors, e.g. this is being characterized as a 'rejected':

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


but memory_region_access_valid() just returns a bool, and we remain
oblivious about about the nature of the error found.  Patch 1 adds an
extra parameter in memory_region_access_valid() to write the reason code
for a valid/invalid access. 

Patch 2 changes memory_region_dispatch_(read|write) to return the reason
code from memory_region_access_valid() in case an error was found.  This
will change error codes from DECODE_ERROR to ACCESS_ERROR when there's a
permission error involved, so before proposing this change I did a
search in all cases where DECODE_ERROR is handled via
memory_region_dispatch_* APIs.  I didn't see any code where a
DECODE_ERROR is ignored (i.e treated as an OK result) and that would be
impacted by this change.  Gitlab CI didn't complain about it either, so
I feel moderately confident that this change can be done without too
much harm.


[1] https://gitlab.com/qemu-project/qemu/-/work_items/3502
[2] https://lore.kernel.org/qemu-devel/20260522172502.320529-1-daniel.barboza@oss.qualcomm.com/

Daniel Henrique Barboza (2):
  system/memory.c: add MemTxResult arg in memory_region_access_valid()
  memory.c: read memory_region_access_valid() err reason in
    memory_region_dispatch_*

 hw/s390x/s390-pci-inst.c |  2 +-
 include/system/memory.h  |  2 +-
 system/memory.c          | 25 ++++++++++++++++++++-----
 system/physmem.c         |  3 ++-
 4 files changed, 24 insertions(+), 8 deletions(-)

-- 
2.43.0