All of lore.kernel.org
 help / color / mirror / Atom feed
* [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
@ 2021-10-04  9:08 torvic9
  2021-10-04  9:26 ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: torvic9 @ 2021-10-04  9:08 UTC (permalink / raw)
  To: pbonzini, seanjc, vkuznets, kvm; +Cc: linux-kernel, tglx, bp

I encounter the following issue when compiling 5.15-rc4 with clang-14:

In file included from arch/x86/kvm/mmu/mmu.c:27:
arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
        return __is_bad_mt_xwr(rsvd_check, spte) |
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                                 ||
arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning

(no issue with gcc-11)

Tor

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-04  9:08 [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14 torvic9
@ 2021-10-04  9:26 ` Paolo Bonzini
  2021-10-04  9:30   ` torvic9
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2021-10-04  9:26 UTC (permalink / raw)
  To: torvic9, seanjc, vkuznets, kvm; +Cc: linux-kernel, tglx, bp

On 04/10/21 11:08, torvic9@mailbox.org wrote:
> I encounter the following issue when compiling 5.15-rc4 with clang-14:
> 
> In file included from arch/x86/kvm/mmu/mmu.c:27:
> arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
>          return __is_bad_mt_xwr(rsvd_check, spte) |
>                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>                                                   ||
> arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning

The warning is wrong, as mentioned in the line right above:

         /*
          * Use a bitwise-OR instead of a logical-OR to aggregate the reserved
          * bits and EPT's invalid memtype/XWR checks to avoid an extra Jcc
          * (this is extremely unlikely to be short-circuited as true).
          */

Paolo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-04  9:26 ` Paolo Bonzini
@ 2021-10-04  9:30   ` torvic9
  2021-10-04  9:49     ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: torvic9 @ 2021-10-04  9:30 UTC (permalink / raw)
  To: Paolo Bonzini, seanjc, vkuznets, kvm, ndesaulniers, nathan
  Cc: linux-kernel, tglx, bp


> Paolo Bonzini <pbonzini@redhat.com> hat am 04.10.2021 11:26 geschrieben:
> 
>  
> On 04/10/21 11:08, torvic9@mailbox.org wrote:
> > I encounter the following issue when compiling 5.15-rc4 with clang-14:
> > 
> > In file included from arch/x86/kvm/mmu/mmu.c:27:
> > arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
> >          return __is_bad_mt_xwr(rsvd_check, spte) |
> >                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >                                                   ||
> > arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning
> 
> The warning is wrong, as mentioned in the line right above:

So it's an issue with clang-14 then?
(I add Nick and Nathan)

> 
>          /*
>           * Use a bitwise-OR instead of a logical-OR to aggregate the reserved
>           * bits and EPT's invalid memtype/XWR checks to avoid an extra Jcc
>           * (this is extremely unlikely to be short-circuited as true).
>           */
> 
> Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-04  9:30   ` torvic9
@ 2021-10-04  9:49     ` Paolo Bonzini
  2021-10-04 10:10       ` torvic9
  2021-10-04 16:13       ` Nick Desaulniers
  0 siblings, 2 replies; 13+ messages in thread
From: Paolo Bonzini @ 2021-10-04  9:49 UTC (permalink / raw)
  To: torvic9, seanjc, vkuznets, kvm, ndesaulniers, nathan
  Cc: linux-kernel, tglx, bp

On 04/10/21 11:30, torvic9@mailbox.org wrote:
> 
>> Paolo Bonzini <pbonzini@redhat.com> hat am 04.10.2021 11:26 geschrieben:
>>
>>   
>> On 04/10/21 11:08, torvic9@mailbox.org wrote:
>>> I encounter the following issue when compiling 5.15-rc4 with clang-14:
>>>
>>> In file included from arch/x86/kvm/mmu/mmu.c:27:
>>> arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
>>>           return __is_bad_mt_xwr(rsvd_check, spte) |
>>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>                                                    ||
>>> arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning
>>
>> The warning is wrong, as mentioned in the line right above:
> 
> So it's an issue with clang-14 then?
> (I add Nick and Nathan)

My clang here doesn't have the option, so I'm going to ask---are you 
using W=1?  I can see why clang is warning for KVM's code, but in my 
opinion such a check should only be in -Wextra.

Paolo

>>
>>           /*
>>            * Use a bitwise-OR instead of a logical-OR to aggregate the reserved
>>            * bits and EPT's invalid memtype/XWR checks to avoid an extra Jcc
>>            * (this is extremely unlikely to be short-circuited as true).
>>            */
>>
>> Paolo
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-04  9:49     ` Paolo Bonzini
@ 2021-10-04 10:10       ` torvic9
  2021-10-04 14:33         ` torvic9
  2021-10-04 16:13       ` Nick Desaulniers
  1 sibling, 1 reply; 13+ messages in thread
From: torvic9 @ 2021-10-04 10:10 UTC (permalink / raw)
  To: Paolo Bonzini, seanjc, vkuznets, kvm, ndesaulniers, nathan
  Cc: linux-kernel, tglx, bp


> Paolo Bonzini <pbonzini@redhat.com> hat am 04.10.2021 11:49 geschrieben:
> 
>  
> On 04/10/21 11:30, torvic9@mailbox.org wrote:
> > 
> >> Paolo Bonzini <pbonzini@redhat.com> hat am 04.10.2021 11:26 geschrieben:
> >>
> >>   
> >> On 04/10/21 11:08, torvic9@mailbox.org wrote:
> >>> I encounter the following issue when compiling 5.15-rc4 with clang-14:
> >>>
> >>> In file included from arch/x86/kvm/mmu/mmu.c:27:
> >>> arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
> >>>           return __is_bad_mt_xwr(rsvd_check, spte) |
> >>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>                                                    ||
> >>> arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning
> >>
> >> The warning is wrong, as mentioned in the line right above:
> > 
> > So it's an issue with clang-14 then?
> > (I add Nick and Nathan)
> 
> My clang here doesn't have the option, so I'm going to ask---are you 
> using W=1?  I can see why clang is warning for KVM's code, but in my 
> opinion such a check should only be in -Wextra.

I don't use any options (not that I'm aware of).
Clang version 14.0.0 5f7a5353301b776ffb0e5fb048992898507bf7ee

> 
> Paolo
> 
> >>
> >>           /*
> >>            * Use a bitwise-OR instead of a logical-OR to aggregate the reserved
> >>            * bits and EPT's invalid memtype/XWR checks to avoid an extra Jcc
> >>            * (this is extremely unlikely to be short-circuited as true).
> >>            */
> >>
> >> Paolo
> >

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-04 10:10       ` torvic9
@ 2021-10-04 14:33         ` torvic9
  0 siblings, 0 replies; 13+ messages in thread
From: torvic9 @ 2021-10-04 14:33 UTC (permalink / raw)
  To: Paolo Bonzini, seanjc, vkuznets, kvm, ndesaulniers, nathan
  Cc: linux-kernel, tglx, bp


> torvic9@mailbox.org hat am 04.10.2021 12:10 geschrieben:
> 
>  
> > Paolo Bonzini <pbonzini@redhat.com> hat am 04.10.2021 11:49 geschrieben:
> > 
> >  
> > On 04/10/21 11:30, torvic9@mailbox.org wrote:
> > > 
> > >> Paolo Bonzini <pbonzini@redhat.com> hat am 04.10.2021 11:26 geschrieben:
> > >>
> > >>   
> > >> On 04/10/21 11:08, torvic9@mailbox.org wrote:
> > >>> I encounter the following issue when compiling 5.15-rc4 with clang-14:
> > >>>
> > >>> In file included from arch/x86/kvm/mmu/mmu.c:27:
> > >>> arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
> > >>>           return __is_bad_mt_xwr(rsvd_check, spte) |
> > >>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >>>                                                    ||
> > >>> arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning
> > >>
> > >> The warning is wrong, as mentioned in the line right above:
> > > 
> > > So it's an issue with clang-14 then?
> > > (I add Nick and Nathan)
> > 
> > My clang here doesn't have the option, so I'm going to ask---are you 
> > using W=1?  I can see why clang is warning for KVM's code, but in my 
> > opinion such a check should only be in -Wextra.
> 
> I don't use any options (not that I'm aware of).
> Clang version 14.0.0 5f7a5353301b776ffb0e5fb048992898507bf7ee

