Add an accessor for gdb physical memory access mode which sets the
the .debug attribute for the MemTxAttribute, and also returns success
to the caller.

GDB with PhyMemMode will now report failure from memory accesses outside
valid system memory addresses, and it is also able to write to ROMs as
GDB virtual memory access can.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 docs/devel/loads-stores.rst | 11 +++++++++++
 include/exec/cpu-common.h   |  3 +++
 gdbstub/system.c            |  7 +------
 system/physmem.c            | 16 ++++++++++++++++
 4 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/docs/devel/loads-stores.rst b/docs/devel/loads-stores.rst
index 9471bac8599..ac2e0d34d67 100644
--- a/docs/devel/loads-stores.rst
+++ b/docs/devel/loads-stores.rst
@@ -481,6 +481,17 @@ would ignore the write attempt).
 
 ``cpu_memory_rw_debug``
 
+``phys_memory_rw_debug``
+~~~~~~~~~~~~~~~~~~~~~~~
+
+Access system memory by physical address for debug purposes.
+
+This function is intended for use by the GDB stub and similar code.
+It takes a physical address and operates on the system address space.
+Access is performed as in cpu_memory_rw_debug().
+
+``phys_memory_rw_debug``
+
 ``dma_memory_*``
 ~~~~~~~~~~~~~~~~
 
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 3771b2130c2..6429dc2331e 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -181,6 +181,9 @@ int ram_block_discard_guest_memfd_range(RAMBlock *rb, uint64_t start,
 /* Returns: 0 on success, -1 on error */
 int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
                         void *ptr, size_t len, bool is_write);
+/* Returns: 0 on success, -1 on error */
+int phys_memory_rw_debug(hwaddr addr, void *buf,
+                         hwaddr len, bool is_write);
 
 /* vl.c */
 void list_cpus(void);
diff --git a/gdbstub/system.c b/gdbstub/system.c
index dd22ff0fb3a..79fcb30f6f0 100644
--- a/gdbstub/system.c
+++ b/gdbstub/system.c
@@ -457,12 +457,7 @@ int gdb_target_memory_rw_debug(CPUState *cpu, hwaddr addr,
                                uint8_t *buf, int len, bool is_write)
 {
     if (phy_memory_mode) {
-        if (is_write) {
-            cpu_physical_memory_write(addr, buf, len);
-        } else {
-            cpu_physical_memory_read(addr, buf, len);
-        }
-        return 0;
+        return phys_memory_rw_debug(addr, buf, len, is_write);
     }
 
     if (cpu->cc->memory_rw_debug) {
diff --git a/system/physmem.c b/system/physmem.c
index e97de3ef65c..aa78b0d514b 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3743,6 +3743,22 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
     return 0;
 }
 
+/* physical memory access for debug (includes writing to ROM) */
+int phys_memory_rw_debug(hwaddr addr, void *buf,
+                         hwaddr len, bool is_write)
+{
+    MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
+    MemTxResult res;
+
+    attrs.debug = 1;
+    res = address_space_rw(&address_space_memory, addr, attrs,
+                           buf, len, is_write);
+    if (res != MEMTX_OK) {
+        return -1;
+    }
+    return 0;
+}
+
 bool cpu_physical_memory_is_io(hwaddr phys_addr)
 {
     MemoryRegion*mr;
-- 
2.47.1

On 3/14/25 00:41, Nicholas Piggin wrote:
> Add an accessor for gdb physical memory access mode which sets the
> the .debug attribute for the MemTxAttribute, and also returns success
> to the caller.
> 
> GDB with PhyMemMode will now report failure from memory accesses outside
> valid system memory addresses, and it is also able to write to ROMs as
> GDB virtual memory access can.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   docs/devel/loads-stores.rst | 11 +++++++++++
>   include/exec/cpu-common.h   |  3 +++
>   gdbstub/system.c            |  7 +------
>   system/physmem.c            | 16 ++++++++++++++++
>   4 files changed, 31 insertions(+), 6 deletions(-)
> 

I think you might as well put this function in gdbstub/system.c
and not export (or document) it.


r~

On Sat Mar 15, 2025 at 7:19 AM AEST, Richard Henderson wrote:
> On 3/14/25 00:41, Nicholas Piggin wrote:
>> Add an accessor for gdb physical memory access mode which sets the
>> the .debug attribute for the MemTxAttribute, and also returns success
>> to the caller.
>> 
>> GDB with PhyMemMode will now report failure from memory accesses outside
>> valid system memory addresses, and it is also able to write to ROMs as
>> GDB virtual memory access can.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>   docs/devel/loads-stores.rst | 11 +++++++++++
>>   include/exec/cpu-common.h   |  3 +++
>>   gdbstub/system.c            |  7 +------
>>   system/physmem.c            | 16 ++++++++++++++++
>>   4 files changed, 31 insertions(+), 6 deletions(-)
>> 
>
> I think you might as well put this function in gdbstub/system.c
> and not export (or document) it.

A possible advantage this way is gdbstub not knowing precise
details of the memory transaction (i.e., .debug = 1), but I
can do that. Will submit a v2.

Thanks,
Nick