qemu: fix some warnings

PATCH In review Jan Kiszka 3 participants 6 messages Jan 21, 2008 – Jan 24, 2008 2 messages not in the archive KVM

Share & tools

Jan Kiszka

author

2008-01-21 12:46 UTC #0 lore raw

PATCH 1/1 qemu: fix some warnings

Here are 4 more warnings fixes (actually, I should sent 2 of them to qemu...). Nothing critical, just less noise during compilation.

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.

Jan

---
 qemu/hw/pc.c       |   31 -------------------------------
 qemu/kvm-tpr-opt.c |   13 -------------
 qemu/migration.c   |    4 ++--
 qemu/vl.c          |    2 +-
 4 files changed, 3 insertions(+), 47 deletions(-)

qemu/hw/pc.c +0 ~~-31~~

Index: b/qemu/hw/pc.c
===================================================================
@@ -450,37 +450,6 @@ static void generate_bootsect(uint32_t g
     bdrv_set_boot_sector(drives_table[hda].bdrv, bootsect, sizeof(bootsect));
 }
 
-static int load_kernel(const char *filename, uint8_t *addr,
-                       uint8_t *real_addr)
-{
-    int fd, size;
-    int setup_sects;
-
-    fd = open(filename, O_RDONLY | O_BINARY);
-    if (fd < 0)
-        return -1;
-
-    /* load 16 bit code */
-    if (read(fd, real_addr, 512) != 512)
-        goto fail;
-    setup_sects = real_addr[0x1F1];
-    if (!setup_sects)
-        setup_sects = 4;
-    if (read(fd, real_addr + 512, setup_sects * 512) !=
-        setup_sects * 512)
-        goto fail;
-
-    /* load 32 bit code */
-    size = read(fd, addr, 16 * 1024 * 1024);
-    if (size < 0)
-        goto fail;
-    close(fd);
-    return size;
- fail:
-    close(fd);
-    return -1;
-}
-
 static long get_file_size(FILE *f)
 {
     long where, size;

qemu/kvm-tpr-opt.c +0 ~~-13~~

Index: b/qemu/kvm-tpr-opt.c
===================================================================
@@ -76,19 +76,6 @@ static void write_byte_virt(CPUState *en
     stb_phys(map_addr(&sregs, virt, NULL), b);
 }
 
-static uint32_t get_bios_map(CPUState *env, unsigned *perms)
-{
-    uint32_t v;
-    struct kvm_sregs sregs;
-
-    kvm_get_sregs(kvm_context, env->cpu_index, &sregs);
-
-    for (v = -4096u; v != 0; v -= 4096)
-	if (map_addr(&sregs, v, perms) == 0xe0000)
-	    return v;
-    return -1u;
-}
-
 struct vapic_bios {
     char signature[8];
     uint32_t virt_base;

qemu/migration.c +2 -2

Index: b/qemu/migration.c
===================================================================
@@ -808,7 +808,7 @@ static int migrate_incoming_fd(int fd)
 
     size = qemu_get_be32(f);
     if (size != phys_ram_size) {
-        fprintf(stderr, "migration: memory size mismatch: recv %u mine %u\n",
+        fprintf(stderr, "migration: memory size mismatch: recv %u mine %lu\n",
                 size, phys_ram_size);
 	return MIG_STAT_DST_MEM_SIZE_MISMATCH;
     }

@@ -1063,7 +1063,7 @@ void do_info_migration(void)
 	    term_printf("Transfer rate %3.1f mb/s\n",
 			(double)s->bps / (1024 * 1024));
 	term_printf("Iteration %d\n", s->iteration);
-	term_printf("Transferred %d/%d pages\n", s->updated_pages, phys_ram_size >> TARGET_PAGE_BITS);
+	term_printf("Transferred %d/%lu pages\n", s->updated_pages, phys_ram_size >> TARGET_PAGE_BITS);
 	if (s->iteration)
 	    term_printf("Last iteration found %d dirty pages\n", s->last_updated_pages);
     } else {

qemu/vl.c +1 -1

Index: b/qemu/vl.c
===================================================================
@@ -8862,7 +8862,7 @@ int main(int argc, char **argv)
                 if (ram_size <= 0)
                     help(1);
                 if (ram_size > PHYS_RAM_MAX_SIZE) {
-                    fprintf(stderr, "qemu: at most %d MB RAM can be simulated\n",
+                    fprintf(stderr, "qemu: at most %llu MB RAM can be simulated\n",
                             PHYS_RAM_MAX_SIZE / (1024 * 1024));
                     exit(1);
                 }

Signature · 7 lines

Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

A message that is not in the archive (sent off-list or not yet received): [email protected]

Jan Kiszka

author

2008-01-21 13:06 UTC #2 lore raw

Quoted 9 lines · Jan Kiszka

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.

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

Jan

---
 qemu/exec.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

qemu/exec.c +7 -7

Index: 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",
                 size, phys_ram_size);
         abort();
     }

@@ -2455,8 +2455,8 @@ static CPUWriteMemoryFunc *subpage_write
     &subpage_writel,
 };
 
-static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
-                             int memory)
+static int subpage_register (subpage_t *mmio, target_phys_addr_t start,
+                             target_phys_addr_t end, int memory)
 {
     int idx, eidx;

@@ -2478,7 +2478,7 @@ static int subpage_register (subpage_t *
     return 0;
 }
 
-static void *subpage_init (target_phys_addr_t base, uint32_t *phys,
+static void *subpage_init (target_phys_addr_t base, target_phys_addr_t *phys,
                            int orig_memory)
 {
     subpage_t *mmio;

Signature · 7 lines

Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

A message that is not in the archive (sent off-list or not yet received): [email protected]

Carlo Marcelo Arenas Belon

2008-01-21 14:00 UTC #4 lore raw

Quoted 10 lines · Jan Kiszka

Quoted 9 lines · Jan Kiszka
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.

Quoted 2 lines

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

Quoted 22 lines

---
 qemu/exec.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Index: kvm-userspace/qemu/exec.c
===================================================================

qemu/exec.c +3 -3

@@ -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)                                     \

qemu/exec.c ram_addr_t qemu_ram_alloc(unsigned long · line 2141

Quoted 5 lines

qemu/exec.c +1 -1

@@ -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

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Avi Kivity

2008-01-24 06:34 UTC #5 lore raw

In reply to a message not in the archive

Quoted 9 lines · Jan Kiszka

Quoted 7 lines · Jan Kiszka
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);

Quoted 8 lines

Quoted 3 lines
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.
At least it should cause no harm to convert to target_phys_addr_t, so here is a cleanup patch for qemu/exec.c.

Please post these patches to qemu-devel, in order to limit the divergence between kvm's version of qemu and upstream.

Signature · 7 lines

Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Carlo Marcelo Arenas Belon

2008-01-21 14:22 UTC #6 lore raw

In reply to a message not in the archive

Quoted 2 lines · Jan Kiszka

Here are 4 more warnings fixes (actually, I should sent 2 of them to qemu...). Nothing critical, just less noise during compilation.

probably a good idea having them in independent patches as they are unrelated (other by the fact that they are all warnings in current git).

qemu/vl.c int main(int argc, char **argv) · line 8866

Quoted 11 lines

Index: b/qemu/vl.c
===================================================================

qemu/vl.c +1 -1

@@ -8862,7 +8862,7 @@ int main(int argc, char **argv)
                 if (ram_size <= 0)
                     help(1);
                 if (ram_size > PHYS_RAM_MAX_SIZE) {
-                    fprintf(stderr, "qemu: at most %d MB RAM can be simulated\n",
+                    fprintf(stderr, "qemu: at most %llu MB RAM can be simulated\n",
                             PHYS_RAM_MAX_SIZE / (1024 * 1024));

using TARGET_FMT_lu instead of "%llu" would seem more appropriate here because this is meant to reflect a physical memory address, but then the fact that kvm is using the x64_64 target also for 32 bit will mean that the definition of PHYS_RAM_MAX_SIZE has to be made somehow also HOST specific.

for my take on that (which will need to be updated with your version of the exec.c changes and re-tested) look at :

http://tapir.sajinet.com.pe/gentoo/portage/app-emulation/kvm/files/kvm-51-qemu-ramaddr.patch

Carlo

Jan Kiszka

author

2008-01-22 10:06 UTC #7 lore raw

Quoted 6 lines · Carlo Marcelo Arenas Belon

Quoted 2 lines · Jan Kiszka
Here are 4 more warnings fixes (actually, I should sent 2 of them to qemu...). Nothing critical, just less noise during compilation.
probably a good idea having them in independent patches as they are unrelated (other by the fact that they are all warnings in current git).

Will do (later).

Quoted 18 lines

qemu/vl.c int main(int argc, char **argv) · line 8866
Quoted 12 lines
Index: b/qemu/vl.c
===================================================================
qemu/vl.c +1 -1
@@ -8862,7 +8862,7 @@ int main(int argc, char **argv)
                 if (ram_size <= 0)
                     help(1);
                 if (ram_size > PHYS_RAM_MAX_SIZE) {
-                    fprintf(stderr, "qemu: at most %d MB RAM can be simulated\n",
+                    fprintf(stderr, "qemu: at most %llu MB RAM can be simulated\n",
                             PHYS_RAM_MAX_SIZE / (1024 * 1024));
using TARGET_FMT_lu instead of "%llu" would seem more appropriate here because this is meant to reflect a physical memory address, but then the fact that kvm is using the x64_64 target also for 32 bit will mean that the definition of PHYS_RAM_MAX_SIZE has to be made somehow also HOST specific.

Quoted 5 lines

for my take on that (which will need to be updated with your version of the exec.c changes and re-tested) look at :
http://tapir.sajinet.com.pe/gentoo/portage/app-emulation/kvm/files/kvm-51-qemu-ramaddr.patch

Ah, that's great. So what about this rebased and extended version:

---
 qemu/exec.c      |   10 +++++-----
 qemu/migration.c |    4 ++--
 qemu/vl.c        |    8 ++++++--
 3 files changed, 13 insertions(+), 9 deletions(-)

qemu/exec.c +5 -5

Index: 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,
+static int subpage_register (subpage_t *mmio, ram_addr_t start, ram_addr_t end,
                              int memory);
-static void *subpage_init (target_phys_addr_t base, uint32_t *phys,
+static void *subpage_init (target_phys_addr_t base, ram_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",
                 size, phys_ram_size);
         abort();
     }

@@ -2455,7 +2455,7 @@ static CPUWriteMemoryFunc *subpage_write
     &subpage_writel,
 };
 
-static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
+static int subpage_register (subpage_t *mmio, ram_addr_t start, ram_addr_t end,
                              int memory)
 {
     int idx, eidx;

@@ -2478,7 +2478,7 @@ static int subpage_register (subpage_t *
     return 0;
 }
 
-static void *subpage_init (target_phys_addr_t base, uint32_t *phys,
+static void *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
                            int orig_memory)
 {
     subpage_t *mmio;

qemu/migration.c +2 -2

Index: kvm-userspace/qemu/migration.c
===================================================================
@@ -808,7 +808,7 @@ static int migrate_incoming_fd(int fd)
 
     size = qemu_get_be32(f);
     if (size != phys_ram_size) {
-        fprintf(stderr, "migration: memory size mismatch: recv %u mine %u\n",
+        fprintf(stderr, "migration: memory size mismatch: recv %u mine %lu\n",
                 size, phys_ram_size);
 	return MIG_STAT_DST_MEM_SIZE_MISMATCH;
     }

@@ -1063,7 +1063,7 @@ void do_info_migration(void)
 	    term_printf("Transfer rate %3.1f mb/s\n",
 			(double)s->bps / (1024 * 1024));
 	term_printf("Iteration %d\n", s->iteration);
-	term_printf("Transferred %d/%d pages\n", s->updated_pages, phys_ram_size >> TARGET_PAGE_BITS);
+	term_printf("Transferred %d/%lu pages\n", s->updated_pages, phys_ram_size >> TARGET_PAGE_BITS);
 	if (s->iteration)
 	    term_printf("Last iteration found %d dirty pages\n", s->last_updated_pages);
     } else {

qemu/vl.c +6 -2

Index: kvm-userspace/qemu/vl.c
===================================================================
@@ -148,10 +148,14 @@ int inet_aton(const char *cp, struct in_
 //#define DEBUG_UNUSED_IOPORT
 //#define DEBUG_IOPORT
 
-#if HOST_LONG_BITS < 64
+#if TARGET_LONG_BITS < 64
 #define PHYS_RAM_MAX_SIZE (2047 * 1024 * 1024)
 #else
+#if HOST_LONG_BITS < 64
 #define PHYS_RAM_MAX_SIZE (2047 * 1024 * 1024 * 1024ULL)
+#else
+#define PHYS_RAM_MAX_SIZE (2047 * 1024 * 1024 * 1024UL)
+#endif
 #endif
 
 #ifdef TARGET_PPC

@@ -8862,7 +8866,7 @@ int main(int argc, char **argv)
                 if (ram_size <= 0)
                     help(1);
                 if (ram_size > PHYS_RAM_MAX_SIZE) {
-                    fprintf(stderr, "qemu: at most %d MB RAM can be simulated\n",
+                    fprintf(stderr, "qemu: at most " TARGET_FMT_lu " MB RAM can be simulated\n",
                             PHYS_RAM_MAX_SIZE / (1024 * 1024));
                     exit(1);
                 }

Signature · 7 lines

Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/