Probably the cause for this bug is this recent llvm commit:
https://github.com/llvm/llvm-project/commit/f59cc9542bfb461d16ad12b2cc4be4abbfd9d96e

> 
> > 
> > Paolo
> > 
> > >>
> > >>           /*
> > >>            * Use a bitwise-OR instead of a logical-OR to aggregate the reserved
> > >>            * bits and EPT's invalid memtype/XWR checks to avoid an extra Jcc
> > >>            * (this is extremely unlikely to be short-circuited as true).
> > >>            */
> > >>
> > >> Paolo
> > >

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-04  9:49     ` Paolo Bonzini
  2021-10-04 10:10       ` torvic9
@ 2021-10-04 16:13       ` Nick Desaulniers
  2021-10-04 17:12         ` Jim Mattson
  1 sibling, 1 reply; 13+ messages in thread
From: Nick Desaulniers @ 2021-10-04 16:13 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: torvic9, seanjc, vkuznets, kvm, nathan, linux-kernel, tglx, bp

On Mon, Oct 4, 2021 at 2:49 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 04/10/21 11:30, torvic9@mailbox.org wrote:
> >
> >> Paolo Bonzini <pbonzini@redhat.com> hat am 04.10.2021 11:26 geschrieben:
> >>
> >>
> >> On 04/10/21 11:08, torvic9@mailbox.org wrote:
> >>> I encounter the following issue when compiling 5.15-rc4 with clang-14:
> >>>
> >>> In file included from arch/x86/kvm/mmu/mmu.c:27:
> >>> arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
> >>>           return __is_bad_mt_xwr(rsvd_check, spte) |
> >>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>                                                    ||
> >>> arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning
> >>
> >> The warning is wrong, as mentioned in the line right above:
> >
> > So it's an issue with clang-14 then?
> > (I add Nick and Nathan)
>
> My clang here doesn't have the option, so I'm going to ask---are you
> using W=1?  I can see why clang is warning for KVM's code, but in my
> opinion such a check should only be in -Wextra.

