Re: [PATCH for-4.6] xen/mm: populate_physmap: validate correctly the gfn for direct mapped domain

[Julien Grall <julien.grall@citrix.com>]

Hi Jan,

On 12/08/2015 10:16, Jan Beulich wrote:
>>>> On 11.08.15 at 19:41, <julien.grall@citrix.com> wrote:
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -126,22 +126,28 @@ static void populate_physmap(struct memop_args *a)
>>               if ( is_domain_direct_mapped(d) )
>>               {
>>                   mfn = gpfn;
>> -                if ( !mfn_valid(mfn) )
>> +
>> +                for ( j = 0; j < (1 << a->extent_order); j++, mfn++ )
>
> While benign I think we shouldn't repeat mistakes like this made
> elsewhere in the code: At the very least this should be 1U, but
> with j (needlessly, just like i) being unsigned long it would be
> more consistent for this to be 1UL unless we clean up the variable
> types (which I think I'll do).

I admit that I copied the loop at then end of the function without 
thinking if it there was some issue or not.

>
> Since the change is so minor, I'd be fine with making it while
> committing, unless you object.

I'm fine with that.

Regards,

-- 
Julien Grall