Re: [Xen-devel] [v6][PATCH 6/7] hvmloader: check to reserved device memory maps in e820

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.09.14 at 08:28, <tiejun.chen@xxxxxxxxx> wrote:
> On 2014/9/11 23:57, Jan Beulich wrote:
>>>>> On 10.09.14 at 07:49, <tiejun.chen@xxxxxxxxx> wrote:
>>> + do_real_construct:
>>> +    for ( i = 0; i < nr_map; i++ )
>>> +    {
>>> +        rdm_start = map[i].start_pfn << PAGE_SHIFT;
>>> +        rdm_end = rdm_start + map[i].nr_pages * PAGE_SIZE;
>>
>> Same comment as for an earlier patch.
> 
>          rdm_start = map[i].start_pfn << XC_PAGE_SHIFT;
>          rdm_end = rdm_start + map[i].nr_pages << XC_PAGE_SHIFT;
> 
> But there's no such definition in tools/firmware/, and this is defined 
> in xenctl.h. But if I include xenctl.h here, something else is 
> redefined. So here I have to define this, XC_PAGE_SHIFTseparately.

Iirc the earlier comment was on a libxc change. Obviously using its
definitions in hvmloader is forbidden. Out of the three things that
were pointed out wrong there, two still apply (and I think you should
have the smarts to filter out the one that doesn't rather than me
having to re-explain the same issue here again).

>>> +
>>> +                    /* Then fill RMRR into that entry. */
>>> +                    e820[j+1].addr = rdm_start;
>>> +                    e820[j+1].size = rdm_end - rdm_start;
>>> +                    e820[j+1].type = E820_RESERVED;
>>> +                    nr++;
>>> +                }
>>> +                insert++;
>>> +            }
>>> +            /* Already at the end. */
>>> +            else if ( (rdm_start > end) && !start )
>>
>> How would !start represent the end of anything?
> 
> end = e820[j].addr + e820[j].size;
> start = e820[j+1].addr;
> 
> so if end = 0xXXXXXXXX and start = 0, does this mean we already are at 
> the end of all valid e820 entries?

How would start validly be zero (i.e. other than incidentally due to
reading from an uninitialized entry)?

Jan


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