* arch/x86/kvm/svm.c:3600: possible bad operator ?
@ 2016-05-23 11:22 David Binderman
2016-05-23 11:42 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: David Binderman @ 2016-05-23 11:22 UTC (permalink / raw)
To: joro, pbonzini, rkrcmar, kvm, dcb314
Hello there,
arch/x86/kvm/svm.c:3600:45: warning: logical ‘and’ applied to
non-boolean constant [-Wlogical-op]
Source code is
u32 index = svm->vmcb->control.exit_info_2 && 0xFF;
Maybe better code
u32 index = svm->vmcb->control.exit_info_2 & 0xFF;
Regards
David Binderman
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: arch/x86/kvm/svm.c:3600: possible bad operator ?
2016-05-23 11:22 arch/x86/kvm/svm.c:3600: possible bad operator ? David Binderman
@ 2016-05-23 11:42 ` Paolo Bonzini
2016-05-23 14:52 ` Thomas Huth
0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2016-05-23 11:42 UTC (permalink / raw)
To: David Binderman, joro, rkrcmar, kvm, dcb314
On 23/05/2016 13:22, David Binderman wrote:
> Hello there,
>
> arch/x86/kvm/svm.c:3600:45: warning: logical ‘and’ applied to
> non-boolean constant [-Wlogical-op]
>
> Source code is
>
> u32 index = svm->vmcb->control.exit_info_2 && 0xFF;
>
> Maybe better code
>
> u32 index = svm->vmcb->control.exit_info_2 & 0xFF;
Yes, there's a bug there. However it's only affecting a tracepoint,
which is why it probably wasn't noticed.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: arch/x86/kvm/svm.c:3600: possible bad operator ?
2016-05-23 11:42 ` Paolo Bonzini
@ 2016-05-23 14:52 ` Thomas Huth
2016-05-23 15:05 ` David Binderman
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2016-05-23 14:52 UTC (permalink / raw)
To: Paolo Bonzini, David Binderman, joro, rkrcmar, kvm, dcb314,
Suravee Suthikulpanit
On 23.05.2016 13:42, Paolo Bonzini wrote:
>
> On 23/05/2016 13:22, David Binderman wrote:
>> Hello there,
>>
>> arch/x86/kvm/svm.c:3600:45: warning: logical ‘and’ applied to
>> non-boolean constant [-Wlogical-op]
>>
>> Source code is
>>
>> u32 index = svm->vmcb->control.exit_info_2 && 0xFF;
>>
>> Maybe better code
>>
>> u32 index = svm->vmcb->control.exit_info_2 & 0xFF;
>
> Yes, there's a bug there. However it's only affecting a tracepoint,
> which is why it probably wasn't noticed.
This define in the same file also looks quite suspicious:
#define AVIC_HPA_MASK ~((0xFFFULL << 52) || 0xFFF)
Strange that you did not get a warning on that one, too?
Thomas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: arch/x86/kvm/svm.c:3600: possible bad operator ?
2016-05-23 14:52 ` Thomas Huth
@ 2016-05-23 15:05 ` David Binderman
2016-05-23 15:11 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: David Binderman @ 2016-05-23 15:05 UTC (permalink / raw)
To: Thomas Huth
Cc: Paolo Bonzini, joro, rkrcmar, kvm, dcb314, Suravee Suthikulpanit
Hello there,
On Mon, May 23, 2016 at 3:52 PM, Thomas Huth <thuth@redhat.com> wrote:
> This define in the same file also looks quite suspicious:
>
> #define AVIC_HPA_MASK ~((0xFFFULL << 52) || 0xFFF)
>
> Strange that you did not get a warning on that one, too?
I did, but the warnings were buried in a morass of other warnings, so
I didn't notice it ;-<
[linux-next/arch/x86/kvm/svm.c:1075]: (style) Boolean result is used
in bitwise operation. Clarify expression with parentheses.
[linux-next/arch/x86/kvm/svm.c:1076]: (style) Boolean result is used
in bitwise operation. Clarify expression with parentheses.
[linux-next/arch/x86/kvm/svm.c:1077]: (style) Boolean result is used
in bitwise operation. Clarify expression with parentheses.
Three warnings, one for each use of the macro AVIC_HPA_MASK.
My apologies for not spotting this sooner. In order to avoid further
public embarrassment, here are a couple of other minor things I
found in x86/kvm/*.c, which might be worth fixing.
arch/x86/kvm/cpuid.c:867]: (style) Variable 'function' is assigned a
value that is never used.
arch/x86/kvm/x86.c:2142]: (style) Variable 'gpa_offset' is assigned a
value that is never used.
Regards
David Binderman
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: arch/x86/kvm/svm.c:3600: possible bad operator ?
2016-05-23 15:05 ` David Binderman
@ 2016-05-23 15:11 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2016-05-23 15:11 UTC (permalink / raw)
To: David Binderman, Thomas Huth
Cc: joro, rkrcmar, kvm, dcb314, Suravee Suthikulpanit
On 23/05/2016 17:05, David Binderman wrote:
> Hello there,
>
> On Mon, May 23, 2016 at 3:52 PM, Thomas Huth <thuth@redhat.com> wrote:
>> This define in the same file also looks quite suspicious:
>>
>> #define AVIC_HPA_MASK ~((0xFFFULL << 52) || 0xFFF)
>>
>> Strange that you did not get a warning on that one, too?
>
> I did, but the warnings were buried in a morass of other warnings, so
> I didn't notice it ;-<
>
> [linux-next/arch/x86/kvm/svm.c:1075]: (style) Boolean result is used
> in bitwise operation. Clarify expression with parentheses.
> [linux-next/arch/x86/kvm/svm.c:1076]: (style) Boolean result is used
> in bitwise operation. Clarify expression with parentheses.
> [linux-next/arch/x86/kvm/svm.c:1077]: (style) Boolean result is used
> in bitwise operation. Clarify expression with parentheses.
>
> Three warnings, one for each use of the macro AVIC_HPA_MASK.
Thanks, I'll squash these in:
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 2eb615e400e3..1163e8173e5a 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -84,7 +84,7 @@ MODULE_DEVICE_TABLE(x86cpu, svm_cpu_id);
#define TSC_RATIO_MIN 0x0000000000000001ULL
#define TSC_RATIO_MAX 0x000000ffffffffffULL
-#define AVIC_HPA_MASK ~((0xFFFULL << 52) || 0xFFF)
+#define AVIC_HPA_MASK ~((0xFFFULL << 52) | 0xFFF)
/*
* 0xff is broadcast, so the max index allowed for physical APIC ID
Paolo
> My apologies for not spotting this sooner. In order to avoid further
> public embarrassment, here are a couple of other minor things I
> found in x86/kvm/*.c, which might be worth fixing.
>
> arch/x86/kvm/cpuid.c:867]: (style) Variable 'function' is assigned a
> value that is never used.
> arch/x86/kvm/x86.c:2142]: (style) Variable 'gpa_offset' is assigned a
> value that is never used.
>
>
> Regards
>
> David Binderman
>
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-23 15:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-23 11:22 arch/x86/kvm/svm.c:3600: possible bad operator ? David Binderman
2016-05-23 11:42 ` Paolo Bonzini
2016-05-23 14:52 ` Thomas Huth
2016-05-23 15:05 ` David Binderman
2016-05-23 15:11 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.