[Qemu-devel] [Bug 1625216] [NEW] memory writes via gdb don't work for memory mapped hardware

* [Qemu-devel] [Bug 1625216] [NEW] memory writes via gdb don't work for memory mapped hardware
@ 2016-09-19 14:37 Andreas Rasmusson
  2016-09-19 16:13 ` no-reply
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Andreas Rasmusson @ 2016-09-19 14:37 UTC (permalink / raw)
  To: qemu-devel

Public bug reported:

When I remote-debug a qemu-guest and attempt to write to a memory mapped location, the
write-handler for the concerned device will not be called. All write-requiests from
gdb are delegated to cpu_physical_memory_write_rom(...). a function that writes to the 
underlying ram-block.

I believe requests to memory mapped hardware should be delegated to 
address_space_rw(). 

example:
;; a memory mapped device. No effect, the write-handler is not called
(gdb) set *0xfff3c000 = 48

;; a ram or rom-block. Thos works. The value is changed.
(gdb) set *0x100000 = 48


----------------------------------------

Here's my suggested patch. As noted in the comment, it could perhaps be
improved for the (rare) case when the write-request from gdb spans multiple 
memory regions.

$ git diff   85bc2a15121e8bcd9f15eb75794a1eacca9d84bd HEAD ../exec.c

diff --git a/exec.c b/exec.c
index c4f9036..45ef896 100644
--- a/exec.c
+++ b/exec.c
@@ -3676,6 +3676,7 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
     int l;
     hwaddr phys_addr;
     target_ulong page;
+    bool is_memcpy_access;
 
     while (len > 0) {
         int asidx;
@@ -3691,13 +3692,32 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
         if (l > len)
             l = len;
         phys_addr += (addr & ~TARGET_PAGE_MASK);
+
         if (is_write) {
+            /* if ram/rom region we access the memory 
+               via memcpy instead of via the cpu */
+            hwaddr mr_len, addr1;
+            AddressSpace *as = cpu->cpu_ases[asidx].as;
+            MemoryRegion *mr = address_space_translate(as, phys_addr, &addr1, &mr_len, is_write);
+            is_memcpy_access  = memory_region_is_ram(mr) || memory_region_is_romd(mr);
+            if(mr_len < len) {
+                /* TODO, mimic more of the loop over mr chunks as 
+                   done in cpu_physical_memory_write_internal */ 
+                printf("warning: we dont know whether all bytes "
+                       "to be written are ram/rom or io\n");
+            }
+        }
+        else {
+            is_memcpy_access = false;
+        }
+        
+        if (is_write && is_memcpy_access) {
             cpu_physical_memory_write_rom(cpu->cpu_ases[asidx].as,
                                           phys_addr, buf, l);
         } else {
             address_space_rw(cpu->cpu_ases[asidx].as, phys_addr,
                              MEMTXATTRS_UNSPECIFIED,
-                             buf, l, 0);
+                             buf, l, is_write);
         }
         len -= l;
         buf += l;

** Affects: qemu
     Importance: Undecided
         Status: New


** Tags: gdb mmio write

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1625216

Title:
  memory writes via gdb don't work for memory mapped hardware

Status in QEMU:
  New

Bug description:
  When I remote-debug a qemu-guest and attempt to write to a memory mapped location, the
  write-handler for the concerned device will not be called. All write-requiests from
  gdb are delegated to cpu_physical_memory_write_rom(...). a function that writes to the 
  underlying ram-block.

  I believe requests to memory mapped hardware should be delegated to 
  address_space_rw(). 

  example:
  ;; a memory mapped device. No effect, the write-handler is not called
  (gdb) set *0xfff3c000 = 48

  ;; a ram or rom-block. Thos works. The value is changed.
  (gdb) set *0x100000 = 48

  
  ----------------------------------------

  Here's my suggested patch. As noted in the comment, it could perhaps be
  improved for the (rare) case when the write-request from gdb spans multiple 
  memory regions.

  $ git diff   85bc2a15121e8bcd9f15eb75794a1eacca9d84bd HEAD ../exec.c
  diff --git a/exec.c b/exec.c
  index c4f9036..45ef896 100644
  --- a/exec.c
  +++ b/exec.c
  @@ -3676,6 +3676,7 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
       int l;
       hwaddr phys_addr;
       target_ulong page;
  +    bool is_memcpy_access;
   
       while (len > 0) {
           int asidx;
  @@ -3691,13 +3692,32 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
           if (l > len)
               l = len;
           phys_addr += (addr & ~TARGET_PAGE_MASK);
  +
           if (is_write) {
  +            /* if ram/rom region we access the memory 
  +               via memcpy instead of via the cpu */
  +            hwaddr mr_len, addr1;
  +            AddressSpace *as = cpu->cpu_ases[asidx].as;
  +            MemoryRegion *mr = address_space_translate(as, phys_addr, &addr1, &mr_len, is_write);
  +            is_memcpy_access  = memory_region_is_ram(mr) || memory_region_is_romd(mr);
  +            if(mr_len < len) {
  +                /* TODO, mimic more of the loop over mr chunks as 
  +                   done in cpu_physical_memory_write_internal */ 
  +                printf("warning: we dont know whether all bytes "
  +                       "to be written are ram/rom or io\n");
  +            }
  +        }
  +        else {
  +            is_memcpy_access = false;
  +        }
  +        
  +        if (is_write && is_memcpy_access) {
               cpu_physical_memory_write_rom(cpu->cpu_ases[asidx].as,
                                             phys_addr, buf, l);
           } else {
               address_space_rw(cpu->cpu_ases[asidx].as, phys_addr,
                                MEMTXATTRS_UNSPECIFIED,
  -                             buf, l, 0);
  +                             buf, l, is_write);
           }
           len -= l;
           buf += l;

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1625216/+subscriptions

^ permalink raw reply related	[flat|nested] 5+ messages in thread