Re: [Xen-devel] [v3][PATCH 07/16] hvmloader/pci: skip reserved ranges

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.06.15 at 09:10, <tiejun.chen@xxxxxxxxx> wrote:
> On 2015/6/16 17:40, Jan Beulich wrote:
>>>>> On 16.06.15 at 11:29, <tiejun.chen@xxxxxxxxx> wrote:
>>> I'm trying to walk into this direction:
>>>
>>>       /*
>>>        * We'll skip all space overlapping with reserved memory later,
>>>        * so we need to increase mmio_total to compensate them.
>>>        */
>>>       for ( j = 0; j < memory_map.nr_map ; j++ )
>>>       {
>>>           uint64_t conflict_size = 0;
>>>           if ( memory_map.map[j].type != E820_RAM )
>>>           {
>>>               reserved_start = memory_map.map[j].addr;
>>>               reserved_size = memory_map.map[j].size;
>>>               reserved_end = reserved_start + reserved_size;
>>>               if ( check_overlap(pci_mem_start, pci_mem_end - pci_mem_start,
>>>                                  reserved_start, reserved_size) )
>>>               {
>>>                   /*
>>>                    * Calculate how much mmio range conflict with
>>>                    * reserved device memory.
>>>                    */
>>>                   conflict_size += reserved_size;
>>>
>>>                   /*
>>>                    * But we may need to subtract those sizes beyond the
>>>                    * pci memory, [pci_mem_start, pci_mem_end].
>>>                    */
>>>                   if ( reserved_start < pci_mem_start )
>>>                       conflict_size -= (pci_mem_start - reserved_start);
>>>                   if ( reserved_end > pci_mem_end )
>>>                       conflict_size -= (reserved_end - pci_mem_end);
>>>               }
>>>           }
>>>
>>>           if ( conflict_size )
>>>           {
>>>               uint64_t conflict_size = max_t(
>>>                       uint64_t, conflict_size, max_bar_sz);
>>>               conflict_size &= ~(conflict_size - 1);
>>>               mmio_total += conflict_size;
>>>           }
>>>       }
>>
>> This last thing goes in the right direction, but is complete overkill
>> when you have a small reserved region and a huge BAR. You
> 
> Yeah, this may waste some spaces in this worst case but I this think 
> this can guarantee our change don't impact on the original expectation, 
> right?

"Some space" may be multiple Gb (e.g. the frame buffer of a graphics
card), which is totally unacceptable.

>> ought to work out the smallest power-of-2 region enclosing the
> 
> Okay. I remember the smallest size of a given PCI I/O space is 8 bytes, 
> and the smallest size of a PCI memory space is 16 bytes. So
> 
> /* At least 16 bytes to align a PCI BAR size. */
> uint64_t align = 16;
> 
> reserved_start = memory_map.map[j].addr;
> reserved_size = memory_map.map[j].size;
> 
> reserved_start = (reserved_star + align) & ~(align - 1);
> reserved_size = (reserved_size + align) & ~(align - 1);
> 
> Is this correct?

Simply aligning the region doesn't help afaict. You need to fit it
with the other MMIO allocations.

>> reserved range (albeit there are tricky corner cases to consider).
>>
> 
> Yeah, its a little tricky since RMRR always owns a fixed start address, 
> so we can't reorder them with all pci bars. I just think at least we 
> should provide a correct solution now, then further look into what can 
> be optimized. So I think we'd better get conflict_size with 
> max(conflict_size, max_bar_sz), right?

As per above - no, this is not an option.

Jan


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