x86: fix warning without CONFIG_KVM

PATCH Applied Mark McLoughlin 5 participants 12 messages Nov 13, 2008 – Nov 14, 2008

lore

Share & tools

lore

Mark McLoughlin

author

2008-11-13 16:43 UTC #0 lore raw

PATCH 1/3 x86: fix warning without CONFIG_KVM

Warning is:

target-i386/helper.c: In function `cpu_x86_cpuid':
target-i386/helper.c:1373: warning: implicit declaration of function `host_cpuid'

Signed-off-by Mark McLoughlin <[email protected]>

---
 target-i386/helper.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

target-i386/helper.c +2 -2

index 7ddb4ce..c8b8be9 100644
@@ -1296,10 +1296,10 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState *env, target_ulong addr)
 }
 #endif /* !CONFIG_USER_ONLY */
 
-#if defined(CONFIG_KVM)
 static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t *ebx,
                        uint32_t *ecx, uint32_t *edx)
 {
+#if defined(CONFIG_KVM)
     uint32_t vec[4];
 
 #ifdef __x86_64__

@@ -1327,8 +1327,8 @@ static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t *ebx,
 	*ecx = vec[2];
     if (edx)
 	*edx = vec[3];
-}
 #endif
+}
 
 void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
                    uint32_t *eax, uint32_t *ebx,

Signature · 1 line

1.5.4.3

Mark McLoughlin

author

2008-11-13 16:43 UTC #1 lore raw

PATCH 2/3 dyngen: fix some warnings about unused functions

Warnings were:

dyngen.c:292: error: ‘pstrcpy’ defined but not used

Only used with CONFIG_FORMAT_COFF

dyngen.c:319: error: ‘swab32ss’ defined but not used

With ELF_USES_RELOCA, either swab32ss() or swab64ss() will be used depending on ELF_CLASS. Easier to just mark them both as unused.

dyngen.c:334: error: ‘get16’ defined but not used dyngen.c:343: error: ‘get32’ defined but not used dyngen.c:352: error: ‘put16’ defined but not used dyngen.c:359: error: ‘put32’ defined but not used

Maybe be used for some host arches; easiest to mark them as unused.

dyngen.c:425: error: ‘elf_swap_phdr’ defined but not used

This one is completely unused; just delete.

Signed-off-by Mark McLoughlin <[email protected]>

---
 dyngen.c |   28 ++++++++++------------------
 1 files changed, 10 insertions(+), 18 deletions(-)

dyngen.c +10 ~~-18~~

index 68f23ad..c55a115 100644
@@ -32,6 +32,8 @@
 
 #include "config-host.h"
 
+#define ATTRIBUTE_UNUSED __attribute__((unused))
+
 /* NOTE: we test CONFIG_WIN32 instead of _WIN32 to enabled cross
    compilation */
 #if defined(CONFIG_WIN32)

@@ -288,6 +290,7 @@ static int strstart(const char *str, const char *val, const char **ptr)
     return 1;
 }
 
+#ifdef CONFIG_FORMAT_COFF
 static void pstrcpy(char *buf, int buf_size, const char *str)
 {
     int c;

@@ -304,6 +307,7 @@ static void pstrcpy(char *buf, int buf_size, const char *str)
     }
     *q = '\0';
 }
+#endif /* CONFIG_FORMAT_COFF */
 
 static void swab16s(uint16_t *p)
 {

@@ -315,7 +319,7 @@ static void swab32s(uint32_t *p)
     *p = bswap32(*p);
 }
 
-static void swab32ss(int32_t *p)
+static void ATTRIBUTE_UNUSED swab32ss(int32_t *p)
 {
     *p = bswap32(*p);
 }

@@ -325,12 +329,12 @@ static void swab64s(uint64_t *p)
     *p = bswap64(*p);
 }
 
-static void swab64ss(int64_t *p)
+static void ATTRIBUTE_UNUSED swab64ss(int64_t *p)
 {
     *p = bswap64(*p);
 }
 
-static uint16_t get16(uint16_t *p)
+static uint16_t ATTRIBUTE_UNUSED get16(uint16_t *p)
 {
     uint16_t val;
     val = *p;

@@ -339,7 +343,7 @@ static uint16_t get16(uint16_t *p)
     return val;
 }
 
