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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.09.14 at 17:57, <JBeulich@xxxxxxxx> wrote:
>>>> On 10.09.14 at 07:49, <tiejun.chen@xxxxxxxxx> wrote:
>> +static unsigned int construct_rdm_e820_maps(unsigned int nr,
>> +                                            uint32_t nr_map,
>> +                                            struct 
> xen_mem_reserved_device_memory *map,
>> +                                            struct e820entry *e820)
>> +{
>> +    unsigned int i = 0, j = 0, m = 0, sum_nr = nr + nr_map;
> 
> Some or all of the initializers here are pointless.
> 
>> +    uint64_t start, end, rdm_start, rdm_end;
>> +    unsigned int insert = 0, do_insert = 0;
>> +
>> + 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.
> 
>> +
>> +        for ( j = 0; j < nr - 1; j++ )
>> +        {
>> +            end = e820[j].addr + e820[j].size;
>> +            start = e820[j+1].addr;
> 
> So now you don't check the last region at all? You just need to make
> this assignment conditional upon whether you're in the last iteration.

Furthermore
- I don't think an overlap with an E820_RESERVED or E820_UNUSABLE
  region would be a problem (you'd just need to extend the former or
  amend the latter)
- you only check here rather than making sure no RAM gets placed
  into these address regions in the first place (or rather than relocating
  the RAM as is done for at least some MMIO/RAM overlaps)

Jan


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