From patchwork Tue Jun 19 17:08:55 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nick Desaulniers X-Patchwork-Id: 10475133 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8E59C60383 for ; Tue, 19 Jun 2018 17:09:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7AFE528CB5 for ; Tue, 19 Jun 2018 17:09:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6F19F28D9B; Tue, 19 Jun 2018 17:09:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0F89028CB5 for ; Tue, 19 Jun 2018 17:09:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966800AbeFSRJL (ORCPT ); Tue, 19 Jun 2018 13:09:11 -0400 Received: from mail-pl0-f66.google.com ([209.85.160.66]:41332 "EHLO mail-pl0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966449AbeFSRJI (ORCPT ); Tue, 19 Jun 2018 13:09:08 -0400 Received: by mail-pl0-f66.google.com with SMTP id w8-v6so176879ply.8 for ; Tue, 19 Jun 2018 10:09:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=bxeF8FT51s8VsfayKODgispQy/SzF3AFKefvkclxn7M=; b=L+yziCH1pGc9srMiiv8qvCW/GHxKUcruYWfwwXhE7wCfjfq1WpyNjEABHnv4VL/KYi iN9KVdC3CM2epGyB9CJmE7Ikrb+GsJdiGfGFFagj7k4y42mdiJe4zcLlYMfOVPc6pNf2 rb8lj6C0Z5pfnTgP+/ZRGxttNewyO40/GZ5+AynJtDQood58umBQfmNvh4XIObmtnqAh R/lO1h296NtmhpVbsy5k5qLGBWPp516kznwC8Mz2257DuTKeFEp+kUpDLovhNlp3wkgg Xm5t6I4n+pkduzP3n4G6VKbvtuFit3Js0Utfft5W9mwh5Mo0MW0oj6peCScJksBFGved UpnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=bxeF8FT51s8VsfayKODgispQy/SzF3AFKefvkclxn7M=; b=OjCl7ZjBVKfD2hI1GIkWOsa3uNd4Z3Wpz307hzO51faACGKaOWEu5hy47YhnkR0n7I WlfqiwkFgmgYjhvy8cijulnfBgouDNcpUm9trVS9b7Yxg/Nh4ke/+vkfIDHUkgx8uC/e Z5tboPKHSt0Y9IswiJAAKrGNz7/NANFQe6D8YeGcOERyu7ijf+IX3tA18v+mDbr//mhw p5LfvK+X/aZXiP5+k8hJbLDbcFAbZqYUIU0TQVtQJYDqEDrYY3SFC4/Mq2l8xcNK76wS uk6hZwxe/+t/KmfYhycvDfbn6N/xEMJtf+ZvTFh7UsrTacG3Sg44I0Go+NZavYVFk2/P hCdA== X-Gm-Message-State: APt69E0ckB4H0LPLx4zV7P2/xvkZypBaFhL/ne2jVgqDyO9T+DFBRRd+ d1VAQtPAeVdgzsyAIXvFcNcRJIXax+DelHsOFGIWOQ== X-Google-Smtp-Source: ADUXVKLxWbC12An6igp3YyeGwcBf3epj/mnnDAAzSJCjgHNvSczXe6pQURjXp+/P7I37j1pp/hkYF18abdguMlGMrIg= X-Received: by 2002:a17:902:8d8b:: with SMTP id v11-v6mr20057422plo.20.1529428148096; Tue, 19 Jun 2018 10:09:08 -0700 (PDT) MIME-Version: 1.0 References: <20180615174731.47588-1-mka@chromium.org> <20180615182945.GN88063@google.com> In-Reply-To: From: Nick Desaulniers Date: Tue, 19 Jun 2018 10:08:55 -0700 Message-ID: Subject: Re: [PATCH] kvm: x86: mmu: Add cast to negated bitmasks in update_permission_bitmask() To: pbonzini@redhat.com Cc: joe@perches.com, Matthias Kaehlcke , rkrcmar@redhat.com, Thomas Gleixner , hpa@zytor.com, x86@kernel.org, kvm@vger.kernel.org, LKML Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Jun 19, 2018 at 8:19 AM Paolo Bonzini wrote: > > On 15/06/2018 20:45, Nick Desaulniers wrote: > >> > >>> In any case I think it it preferable to fix the code over disabling > >>> the warning, unless the warning is bogus or there are just too many > >>> occurrences. > >> Maybe. > > Spurious warning today, actual bug tomorrow? I prefer to not to > > disable warnings wholesale. They don't need to find actual bugs to be > > useful. Flagging code that can be further specified does not hurt. > > Part of the effort to compile the kernel with different compilers is > > to add warning coverage, not remove it. That said, there may be > > warnings that are never useful (or at least due to some invariant that > > only affects the kernel). I cant think of any off the top of my head, > > but I'm also not sure this is one. > > This one really makes the code uglier though, so I'm not really inclined > to applying the patch. Note that of the three variables (w, u, x), only u is used later on. What about declaring them as negated with the cast, that way there's no cast in a ternary? Ex: bool cr4_smep = kvm_read_cr4_bits(vcpu, X86_CR4_SMEP) != 0; @@ -4278,11 +4279,11 @@ static void update_permission_bitmask(struct kvm_vcpu *vcpu, */ /* Faults from writes to non-writable pages */ - u8 wf = (pfec & PFERR_WRITE_MASK) ? ~w : 0; + u8 wf = (pfec & PFERR_WRITE_MASK) ? w_not : 0; /* Faults from user mode accesses to supervisor pages */ - u8 uf = (pfec & PFERR_USER_MASK) ? ~u : 0; + u8 uf = (pfec & PFERR_USER_MASK) ? u_not : 0; /* Faults from fetches of non-executable pages*/ - u8 ff = (pfec & PFERR_FETCH_MASK) ? ~x : 0; + u8 ff = (pfec & PFERR_FETCH_MASK) ? x_not : 0; /* Faults from kernel mode fetches of user pages */ u8 smepf = 0; /* Faults from kernel mode accesses of user pages */ Maybe you have a better naming scheme than *_not ? What do you think? diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index d594690d8b95..53673ad4b295 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -4261,8 +4261,9 @@ static void update_permission_bitmask(struct kvm_vcpu *vcpu, { unsigned byte; - const u8 x = BYTE_MASK(ACC_EXEC_MASK); - const u8 w = BYTE_MASK(ACC_WRITE_MASK); + const u8 x_not = (u8)~BYTE_MASK(ACC_EXEC_MASK); + const u8 w_not = (u8)~BYTE_MASK(ACC_WRITE_MASK); + const u8 u_not = (u8)~BYTE_MASK(ACC_USER_MASK); const u8 u = BYTE_MASK(ACC_USER_MASK);