This is a newly added warning in top of tree clang.

>
> Paolo
>
> >>
> >>           /*
> >>            * Use a bitwise-OR instead of a logical-OR to aggregate the reserved
> >>            * bits and EPT's invalid memtype/XWR checks to avoid an extra Jcc
> >>            * (this is extremely unlikely to be short-circuited as true).
> >>            */
> >>
> >> Paolo
> >
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-04 16:13       ` Nick Desaulniers
@ 2021-10-04 17:12         ` Jim Mattson
  2021-10-14 17:50           ` Nathan Chancellor
  0 siblings, 1 reply; 13+ messages in thread
From: Jim Mattson @ 2021-10-04 17:12 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Paolo Bonzini, torvic9, seanjc, vkuznets, kvm, nathan,
	linux-kernel, tglx, bp

On Mon, Oct 4, 2021 at 9:13 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Mon, Oct 4, 2021 at 2:49 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On 04/10/21 11:30, torvic9@mailbox.org wrote:
> > >
> > >> Paolo Bonzini <pbonzini@redhat.com> hat am 04.10.2021 11:26 geschrieben:
> > >>
> > >>
> > >> On 04/10/21 11:08, torvic9@mailbox.org wrote:
> > >>> I encounter the following issue when compiling 5.15-rc4 with clang-14:
> > >>>
> > >>> In file included from arch/x86/kvm/mmu/mmu.c:27:
> > >>> arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
> > >>>           return __is_bad_mt_xwr(rsvd_check, spte) |
> > >>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >>>                                                    ||
> > >>> arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning
> > >>
> > >> The warning is wrong, as mentioned in the line right above:

Casting the bool to an int doesn't seem that onerous.

> > > So it's an issue with clang-14 then?
> > > (I add Nick and Nathan)
> >
> > My clang here doesn't have the option, so I'm going to ask---are you
> > using W=1?  I can see why clang is warning for KVM's code, but in my
> > opinion such a check should only be in -Wextra.
>
> This is a newly added warning in top of tree clang.
>
> >
> > Paolo
> >
> > >>
> > >>           /*
> > >>            * Use a bitwise-OR instead of a logical-OR to aggregate the reserved
> > >>            * bits and EPT's invalid memtype/XWR checks to avoid an extra Jcc
> > >>            * (this is extremely unlikely to be short-circuited as true).
> > >>            */
> > >>
> > >> Paolo
> > >
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-04 17:12         ` Jim Mattson
@ 2021-10-14 17:50           ` Nathan Chancellor
  2021-10-14 18:31             ` Sean Christopherson
  0 siblings, 1 reply; 13+ messages in thread