-static uint32_t get32(uint32_t *p)
+static uint32_t ATTRIBUTE_UNUSED get32(uint32_t *p)
 {
     uint32_t val;
     val = *p;

@@ -348,14 +352,14 @@ static uint32_t get32(uint32_t *p)
     return val;
 }
 
-static void put16(uint16_t *p, uint16_t val)
+static void ATTRIBUTE_UNUSED put16(uint16_t *p, uint16_t val)
 {
     if (do_swap)
         val = bswap16(val);
     *p = val;
 }
 
-static void put32(uint32_t *p, uint32_t val)
+static void ATTRIBUTE_UNUSED put32(uint32_t *p, uint32_t val)
 {
     if (do_swap)
         val = bswap32(val);

@@ -421,18 +425,6 @@ static void elf_swap_shdr(struct elf_shdr *h)
   swabls(&h->	sh_entsize);		/* Entry size if section holds table */
 }
 
-static void elf_swap_phdr(struct elf_phdr *h)
-{
-    swab32s(&h->p_type);			/* Segment type */
-    swabls(&h->p_offset);		/* Segment file offset */
-    swabls(&h->p_vaddr);		/* Segment virtual address */
-    swabls(&h->p_paddr);		/* Segment physical address */
-    swabls(&h->p_filesz);		/* Segment size in file */
-    swabls(&h->p_memsz);		/* Segment size in memory */
-    swab32s(&h->p_flags);		/* Segment flags */
-    swabls(&h->p_align);		/* Segment alignment */
-}
-
 static void elf_swap_rel(ELF_RELOC *rel)
 {
     swabls(&rel->r_offset);

Signature · 1 line

1.5.4.3

Mark McLoughlin

author

2008-11-13 16:43 UTC #2 lore raw

PATCH 3/3 cris: fix a segfault if pflash drive not found

drive_get_index() returns -1 if a drive isn't found; don't use -1 to index drives_table.

Signed-off-by Mark McLoughlin <[email protected]>

---
 hw/etraxfs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

hw/etraxfs.c +1 -1

index 4f93cd8..2f6e2ee 100644
@@ -83,7 +83,7 @@ void bareetraxfs_init (ram_addr_t ram_size, int vga_ram_size,
     phys_flash = qemu_ram_alloc(FLASH_SIZE);
     i = drive_get_index(IF_PFLASH, 0, 0);
     pflash_cfi02_register(0x0, phys_flash,
-                          drives_table[i].bdrv, (64 * 1024),
+                          i != -1 ? drives_table[i].bdrv : NULL, (64 * 1024),
                           FLASH_SIZE >> 16,
                           1, 2, 0x0000, 0x0000, 0x0000, 0x0000,
                           0x555, 0x2aa);

Signature · 1 line

1.5.4.3

Anthony Liguori

2008-11-13 19:37 UTC #3 lore raw

[Qemu-devel] Re: [PATCH 3/3] cris: fix a segfault if pflash drive not found

Quoted 3 lines · Mark McLoughlin

drive_get_index() returns -1 if a drive isn't found; don't use -1 to index drives_table.

Looks fine to me. Edgar?

Regards,

Anthony Liguori

Quoted 19 lines

Signed-off-by Mark McLoughlin <[email protected]>

---
 hw/etraxfs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

hw/etraxfs.c +1 -1

index 4f93cd8..2f6e2ee 100644
@@ -83,7 +83,7 @@ void bareetraxfs_init (ram_addr_t ram_size, int vga_ram_size,
     phys_flash = qemu_ram_alloc(FLASH_SIZE);
     i = drive_get_index(IF_PFLASH, 0, 0);
     pflash_cfi02_register(0x0, phys_flash,
-                          drives_table[i].bdrv, (64 * 1024),
+                          i != -1 ? drives_table[i].bdrv : NULL, (64 * 1024),
                           FLASH_SIZE >> 16,
                           1, 2, 0x0000, 0x0000, 0x0000, 0x0000,
                           0x555, 0x2aa);

Edgar E. Iglesias

2008-11-13 20:57 UTC #4 lore raw

Re: [Qemu-devel] Re: [PATCH 3/3] cris: fix a segfault if pflash drive not found

Quoted 6 lines · Anthony Liguori

Quoted 3 lines · Mark McLoughlin
drive_get_index() returns -1 if a drive isn't found; don't use -1 to index drives_table.
Looks fine to me. Edgar?

Look OK, I'll apply it later tonight.

Thanks

Quoted 28 lines

Regards,

Anthony Liguori

Quoted 21 lines

Signed-off-by Mark McLoughlin <[email protected]>

---
 hw/etraxfs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

hw/etraxfs.c +1 -1

index 4f93cd8..2f6e2ee 100644
@@ -83,7 +83,7 @@ void bareetraxfs_init (ram_addr_t ram_size, int 
 vga_ram_size,
     phys_flash = qemu_ram_alloc(FLASH_SIZE);
     i = drive_get_index(IF_PFLASH, 0, 0);
     pflash_cfi02_register(0x0, phys_flash,
-                          drives_table[i].bdrv, (64 * 1024),
+                          i != -1 ? drives_table[i].bdrv : NULL, (64 * 
 1024),
                           FLASH_SIZE >> 16,

                           1, 2, 0x0000, 0x0000, 0x0000, 0x0000,
                           0x555, 0x2aa);

Jan Kiszka

2008-11-13 17:11 UTC #5 lore raw

[Qemu-devel] Re: [PATCH 2/3] dyngen: fix some warnings about unused functions

In reply to Mark McLoughlin

Quoted 17 lines · Mark McLoughlin

Warnings were:
dyngen.c:292: error: ‘pstrcpy’ defined but not used
Only used with CONFIG_FORMAT_COFF
dyngen.c:319: error: ‘swab32ss’ defined but not used
With ELF_USES_RELOCA, either swab32ss() or swab64ss() will be used depending on ELF_CLASS. Easier to just mark them both as unused.
dyngen.c:334: error: ‘get16’ defined but not used dyngen.c:343: error: ‘get32’ defined but not used dyngen.c:352: error: ‘put16’ defined but not used dyngen.c:359: error: ‘put32’ defined but not used

Quoted 6 lines

Maybe be used for some host arches; easiest to mark them as unused.
dyngen.c:425: error: ‘elf_swap_phdr’ defined but not used
This one is completely unused; just delete.

As far as I understood, the whole file will disappear soon (once ongoing PPC conversion to TCG is completed). I gave up hunting after the dyngen warning mess for this reason.

However, welcome to the club of warning fighters! Hope you'll have more luck than others (including me). :)

Jan

Signature · 2 lines

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

Anthony Liguori

2008-11-13 19:39 UTC #6 lore raw

[Qemu-devel] Re: [PATCH 2/3] dyngen: fix some warnings about unused functions

In reply to Mark McLoughlin

Quoted 17 lines · Mark McLoughlin

Warnings were:
dyngen.c:292: error: ‘pstrcpy’ defined but not used
Only used with CONFIG_FORMAT_COFF
dyngen.c:319: error: ‘swab32ss’ defined but not used
With ELF_USES_RELOCA, either swab32ss() or swab64ss() will be used depending on ELF_CLASS. Easier to just mark them both as unused.
dyngen.c:334: error: ‘get16’ defined but not used dyngen.c:343: error: ‘get32’ defined but not used dyngen.c:352: error: ‘put16’ defined but not used dyngen.c:359: error: ‘put32’ defined but not used

Quoted 7 lines

Maybe be used for some host arches; easiest to mark them as unused.
dyngen.c:425: error: ‘elf_swap_phdr’ defined but not used
This one is completely unused; just delete.

Since dyngen is going away soon, I'm not eager to make changes to it that could possibly break on some weird architecture. Do you think it would be terrible to just ignore this for now?

If someone else is more confident that this is okay, I think it's fine to commit.

Regards,

Anthony Liguori

Mark McLoughlin

author

2008-11-14 15:26 UTC #7 lore raw

[Qemu-devel] Re: [PATCH 2/3] dyngen: fix some warnings about unused functions

Quoted 21 lines · Anthony Liguori

Quoted 20 lines · Mark McLoughlin
Warnings were:
dyngen.c:292: error: ‘pstrcpy’ defined but not used
Only used with CONFIG_FORMAT_COFF
dyngen.c:319: error: ‘swab32ss’ defined but not used
With ELF_USES_RELOCA, either swab32ss() or swab64ss() will be used depending on ELF_CLASS. Easier to just mark them both as unused.
dyngen.c:334: error: ‘get16’ defined but not used dyngen.c:343: error: ‘get32’ defined but not used dyngen.c:352: error: ‘put16’ defined but not used dyngen.c:359: error: ‘put32’ defined but not used
Maybe be used for some host arches; easiest to mark them as unused.

Quoted 8 lines

Quoted 4 lines
dyngen.c:425: error: ‘elf_swap_phdr’ defined but not used
This one is completely unused; just delete.
Since dyngen is going away soon, I'm not eager to make changes to it that could possibly break on some weird architecture. Do you think it would be terrible to just ignore this for now?

Yeah, that's fine - I was trying to make it possible to build with --enable-werror to so it would be easier to spot new warnings.

The tcg 'TARGET_LONG_BITS is not defined' warning seems much more nasty to solve anyway.

Well, slirp too - but you can build with --disable-slirp to get around that.

Cheers,
Mark.

Jan Kiszka

2008-11-13 17:10 UTC #8 lore raw

In reply to Mark McLoughlin

Quoted 27 lines · Mark McLoughlin

Warning is:

target-i386/helper.c: In function `cpu_x86_cpuid':
target-i386/helper.c:1373: warning: implicit declaration of function `host_cpuid'

Signed-off-by Mark McLoughlin <[email protected]>

---
 target-i386/helper.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

target-i386/helper.c +1 -1

index 7ddb4ce..c8b8be9 100644
@@ -1296,10 +1296,10 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState *env, target_ulong addr)
 }
 #endif /* !CONFIG_USER_ONLY */
 
-#if defined(CONFIG_KVM)
 static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t *ebx,
                        uint32_t *ecx, uint32_t *edx)
 {
+#if defined(CONFIG_KVM)
     uint32_t vec[4];
 
 #ifdef __x86_64__

target-i386/helper.c static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t *ebx, · line 1331

Quoted 6 lines

target-i386/helper.c +1 -1

@@ -1327,8 +1327,8 @@ static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t *ebx,
 	*ecx = vec[2];
     if (edx)
 	*edx = vec[3];
-}
 #endif
+}

