Re: [BUG] [5.15] Compilation error in arch/x86/kvm/mmu/spte.h with clang-14

[Sean Christopherson <seanjc@google.com>]

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)