linux-kernel - Re: [PATCH] KVM: MMU: speedup update_permission

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 12 Sep 2017 10:20:27 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     David Hildenbrand <david@...hat.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Cc:     jmattson@...gle.com
Subject: Re: [PATCH] KVM: MMU: speedup update_permission_bitmask

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