Isn't it cleaner to provide an empty static inline in the !CONFIG_KVM case? Not all compilers might be smart enough to optimize this function away.

Jan

Signature · 2 lines

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

Anthony Liguori

2008-11-13 19:31 UTC #9 lore raw

Quoted 4 lines · Jan Kiszka

Isn't it cleaner to provide an empty static inline in the !CONFIG_KVM case? Not all compilers might be smart enough to optimize this function away.

kvm_enabled() is (0) when !CONFIG_KVM so that's why this is a warning and not an error. The function call disappears during linking from dead code elimination. There is no routine overhead when !CONFIG_KVM.

Regards,

Anthony Liguori

Quoted 3 lines

Jan

Jamie Lokier

2008-11-14 03:22 UTC #10 lore raw

Quoted 8 lines · Anthony Liguori

Quoted 3 lines · Jan Kiszka
Isn't it cleaner to provide an empty static inline in the !CONFIG_KVM case? Not all compilers might be smart enough to optimize this function away.
kvm_enabled() is (0) when !CONFIG_KVM so that's why this is a warning and not an error. The function call disappears during linking from dead code elimination. There is no routine overhead when !CONFIG_KVM.

I think it's in reference to:

Quoted 2 lines

static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t *ebx,
                        uint32_t *ecx, uint32_t *edx)

This compiles to an empty function if !CONFIG_KVM. Current GCC will remove the function call by auto-inlining, but some compilers (including older GCC) will produce an empty function.

If you're saying !CONFIG_KVM results in the _call_ to host_cpuid() being removed, I'm thinking that the option to provide the guest with the same features as the host CPU is still useful if KVM is disabled... I have a real use-case for this. It's installing some versions of Windows in a guest, to be used with KVM after installation, because the install process is more reliable (and faster!) with KVM disabled but after that it works.

-- Jamie

Anthony Liguori

Applied

2008-11-13 19:38 UTC #11 lore raw

In reply to Mark McLoughlin

Quoted 7 lines · Mark McLoughlin

Warning is:
target-i386/helper.c: In function `cpu_x86_cpuid':
target-i386/helper.c:1373: warning: implicit declaration of function `host_cpuid'
Signed-off-by Mark McLoughlin <[email protected]>

Applied. Thanks.

Regards,

Anthony Liguori