On Fri, 24 Jun 2011, Peter Maydell wrote: > On 24 June 2011 12:08, <stefano.stabell...@eu.citrix.com> wrote: > > From: Stefano Stabellini <stefano.stabell...@eu.citrix.com> > > > > qemu_ram_ptr_length should take ram_addr_t as argument rather than > > target_phys_addr_t because is doing comparisons with RAMBlock addresses. > > > > cpu_physical_memory_map should create a ram_addr_t address to pass to > > qemu_ram_ptr_length from PhysPageDesc phys_offset. > > > > Remove code after abort() in qemu_ram_ptr_length. > > This does fix vexpress. However I think we're still doing the wrong > thing if the bounce buffer is already in use and addr points at an > IO page. In the old code, we would break out of the loop on the > if (done || bounce.buffer) condition, set *plen to 0 [because done==0 > since this is the first page] and return. Now we break out of the > loop but will fall into the call to qemu_ram_ptr_length() with a > bogus addr1 and probably abort(). > > You probably want to only call qemu_ram_ptr_length() if (todo). > (I don't know if anybody ever calls this routine with a zero input > length, but that would handle that case too.)
I would rather fix qemu_ram_ptr_length to handle 0 as size argument, and then call qemu_ram_ptr_length with 0 size from cpu_physical_memory_map (see appended patch). > It would also be better to either (a) not initialise addr1, if > the compiler is smart enough to know it can't get to the use > without it being initialised or The compiler is not smart enough, unfortunately. > (b) initialise it to an obviously > bogus value if we have to do so to shut the compiler up. All right, I am going to use ULONG_MAX. > (Also 'addr1' is not a fantastic variable name :-)) Agreed, but it is the same as before :) Do you have any better suggestion? Maybe raddr? I admit I am not very imaginative with names. --- diff --git a/exec.c b/exec.c index 7f62332..e6dbddb 100644 --- a/exec.c +++ b/exec.c @@ -3137,6 +3137,8 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) * but takes a size argument */ void *qemu_ram_ptr_length(ram_addr_t addr, ram_addr_t *size) { + if (*size == 0) + return NULL; if (xen_mapcache_enabled()) return qemu_map_cache(addr, *size, 1); else { @@ -4017,7 +4019,7 @@ void *cpu_physical_memory_map(target_phys_addr_t addr, target_phys_addr_t page; unsigned long pd; PhysPageDesc *p; - ram_addr_t addr1 = addr; + ram_addr_t addr1 = ULONG_MAX; ram_addr_t rlen; void *raddr;