* [Qemu-devel] [PATCH] kvm: Fix enable_cap helpers on older gcc
@ 2014-05-11 16:11 Alexander Graf
2014-05-12 7:33 ` Thomas Huth
0 siblings, 1 reply; 3+ messages in thread
From: Alexander Graf @ 2014-05-11 16:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck, Thomas Huth
Commit 40f1ee27aa1 introduced handy helpers for enable_cap calls on
vcpu and vm level. Unfortunately some older gcc versions (4.7.1, 4.6)
seem to choke on signedness detection in inline created variables:
target-ppc/kvm.c: In function 'kvmppc_booke_watchdog_enable':
target-ppc/kvm.c:1302:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
target-ppc/kvm.c: In function 'kvmppc_set_papr':
target-ppc/kvm.c:1504:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
We can easily work around this by not doing a compare with 0, but instead
shift all values up by one. The resulting binary should look the same, as
all those ARRAY_SIZE values are compile time statically optimized.
This patch fixes compile errors on some of my systems.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
include/sysemu/kvm.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 192fe89..989c261 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -302,9 +302,9 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
}; \
uint64_t args_tmp[] = { __VA_ARGS__ }; \
int i; \
- for (i = 0; i < ARRAY_SIZE(args_tmp) && \
- i < ARRAY_SIZE(cap.args); i++) { \
- cap.args[i] = args_tmp[i]; \
+ for (i = 1; i < (MIN(ARRAY_SIZE(args_tmp), \
+ ARRAY_SIZE(cap.args)) + 1); i++) { \
+ cap.args[i - 1] = args_tmp[i - 1]; \
} \
kvm_vm_ioctl(s, KVM_ENABLE_CAP, &cap); \
})
@@ -317,9 +317,9 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
}; \
uint64_t args_tmp[] = { __VA_ARGS__ }; \
int i; \
- for (i = 0; i < ARRAY_SIZE(args_tmp) && \
- i < ARRAY_SIZE(cap.args); i++) { \
- cap.args[i] = args_tmp[i]; \
+ for (i = 1; i < (MIN(ARRAY_SIZE(args_tmp), \
+ ARRAY_SIZE(cap.args)) + 1); i++) { \
+ cap.args[i - 1] = args_tmp[i - 1]; \
} \
kvm_vcpu_ioctl(cpu, KVM_ENABLE_CAP, &cap); \
})
--
1.8.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] kvm: Fix enable_cap helpers on older gcc
2014-05-11 16:11 [Qemu-devel] [PATCH] kvm: Fix enable_cap helpers on older gcc Alexander Graf
@ 2014-05-12 7:33 ` Thomas Huth
2014-05-12 8:15 ` Cornelia Huck
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Huth @ 2014-05-12 7:33 UTC (permalink / raw)
To: Alexander Graf; +Cc: Cornelia Huck, qemu-devel
On Sun, 11 May 2014 18:11:04 +0200
Alexander Graf <agraf@suse.de> wrote:
> Commit 40f1ee27aa1 introduced handy helpers for enable_cap calls on
> vcpu and vm level. Unfortunately some older gcc versions (4.7.1, 4.6)
> seem to choke on signedness detection in inline created variables:
>
> target-ppc/kvm.c: In function 'kvmppc_booke_watchdog_enable':
> target-ppc/kvm.c:1302:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
> target-ppc/kvm.c: In function 'kvmppc_set_papr':
> target-ppc/kvm.c:1504:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>
> We can easily work around this by not doing a compare with 0, but instead
> shift all values up by one. The resulting binary should look the same, as
> all those ARRAY_SIZE values are compile time statically optimized.
>
> This patch fixes compile errors on some of my systems.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
> include/sysemu/kvm.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 192fe89..989c261 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -302,9 +302,9 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
> }; \
> uint64_t args_tmp[] = { __VA_ARGS__ }; \
> int i; \
> - for (i = 0; i < ARRAY_SIZE(args_tmp) && \
> - i < ARRAY_SIZE(cap.args); i++) { \
> - cap.args[i] = args_tmp[i]; \
> + for (i = 1; i < (MIN(ARRAY_SIZE(args_tmp), \
> + ARRAY_SIZE(cap.args)) + 1); i++) { \
> + cap.args[i - 1] = args_tmp[i - 1]; \
> } \
> kvm_vm_ioctl(s, KVM_ENABLE_CAP, &cap); \
> })
> @@ -317,9 +317,9 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
> }; \
> uint64_t args_tmp[] = { __VA_ARGS__ }; \
> int i; \
> - for (i = 0; i < ARRAY_SIZE(args_tmp) && \
> - i < ARRAY_SIZE(cap.args); i++) { \
> - cap.args[i] = args_tmp[i]; \
> + for (i = 1; i < (MIN(ARRAY_SIZE(args_tmp), \
> + ARRAY_SIZE(cap.args)) + 1); i++) { \
> + cap.args[i - 1] = args_tmp[i - 1]; \
> } \
> kvm_vcpu_ioctl(cpu, KVM_ENABLE_CAP, &cap); \
> })
That looks ... somewhat ugly. I think you either should add a proper
comment here (or somebody will fix it back in the future when reading
the code), ... or would it also work to type-cast the result of
ARRAY_SIZE to "int"? Something like this:
for (i = 0; i < (int)ARRAY_SIZE(args_tmp) && \
i < (int)ARRAY_SIZE(cap.args); i++)
Thomas
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] kvm: Fix enable_cap helpers on older gcc
2014-05-12 7:33 ` Thomas Huth
@ 2014-05-12 8:15 ` Cornelia Huck
0 siblings, 0 replies; 3+ messages in thread
From: Cornelia Huck @ 2014-05-12 8:15 UTC (permalink / raw)
To: Thomas Huth; +Cc: Alexander Graf, qemu-devel
On Mon, 12 May 2014 09:33:39 +0200
Thomas Huth <thuth@linux.vnet.ibm.com> wrote:
> On Sun, 11 May 2014 18:11:04 +0200
> Alexander Graf <agraf@suse.de> wrote:
>
> > Commit 40f1ee27aa1 introduced handy helpers for enable_cap calls on
> > vcpu and vm level. Unfortunately some older gcc versions (4.7.1, 4.6)
> > seem to choke on signedness detection in inline created variables:
> >
> > target-ppc/kvm.c: In function 'kvmppc_booke_watchdog_enable':
> > target-ppc/kvm.c:1302:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
> > target-ppc/kvm.c: In function 'kvmppc_set_papr':
> > target-ppc/kvm.c:1504:21: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
> >
> > We can easily work around this by not doing a compare with 0, but instead
> > shift all values up by one. The resulting binary should look the same, as
> > all those ARRAY_SIZE values are compile time statically optimized.
> >
> > This patch fixes compile errors on some of my systems.
> >
> > Signed-off-by: Alexander Graf <agraf@suse.de>
> > ---
> > include/sysemu/kvm.h | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> > index 192fe89..989c261 100644
> > --- a/include/sysemu/kvm.h
> > +++ b/include/sysemu/kvm.h
> > @@ -302,9 +302,9 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
> > }; \
> > uint64_t args_tmp[] = { __VA_ARGS__ }; \
> > int i; \
> > - for (i = 0; i < ARRAY_SIZE(args_tmp) && \
> > - i < ARRAY_SIZE(cap.args); i++) { \
> > - cap.args[i] = args_tmp[i]; \
> > + for (i = 1; i < (MIN(ARRAY_SIZE(args_tmp), \
> > + ARRAY_SIZE(cap.args)) + 1); i++) { \
> > + cap.args[i - 1] = args_tmp[i - 1]; \
> > } \
> > kvm_vm_ioctl(s, KVM_ENABLE_CAP, &cap); \
> > })
> > @@ -317,9 +317,9 @@ int kvm_check_extension(KVMState *s, unsigned int extension);
> > }; \
> > uint64_t args_tmp[] = { __VA_ARGS__ }; \
> > int i; \
> > - for (i = 0; i < ARRAY_SIZE(args_tmp) && \
> > - i < ARRAY_SIZE(cap.args); i++) { \
> > - cap.args[i] = args_tmp[i]; \
> > + for (i = 1; i < (MIN(ARRAY_SIZE(args_tmp), \
> > + ARRAY_SIZE(cap.args)) + 1); i++) { \
> > + cap.args[i - 1] = args_tmp[i - 1]; \
> > } \
> > kvm_vcpu_ioctl(cpu, KVM_ENABLE_CAP, &cap); \
> > })
>
> That looks ... somewhat ugly. I think you either should add a proper
> comment here (or somebody will fix it back in the future when reading
> the code), ... or would it also work to type-cast the result of
> ARRAY_SIZE to "int"? Something like this:
>
> for (i = 0; i < (int)ARRAY_SIZE(args_tmp) && \
> i < (int)ARRAY_SIZE(cap.args); i++)
If that one works, I'd prefer that. The other version hurts my eyes :(
If it doesn't, I agree it should be properly commented.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-05-12 8:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-11 16:11 [Qemu-devel] [PATCH] kvm: Fix enable_cap helpers on older gcc Alexander Graf
2014-05-12 7:33 ` Thomas Huth
2014-05-12 8:15 ` Cornelia Huck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).