Skip to contentSkip to conversation

ARM64: include: asm: use 'int' instead of 'u32' for variable 'target' in kvm_host.h.

PATCH In review Chen Gang 3 participants 5 messages Jul 22, 2013 – Jul 30, 2013

Subsystems (3)
KVM arm64 KVM arm64
lore
Share & tools
lore

Chen Gang

author
#0 lore raw

PATCH 1/1 ARM64: include: asm: use 'int' instead of 'u32' for variable 'target' in kvm_host.h.

'target' will be set to '-1' in kvm_arch_vcpu_init(), and it need check 'target' whether less than zero or not in kvm_vcpu_initialized().

So need define target as 'int' instead of 'u32', just like ARM has done.

The related warning:

arch/arm64/kvm/../../../arch/arm/kvm/arm.c:497:2: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]

Signed-off-by Chen Gang <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm64/include/asm/kvm_host.h +1 -1
index 644d739..0859a4d 100644
@@ -129,7 +129,7 @@ struct kvm_vcpu_arch {
 	struct kvm_mmu_memory_cache mmu_page_cache;
 
 	/* Target CPU and feature flags */
-	u32 target;
+	int target;
 	DECLARE_BITMAP(features, KVM_VCPU_MAX_FEATURES);
 
 	/* Detect first run of a vcpu */
Signature · 1 line
1.7.7.6
author
#1 lore raw

Hello Maintainers:

Please help check the patch whether OK or not, when you have time.

Thanks.

Quoted 29 lines · Chen Gang

'target' will be set to '-1' in kvm_arch_vcpu_init(), and it need check 'target' whether less than zero or not in kvm_vcpu_initialized().

So need define target as 'int' instead of 'u32', just like ARM has done.

The related warning:

arch/arm64/kvm/../../../arch/arm/kvm/arm.c:497:2: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]

Signed-off-by Chen Gang <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm64/include/asm/kvm_host.h +1 -1
index 644d739..0859a4d 100644
@@ -129,7 +129,7 @@ struct kvm_vcpu_arch {
 	struct kvm_mmu_memory_cache mmu_page_cache;
 
 	/* Target CPU and feature flags */
-	u32 target;
+	int target;
 	DECLARE_BITMAP(features, KVM_VCPU_MAX_FEATURES);
 
 	/* Detect first run of a vcpu */
Signature · 1 line
Chen Gang
#2 lore raw
In reply to Chen Gang
Quoted 30 lines · Chen Gang

'target' will be set to '-1' in kvm_arch_vcpu_init(), and it need check 'target' whether less than zero or not in kvm_vcpu_initialized().

So need define target as 'int' instead of 'u32', just like ARM has done.

The related warning:

arch/arm64/kvm/../../../arch/arm/kvm/arm.c:497:2: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]

Signed-off-by Chen Gang <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm64/include/asm/kvm_host.h +1 -1
index 644d739..0859a4d 100644
@@ -129,7 +129,7 @@ struct kvm_vcpu_arch {
 	struct kvm_mmu_memory_cache mmu_page_cache;
 
 	/* Target CPU and feature flags */
-	u32 target;
+	int target;
 	DECLARE_BITMAP(features, KVM_VCPU_MAX_FEATURES);
 
 	/* Detect first run of a vcpu */
Signature · 1 line
1.7.7.6

This looks ok to me, Marc, was there any reason to declare target a u32 on arm64?

-Christoffer

#3 lore raw
Quoted 28 lines · Christoffer Dall
Quoted 27 lines · Chen Gang

'target' will be set to '-1' in kvm_arch_vcpu_init(), and it need check 'target' whether less than zero or not in kvm_vcpu_initialized().

So need define target as 'int' instead of 'u32', just like ARM has done.

The related warning:

arch/arm64/kvm/../../../arch/arm/kvm/arm.c:497:2: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]

