Re: [Xen-devel] [v5][PATCH 09/10] tools:firmware:hvmloader: check to reserve RMRR mappings in e820

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.09.14 at 05:04, <tiejun.chen@xxxxxxxxx> wrote:
> On 2014/9/2 16:47, Jan Beulich wrote:
>>>>> On 26.08.14 at 13:02, <tiejun.chen@xxxxxxxxx> wrote:
>>> +    unsigned int insert = 0, do_insert = 0;
>>> +
>>> +do_real_construct:
>>
>> Please indent labels by at least one space (and there are further
>> coding style issues to address elsewhere).
> 
> Is this necessary?
> 
> static struct tmem_object_root * obj_find(struct tmem_pool *pool, struct 
> oid *oidp)
> {
>      struct rb_node *node;
>      struct tmem_object_root *obj;
> 
> restart_find:
>      read_lock(&pool->pool_rwlock);
>      ...
>                      read_unlock(&pool->pool_rwlock);
>                      goto restart_find;

Question and cited code fragment don't fit together for me, so I
don't think I understand what you're asking. Taking the question
alone - yes, obeying to coding style is necessary.

>>> +    for ( i = 0; i < nr_map; i++ )
>>> +    {
>>> +        rmrr_start = map[i].start_pfn << PAGE_SHIFT;
>>> +        rmrr_end = rmrr_start + map[i].nr_pages * PAGE_SIZE;
>>> +
>>> +        for ( j = 0; j < nr; j++ )
>>> +        {
>>> +            end = e820[j].addr + e820[j].size;
>>> +            start = e820[j+1].addr;
>>
>> This is not valid when j == nr - 1 (last iteration).
>>
> -        for ( j = 0; j < nr; j++ )
> +        for ( j = 0; j < nr - 1; j++ )

And this would skip the last region - I'm not sure that's correct.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel