| Message ID | 20241218193153.151578-1-git@zabka.it (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | physmem: allow cpu_memory_rw_debug to write to MMIO devices | expand |
On 18.12.24 20:31, Stefan Zabka wrote: > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/213 > Signed-off-by: Stefan Zabka <git@zabka.it> > --- > system/physmem.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/system/physmem.c b/system/physmem.c > index dc1db3a384..53cdccefcb 100644 > --- a/system/physmem.c > +++ b/system/physmem.c > @@ -3556,6 +3556,7 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr, > hwaddr phys_addr; > vaddr l, page; > uint8_t *buf = ptr; > + bool is_memcpy_access; > > cpu_synchronize_state(cpu); > while (len > 0) { > @@ -3574,11 +3575,24 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr, > l = len; > phys_addr += (addr & ~TARGET_PAGE_MASK); > if (is_write) { > - res = address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr, > - attrs, buf, l); > + /* if ram/rom region we access the memory > + via memcpy instead of via the cpu */ Coding style says: "Multiline comment blocks should have a row of stars on the left, and the initial /* and terminating */ both on their own lines:" > + hwaddr mr_len, addr1; > + AddressSpace *as = cpu->cpu_ases[asidx].as; > + MemoryRegion *mr = address_space_translate(as, phys_addr, &addr1, &mr_len, is_write, attrs); > + > + is_memcpy_access = memory_region_is_ram(mr) || memory_region_is_romd(mr); > + if(!is_memcpy_access) { > + l = memory_access_size(mr, l, addr1); > + } > + } else { > + is_memcpy_access = false; > + } Is this really required? The additional address_space_translate() is nasty, and flatview_write_continue_step() will perform a memmove directly to the RAMBlock if memory_access_is_direct() allows for it. Would it be possible to always use address_space_rw()? If there is a reason we need the address_space_write_rom(), could we simply fallback to it if address_space_rw(true) failed? That might make it all simpler. > + > + if (is_write && is_memcpy_access) { > + res = address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, l); > } else { > - res = address_space_read(cpu->cpu_ases[asidx].as, phys_addr, > - attrs, buf, l); > + res = address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, l, is_write); > } > if (res != MEMTX_OK) { > return -1;
diff --git a/system/physmem.c b/system/physmem.c index dc1db3a384..53cdccefcb 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -3556,6 +3556,7 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr, hwaddr phys_addr; vaddr l, page; uint8_t *buf = ptr; + bool is_memcpy_access; cpu_synchronize_state(cpu); while (len > 0) { @@ -3574,11 +3575,24 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr, l = len; phys_addr += (addr & ~TARGET_PAGE_MASK); if (is_write) { - res = address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr, - attrs, buf, l); + /* 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, attrs); + + is_memcpy_access = memory_region_is_ram(mr) || memory_region_is_romd(mr); + if(!is_memcpy_access) { + l = memory_access_size(mr, l, addr1); + } + } else { + is_memcpy_access = false; + } + + if (is_write && is_memcpy_access) { + res = address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, l); } else { - res = address_space_read(cpu->cpu_ases[asidx].as, phys_addr, - attrs, buf, l); + res = address_space_rw(cpu->cpu_ases[asidx].as, phys_addr, attrs, buf, l, is_write); } if (res != MEMTX_OK) { return -1;
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/213 Signed-off-by: Stefan Zabka <git@zabka.it> --- system/physmem.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)