On Fri, 24 Jun 2011, Peter Maydell wrote:
> 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.)
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;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home