From: Nathan Chancellor @ 2021-10-14 17:50 UTC (permalink / raw)
  To: Jim Mattson
  Cc: Nick Desaulniers, Paolo Bonzini, torvic9, seanjc, vkuznets, kvm,
	linux-kernel, tglx, bp

On Mon, Oct 04, 2021 at 10:12:33AM -0700, Jim Mattson wrote:
> On Mon, Oct 4, 2021 at 9:13 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > On Mon, Oct 4, 2021 at 2:49 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > >
> > > On 04/10/21 11:30, torvic9@mailbox.org wrote:
> > > >
> > > >> Paolo Bonzini <pbonzini@redhat.com> hat am 04.10.2021 11:26 geschrieben:
> > > >>
> > > >>
> > > >> On 04/10/21 11:08, torvic9@mailbox.org wrote:
> > > >>> I encounter the following issue when compiling 5.15-rc4 with clang-14:
> > > >>>
> > > >>> In file included from arch/x86/kvm/mmu/mmu.c:27:
> > > >>> arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
> > > >>>           return __is_bad_mt_xwr(rsvd_check, spte) |
> > > >>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >>>                                                    ||
> > > >>> arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning
> > > >>
> > > >> The warning is wrong, as mentioned in the line right above:
> 
> Casting the bool to an int doesn't seem that onerous.

Alternatively, could we just change both of the functions to return u64?
I understand that they are being used in boolean contexts only but it
seems like this would make it clear that a boolean or bitwise operator
on them is acceptable.

diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h
index eb7b227fc6cf..0ca215bfe3a3 100644
--- a/arch/x86/kvm/mmu/spte.h
+++ b/arch/x86/kvm/mmu/spte.h
@@ -295,14 +295,14 @@ static inline u64 get_rsvd_bits(struct rsvd_bits_validate *rsvd_check, u64 pte,
 	return rsvd_check->rsvd_bits_mask[bit7][level-1];
 }
 
-static inline bool __is_rsvd_bits_set(struct rsvd_bits_validate *rsvd_check,
-				      u64 pte, int level)
+static inline u64 __is_rsvd_bits_set(struct rsvd_bits_validate *rsvd_check,
+				     u64 pte, int level)
 {
 	return pte & get_rsvd_bits(rsvd_check, pte, level);
 }
 
