Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751330AbdILIUl (ORCPT ); Tue, 12 Sep 2017 04:20:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42732 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751141AbdILIUj (ORCPT ); Tue, 12 Sep 2017 04:20:39 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com A3153356CC Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=pbonzini@redhat.com Subject: Re: [PATCH] KVM: MMU: speedup update_permission_bitmask To: David Hildenbrand , linux-kernel@vger.kernel.org, kvm@vger.kernel.org Cc: jmattson@google.com References: <1503590171-41030-1-git-send-email-pbonzini@redhat.com> From: Paolo Bonzini Message-ID: <6889a355-02ff-d473-681d-5632cf572e44@redhat.com> Date: Tue, 12 Sep 2017 10:20:27 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 12 Sep 2017 08:20:39 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1277 Lines: 45 On 29/08/2017 18:46, David Hildenbrand wrote: >> +#define BYTE_MASK(access) \ >> + ((1 & (access) ? 2 : 0) | \ >> + (2 & (access) ? 4 : 0) | \ >> + (3 & (access) ? 8 : 0) | \ >> + (4 & (access) ? 16 : 0) | \ >> + (5 & (access) ? 32 : 0) | \ >> + (6 & (access) ? 64 : 0) | \ >> + (7 & (access) ? 128 : 0)) >> + > Hmm, I wonder if > > #define BYTE_MASK(access) \ > ((1 & (access) ? (1 << 1) : 0) | \ > (2 & (access) ? (1 << 2) : 0) | \ > ... > > would be easier to get > Yeah, you have a point. Another way to write it is: (1 & (access) ? 0xAA : 0) | \ (2 & (access) ? 0xCC : 0) | \ (4 & (access) ? 0xF0 : 0) but I think yours is the best. >> >> + >> + const u8 x = BYTE_MASK(ACC_EXEC_MASK); >> + const u8 w = BYTE_MASK(ACC_WRITE_MASK); >> + const u8 u = BYTE_MASK(ACC_USER_MASK); >> + >> + bool cr4_smep = kvm_read_cr4_bits(vcpu, X86_CR4_SMEP) != 0; >> + bool cr4_smap = kvm_read_cr4_bits(vcpu, X86_CR4_SMAP) != 0; >> + bool cr0_wp = is_write_protection(vcpu); > > all three can be turned const. (and I'd drop the empty lines in between ..) I am using const to identify a compile-time constant here, so more like "static const" (but I was not sure if C optimizes away static const, so I just used "const"). This explains also the empty line! Paolo