Signed-off-by Chen Gang <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm64/include/asm/kvm_host.h +1 -1
index 644d739..0859a4d 100644
@@ -129,7 +129,7 @@ struct kvm_vcpu_arch {
 	struct kvm_mmu_memory_cache mmu_page_cache;
 
 	/* Target CPU and feature flags */
-	u32 target;
+	int target;
 	DECLARE_BITMAP(features, KVM_VCPU_MAX_FEATURES);
Quoted 6 lines
Quoted 3 lines
arch/arm64/include/asm/kvm_host.h +0 -0
 	/* Detect first run of a vcpu */
Signature · 1 line
1.7.7.6

This looks ok to me, Marc, was there any reason to declare target a u32 on arm64?

No, that's simply a bug. I really wonder why my compiler doesn't scream at something that obvious...

Chen: what compiler version are you using?

I'll queue this fix.

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
author
#4 lore raw
Quoted 36 lines · Marc Zyngier
Quoted 28 lines · Christoffer Dall
Quoted 27 lines · Chen Gang

'target' will be set to '-1' in kvm_arch_vcpu_init(), and it need check 'target' whether less than zero or not in kvm_vcpu_initialized().

So need define target as 'int' instead of 'u32', just like ARM has done.

The related warning:

arch/arm64/kvm/../../../arch/arm/kvm/arm.c:497:2: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]

Signed-off-by Chen Gang <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm64/include/asm/kvm_host.h +1 -1
index 644d739..0859a4d 100644
@@ -129,7 +129,7 @@ struct kvm_vcpu_arch {
 	struct kvm_mmu_memory_cache mmu_page_cache;
 
 	/* Target CPU and feature flags */
-	u32 target;
+	int target;
 	DECLARE_BITMAP(features, KVM_VCPU_MAX_FEATURES);
Quoted 6 lines
Quoted 3 lines
arch/arm64/include/asm/kvm_host.h +0 -0
 	/* Detect first run of a vcpu */
Signature · 1 line
1.7.7.6

This looks ok to me, Marc, was there any reason to declare target a u32 on arm64?

Quoted 3 lines

No, that's simply a bug. I really wonder why my compiler doesn't scream at something that obvious...

I make with "EXTRA_CFLAGS=-W", normal make will not report this warning. ;-)

Quoted 2 lines

Chen: what compiler version are you using?

[root at gchenlinux linux-next]# aarch64-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=aarch64-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/usr/bin/../libexec/gcc/aarch64-linux-gnu/4.7.3/lto-wrapper
Target: aarch64-linux-gnu
Configured with: /cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/src/gcc-linaro-4.7-2013.02/configure --build=i686-build_pc-linux-gnu --host=i686-build_pc-linux-gnu --target=aarch64-linux-gnu --prefix=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/install --with-sysroot=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/install/aarch64-linux-gnu/libc --enable-languages=c,c++,fortran --disable-multilib --with-pkgversion='crosstool-NG linaro-1.13.1-4.7-2013.02-01-20130221 - ARM aarch64 via Linaro' --with-bugurl=https://bugs.launchpad.net/gcc-linaro --enable-__cxa_atexit --disable-libmudflap --disable-libgomp --disable-libssp --with-gmp=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static --with-mpfr=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static --with-mpc=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linu
x/.build/
aarch64-linux-gnu/build/static --with-ppl=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static --with-cloog=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static --with-libelf=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static --with-host-libstdcxx='-L/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/.build/aarch64-linux-gnu/build/static/lib -lpwl' --enable-threads=posix --disable-libstdcxx-pch --enable-linker-build-id --enable-plugin --with-local-prefix=/cbuild/slaves/oorts/crosstool-ng/builds/aarch64-linux-gnu-linux/install/aarch64-linux-gnu/libc --enable-c99 --enable-long-long
Thread model: posix
gcc version 4.7.3 20130205 (prerelease) (crosstool-NG linaro-1.13.1-4.7-2013.02-01-20130221 - ARM aarch64 via Linaro)

Quoted 6 lines

I'll queue this fix.

Thanks,

	M.

Thank you too.

Signature · 1 line
Chen Gang