-static inline bool __is_bad_mt_xwr(struct rsvd_bits_validate *rsvd_check,
-				   u64 pte)
+static inline u64 __is_bad_mt_xwr(struct rsvd_bits_validate *rsvd_check,
+				  u64 pte)
 {
 	return rsvd_check->bad_mt_xwr & BIT_ULL(pte & 0x3f);
 }

> > > > So it's an issue with clang-14 then?
> > > > (I add Nick and Nathan)
> > >
> > > My clang here doesn't have the option, so I'm going to ask---are you
> > > using W=1?  I can see why clang is warning for KVM's code, but in my
> > > opinion such a check should only be in -Wextra.
> >
> > This is a newly added warning in top of tree clang.
> >
> > >
> > > Paolo
> > >
> > > >>
> > > >>           /*
> > > >>            * Use a bitwise-OR instead of a logical-OR to aggregate the reserved
> > > >>            * bits and EPT's invalid memtype/XWR checks to avoid an extra Jcc
> > > >>            * (this is extremely unlikely to be short-circuited as true).
> > > >>            */
> > > >>
> > > >> Paolo
> > > >
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
> 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-14 17:50           ` Nathan Chancellor
@ 2021-10-14 18:31             ` Sean Christopherson
  2021-10-14 19:06               ` Nick Desaulniers
  0 siblings, 1 reply; 13+ messages in thread
From: Sean Christopherson @ 2021-10-14 18:31 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Jim Mattson, Nick Desaulniers, Paolo Bonzini, torvic9, vkuznets,
	kvm, linux-kernel, tglx, bp

On Thu, Oct 14, 2021, Nathan Chancellor wrote:
> On Mon, Oct 04, 2021 at 10:12:33AM -0700, Jim Mattson wrote:
> > On Mon, Oct 4, 2021 at 9:13 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> > >
> > > On Mon, Oct 4, 2021 at 2:49 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > > >
> > > > On 04/10/21 11:30, torvic9@mailbox.org wrote:
> > > > >
> > > > >> Paolo Bonzini <pbonzini@redhat.com> hat am 04.10.2021 11:26 geschrieben:
> > > > >>
> > > > >>
> > > > >> On 04/10/21 11:08, torvic9@mailbox.org wrote:
> > > > >>> I encounter the following issue when compiling 5.15-rc4 with clang-14:
> > > > >>>
> > > > >>> In file included from arch/x86/kvm/mmu/mmu.c:27:
> > > > >>> arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
> > > > >>>           return __is_bad_mt_xwr(rsvd_check, spte) |
> > > > >>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > >>>                                                    ||
> > > > >>> arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning
> > > > >>
> > > > >> The warning is wrong, as mentioned in the line right above:
> > 
> > Casting the bool to an int doesn't seem that onerous.
> 
> Alternatively, could we just change both of the functions to return u64?
> I understand that they are being used in boolean contexts only but it
> seems like this would make it clear that a boolean or bitwise operator
> on them is acceptable.

If we want to fix this, my vote is for casting to an int and updating the comment
in is_rsvd_spte().  I think I'd vote to fix this?  IIRC KVM has had bitwise goofs
in the past that manifested as real bugs, it would be nice to turn this on.

Or maybe add a macro to handle this?  E.g.

diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h
index 7c0b09461349..38aeb4b21925 100644
--- a/arch/x86/kvm/mmu/spte.h
+++ b/arch/x86/kvm/mmu/spte.h
@@ -307,6 +307,12 @@ static inline bool __is_bad_mt_xwr(struct rsvd_bits_validate *rsvd_check,
        return rsvd_check->bad_mt_xwr & BIT_ULL(pte & 0x3f);
 }

+/*
+ * Macro for intentional bitwise-OR of two booleans, which requires casting at
+ * least one of the results to an int to suppress -Wbitwise-instead-of-logical.
+ */
+#define BITWISE_BOOLEAN_OR(a, b) (!!((int)(a) | (int)(b)))
+
 static __always_inline bool is_rsvd_spte(struct rsvd_bits_validate *rsvd_check,
                                         u64 spte, int level)
 {
@@ -315,8 +321,8 @@ static __always_inline bool is_rsvd_spte(struct rsvd_bits_validate *rsvd_check,
         * bits and EPT's invalid memtype/XWR checks to avoid an extra Jcc
         * (this is extremely unlikely to be short-circuited as true).
         */
-       return __is_bad_mt_xwr(rsvd_check, spte) |
-              __is_rsvd_bits_set(rsvd_check, spte, level);
+       return BITWISE_BOOLEAN_OR(__is_bad_mt_xwr(rsvd_check, spte),
+                                 __is_rsvd_bits_set(rsvd_check, spte, level));
 }

 static inline bool spte_can_locklessly_be_made_writable(u64 spte)

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-14 18:31             ` Sean Christopherson
@ 2021-10-14 19:06               ` Nick Desaulniers
  2021-10-14 20:50                 ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: Nick Desaulniers @ 2021-10-14 19:06 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Nathan Chancellor, Jim Mattson, Paolo Bonzini, torvic9, vkuznets,
	kvm, linux-kernel, tglx, bp

On Thu, Oct 14, 2021 at 11:31 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Oct 14, 2021, Nathan Chancellor wrote:
> > On Mon, Oct 04, 2021 at 10:12:33AM -0700, Jim Mattson wrote:
> > > On Mon, Oct 4, 2021 at 9:13 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > >
> > > > On Mon, Oct 4, 2021 at 2:49 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > > > >
> > > > > On 04/10/21 11:30, torvic9@mailbox.org wrote:
> > > > > >
> > > > > >> Paolo Bonzini <pbonzini@redhat.com> hat am 04.10.2021 11:26 geschrieben:
> > > > > >>
> > > > > >>
> > > > > >> On 04/10/21 11:08, torvic9@mailbox.org wrote:
> > > > > >>> I encounter the following issue when compiling 5.15-rc4 with clang-14:
> > > > > >>>
> > > > > >>> In file included from arch/x86/kvm/mmu/mmu.c:27:
> > > > > >>> arch/x86/kvm/mmu/spte.h:318:9: error: use of bitwise '|' with boolean operands [-Werror,-Wbitwise-instead-of-logical]
> > > > > >>>           return __is_bad_mt_xwr(rsvd_check, spte) |
> > > > > >>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > >>>                                                    ||
> > > > > >>> arch/x86/kvm/mmu/spte.h:318:9: note: cast one or both operands to int to silence this warning
> > > > > >>
> > > > > >> The warning is wrong, as mentioned in the line right above:
> > >
> > > Casting the bool to an int doesn't seem that onerous.
> >
> > Alternatively, could we just change both of the functions to return u64?
> > I understand that they are being used in boolean contexts only but it
> > seems like this would make it clear that a boolean or bitwise operator
> > on them is acceptable.
>
> If we want to fix this, my vote is for casting to an int and updating the comment

At the least, I think bitwise operations should only be performed on
unsigned types.

> in is_rsvd_spte().  I think I'd vote to fix this?  IIRC KVM has had bitwise goofs
> in the past that manifested as real bugs, it would be nice to turn this on.
>
> Or maybe add a macro to handle this?  E.g.

I think Nathan's suggestion was much cleaner.  If explicit casts are
enough to silence the warning, then I think Jim's suggestion is even
better (though unsigned, not signed int).

>
> diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h
> index 7c0b09461349..38aeb4b21925 100644
> --- a/arch/x86/kvm/mmu/spte.h
> +++ b/arch/x86/kvm/mmu/spte.h
> @@ -307,6 +307,12 @@ static inline bool __is_bad_mt_xwr(struct rsvd_bits_validate *rsvd_check,
>         return rsvd_check->bad_mt_xwr & BIT_ULL(pte & 0x3f);
>  }
>
> +/*
> + * Macro for intentional bitwise-OR of two booleans, which requires casting at
> + * least one of the results to an int to suppress -Wbitwise-instead-of-logical.
> + */
> +#define BITWISE_BOOLEAN_OR(a, b) (!!((int)(a) | (int)(b)))
> +
>  static __always_inline bool is_rsvd_spte(struct rsvd_bits_validate *rsvd_check,
>                                          u64 spte, int level)
>  {
> @@ -315,8 +321,8 @@ static __always_inline bool is_rsvd_spte(struct rsvd_bits_validate *rsvd_check,
>          * bits and EPT's invalid memtype/XWR checks to avoid an extra Jcc
>          * (this is extremely unlikely to be short-circuited as true).
>          */
> -       return __is_bad_mt_xwr(rsvd_check, spte) |
> -              __is_rsvd_bits_set(rsvd_check, spte, level);
> +       return BITWISE_BOOLEAN_OR(__is_bad_mt_xwr(rsvd_check, spte),
> +                                 __is_rsvd_bits_set(rsvd_check, spte, level));
>  }
>
>  static inline bool spte_can_locklessly_be_made_writable(u64 spte)



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-14 19:06               ` Nick Desaulniers
@ 2021-10-14 20:50                 ` Paolo Bonzini
  2021-10-15  9:17                   ` Christophe de Dinechin
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2021-10-14 20:50 UTC (permalink / raw)
  To: Nick Desaulniers, Sean Christopherson
  Cc: Nathan Chancellor, Jim Mattson, torvic9, vkuznets, kvm,
	linux-kernel, tglx, bp

