Re: [PATCH] KVM: MMU: speedup update_permission_bitmask

[Paolo Bonzini]

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