Re: [PATCH] simplify pdpte check

Avi Kivity
2007-02-01T14:54:27Z

Dor Laor wrote:
>
>> -----Original Message-----
>> From: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org [mailto:kvm-devel-
>> bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org] On Behalf Of Muli Ben-Yehuda
>> Sent: Thursday, February 01, 2007 4:46 PM
>> To: Avi Kivity
>> Cc: kvm-devel
>> Subject: Re: [kvm-devel] [PATCH] simplify pdpte check
>>
>> On Thu, Feb 01, 2007 at 04:07:01PM +0200, Avi Kivity wrote:
>>
>>> Muli Ben-Yehuda wrote:
>>>
>>>> Small cleanup: we check (pdpte & 1) && (pdpte & constant). Instead
>>>> just check (pdpte & (constant | 1)).
>>>>
>>>> Signed-off-by: Muli Ben-Yehuda <muli-7z/5BgaJwgfQT0dZR+AlfA@public.gmane.org>
>>>>
>>>> Index: kernel/kvm_main.c
>>>> ===================================================================
>>>> --- kernel/kvm_main.c (revision 4379)
>>>> +++ kernel/kvm_main.c (working copy)
>>>> @@ -336,7 +336,7 @@
>>>> ret = 1;
>>>> for (i = 0; i < 4; ++i) {
>>>> pdpte = pdpt[offset + i];
>>>> - if ((pdpte & 1) && (pdpte & 0xfffffff0000001e6ull)) {
>>>> + if (pdpte & 0xfffffff0000001e7ull) {
>>>> ret = 0;
>>>> goto out;
>>>> }
>>>>
>>>>
>>>>
>>> The code is not equivalent. For example, pdpte == 2 passes the check
>>> before the patch and fails it after.
>>>
>> err... 2 & 1 == 0, which does not pass the original check. Perhaps the
>> real error is '&&' instead of '||' in the original conditional?
>>
>
>
> What about 'if (pdpte & 0xfffffff0000001e7ull > 0x1)' ?
>

That would work.

> Although the original is more readable.
>

Yes, and we're not exactly performance critical here.

--
error compiling committee.c: too many arguments to function