[Xen-devel] Re: [PATCH] qemu_ram_ptr_length: take ram_addr_t as arguments

[Peter Maydell <peter.maydell@xxxxxxxxxx>]

On 24 June 2011 12:08,  <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>
> 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.)
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 (b) initialise it to an obviously
bogus value if we have to do so to shut the compiler up.

(Also 'addr1' is not a fantastic variable name :-))

-- PMM

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel