Re: [PATCH] KVM: x86: small optimization for is_mtrr_mask calculation

[Paolo Bonzini]

On 05/03/20 16:10, David Laight wrote:
>>>  	index = (msr - 0x200) / 2;
>>> -	is_mtrr_mask = msr - 0x200 - 2 * index;
>>> +	is_mtrr_mask = (msr - 0x200) % 2;
>>>  	cur = &mtrr_state->var_ranges[index];
>>>
>>>  	/* remove the entry if it's in the list. */
>>> @@ -424,7 +424,7 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
>>>  		int is_mtrr_mask;
>>>
>>>  		index = (msr - 0x200) / 2;
>>> -		is_mtrr_mask = msr - 0x200 - 2 * index;
>>> +		is_mtrr_mask = (msr - 0x200) % 2;
>>>  		if (!is_mtrr_mask)
>>>  			*pdata = vcpu->arch.mtrr_state.var_ranges[index].base;
>>>  		else
>>>
>> If you're going to do that, might as well use ">> 1" for index instead
>> of "/ 2", and "msr & 1" for is_mtrr_mask.
> Provided the variables are unsigned it makes little difference
> whether you use / % or >> &.
> At least with / % the two values are the same.

Yes, I'm old-fashioned, but also I prefer ">>" and "&" for both signed
and unsigned, because if ever I need to switch from unsigned to signed I
will get floor-division instead of round-to-zero division (most likely
the code doesn't expect negative remainders if it was using unsigned).

(That perhaps also reflects on me working a lot with Smalltalk long
before switching to the kernel...).

Paolo