Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] KVM: x86: small optimization for is_mtrr_mask calculation
To:     David Laight <David.Laight@ACULAB.COM>,
        linmiaohe <linmiaohe@huawei.com>,
        "rkrcmar@redhat.com" <rkrcmar@redhat.com>,
        "sean.j.christopherson@intel.com" <sean.j.christopherson@intel.com>,
        "vkuznets@redhat.com" <vkuznets@redhat.com>,
        "jmattson@google.com" <jmattson@google.com>,
        "joro@8bytes.org" <joro@8bytes.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "bp@alien8.de" <bp@alien8.de>, "hpa@zytor.com" <hpa@zytor.com>
Cc:     "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>
References: <1583376535-27255-1-git-send-email-linmiaohe@huawei.com>
 <2b678644-fcc0-e853-a53c-2651c1f6a327@redhat.com>
 <dc1870b0ea164015b1c1b6bc4d3248fe@AcuMS.aculab.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <2129995e-3441-f362-aed1-7c247189c136@redhat.com>
Date:   Thu, 5 Mar 2020 16:25:20 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.4.1
MIME-Version: 1.0
In-Reply-To: <dc1870b0ea164015b1c1b6bc4d3248fe@AcuMS.aculab.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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