Re: [PATCH] qemu: fix some warnings

Carlo Marcelo Arenas Belon
2008-01-21T14:00:16Z

On Mon, Jan 21, 2008 at 02:06:01PM +0100, Jan Kiszka wrote:
> Jan Kiszka wrote:
> > At this chance, can anyone comment on these uint32_t types?
> >
> > static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
> > int memory);
> > static void *subpage_init (target_phys_addr_t base, uint32_t *phys,
> > int orig_memory);
> >
> > Shouldn't they be target_phys_addr_t? Can't this cause troubles when
> > building 64-bit targets? Looks like it's even a generic qemu issue.

not sure what you mean here, but qemu doesn't have a mismatching type for
subpage_init because the types they use are all width consistent and tied to
the target processor.

kvm uses the x86_64 target for 32bit and therefore breaks that assumption
triggering this issues AFAIK.

> At least it should cause no harm to convert to target_phys_addr_t, so
> here is a cleanup patch for qemu/exec.c.

the SUBPAGE_IDX might not be correct in this case as it is dependant in
TARGET_PAGE_SIZE and TARGET_PAGE_BITS

> ---
> qemu/exec.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> Index: kvm-userspace/qemu/exec.c
> ===================================================================
> --- kvm-userspace.orig/qemu/exec.c
> +++ kvm-userspace/qemu/exec.c
> @@ -2028,9 +2028,9 @@ static inline void tlb_set_dirty(CPUStat
> }
> #endif /* defined(CONFIG_USER_ONLY) */
>
> -static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
> - int memory);
> -static void *subpage_init (target_phys_addr_t base, uint32_t *phys,
> +static int subpage_register (subpage_t *mmio, target_phys_addr_t start,
> + target_phys_addr_t end, int memory);
> +static void *subpage_init (target_phys_addr_t base, target_phys_addr_t *phys,
> int orig_memory);
> #define CHECK_SUBPAGE(addr, start_addr, start_addr2, end_addr, end_addr2, \
> need_subpage) \
> @@ -2138,7 +2138,7 @@ ram_addr_t qemu_ram_alloc(unsigned long
> {
> ram_addr_t addr;
> if ((phys_ram_alloc_offset + size) > phys_ram_size) {
> - fprintf(stderr, "Not enough memory (requested_size = %lu, max memory = %d)\n",
> + fprintf(stderr, "Not enough memory (requested_size = %lu, max memory = %lu)\n",

TARGET_FMT_lu will be a better match for this format as it is meant to be
defined correctly for each target by qemu.

Carlo