| Message ID | 1370794247-28267-18-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
On 9 June 2013 17:10, Andreas Färber <afaerber@suse.de> wrote: > The memory mapping API uses hwaddr, so use it in the struct, too. > This avoids a header dependency on target_ulong type. > --- a/include/sysemu/memory_mapping.h > +++ b/include/sysemu/memory_mapping.h > @@ -20,7 +20,7 @@ > /* The physical and virtual address in the memory mapping are contiguous. */ > typedef struct MemoryMapping { > hwaddr phys_addr; > - target_ulong virt_addr; > + hwaddr virt_addr; > ram_addr_t length; This seems kind of odd given that HACKING specifically says that target_ulong is for virtual addresses and hwaddr for physical addresses. And in fact all the places that call memory_mapping_list_add_merge_sorted() actually pass a target_ulong as the virt_addr parameter, so maybe what we should be fixing is the function prototype? Incidentally that use of ram_addr_t for length looks kind of suspicious to me; it should probably be a hwaddr. thanks -- PMM
Am 09.06.2013 19:17, schrieb Peter Maydell: > On 9 June 2013 17:10, Andreas Färber <afaerber@suse.de> wrote: >> The memory mapping API uses hwaddr, so use it in the struct, too. >> This avoids a header dependency on target_ulong type. > >> --- a/include/sysemu/memory_mapping.h >> +++ b/include/sysemu/memory_mapping.h >> @@ -20,7 +20,7 @@ >> /* The physical and virtual address in the memory mapping are contiguous. */ >> typedef struct MemoryMapping { >> hwaddr phys_addr; >> - target_ulong virt_addr; >> + hwaddr virt_addr; >> ram_addr_t length; > > This seems kind of odd given that HACKING specifically > says that target_ulong is for virtual addresses and hwaddr > for physical addresses. And in fact all the places that call > memory_mapping_list_add_merge_sorted() actually pass a > target_ulong as the virt_addr parameter, so maybe what we > should be fixing is the function prototype? Maybe. The benefit with going with the overwhelming use of hwaddr is that memory_mapping.o can be compiled once for all targets, saving compilation time. The benefit of target_ulong would be saving a few bytes for 32-bit targets at runtime. > Incidentally that use of ram_addr_t for length looks > kind of suspicious to me; it should probably be a hwaddr. Looking at it strictly from a CPU and build perspective I'll happily leave further code cleanups to someone else. :) Somewhere there's also a misspelling of "memory" left to fix. Andreas
diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h index 6dfb68d..1411cc7 100644 --- a/include/sysemu/memory_mapping.h +++ b/include/sysemu/memory_mapping.h @@ -20,7 +20,7 @@ /* The physical and virtual address in the memory mapping are contiguous. */ typedef struct MemoryMapping { hwaddr phys_addr; - target_ulong virt_addr; + hwaddr virt_addr; ram_addr_t length; QTAILQ_ENTRY(MemoryMapping) next; } MemoryMapping;
The memory mapping API uses hwaddr, so use it in the struct, too. This avoids a header dependency on target_ulong type. Cc: Wen Congyang <wency@cn.fujitsu.com> Signed-off-by: Andreas Färber <afaerber@suse.de> --- include/sysemu/memory_mapping.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)