On 14/10/21 21:06, Nick Desaulniers wrote:
>> If we want to fix this, my vote is for casting to an int and updating the comment
> 
> At the least, I think bitwise operations should only be performed on
> unsigned types.

This is not a bitwise operation, it's a non-short-circuiting boolean 
operation.  I'll apply Jim's suggestion.

Paolo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14
  2021-10-14 20:50                 ` Paolo Bonzini
@ 2021-10-15  9:17                   ` Christophe de Dinechin
  0 siblings, 0 replies; 13+ messages in thread
From: Christophe de Dinechin @ 2021-10-15  9:17 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Nick Desaulniers, Sean Christopherson, Nathan Chancellor,
	Jim Mattson, torvic9, vkuznets, kvm, linux-kernel, tglx, bp



> On 14 Oct 2021, at 22:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 14/10/21 21:06, Nick Desaulniers wrote:
>>> If we want to fix this, my vote is for casting to an int and updating the comment
>> At the least, I think bitwise operations should only be performed on
>> unsigned types.
> 
> This is not a bitwise operation, it's a non-short-circuiting boolean operation.  I'll apply Jim's suggestion.

What about making it an inline function, which would require evaluation of arguments:

	static __always_inline bool BITWISE_BOOLEAN_OR(bool a, bool b)
	{
	    return a || b; // Safe here, because arguments have been evaluated
	}

Suggesting that because I'm always nervous about casts in macros hiding something that the type  system would otherwise catch.


Christophe
> 
> Paolo
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-10-15  9:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-04  9:08 [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14 torvic9
2021-10-04  9:26 ` Paolo Bonzini
2021-10-04  9:30   ` torvic9
2021-10-04  9:49     ` Paolo Bonzini
2021-10-04 10:10       ` torvic9
2021-10-04 14:33         ` torvic9
2021-10-04 16:13       ` Nick Desaulniers
2021-10-04 17:12         ` Jim Mattson
2021-10-14 17:50           ` Nathan Chancellor
2021-10-14 18:31             ` Sean Christopherson
2021-10-14 19:06               ` Nick Desaulniers
2021-10-14 20:50                 ` Paolo Bonzini
2021-10-15  9:17                   ` Christophe de Dinechin

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.