Re: [v8][PATCH 06/16] hvmloader/pci: disable all pci devices conflicting with rdm

[George Dunlap <George.Dunlap@eu.citrix.com>]

On Thu, Jul 16, 2015 at 10:15 PM, Chen, Tiejun <tiejun.chen@intel.com> wrote:
>>
>>           base = (resource->base  + bar_sz - 1) & ~(uint64_t)(bar_sz - 1);
>> +
>> +        /* If we're using mem_resource, check for RMRR conflicts */
>> +        while ( resource == &mem_resource &&
>> +                next_rmrr > 0 &&
>> +                check_overlap(base, bar_sz,
>> +                              memory_map.map[next_rmrr].addr,
>> +                              memory_map.map[next_rmrr].size)) {
>> +            base = memory_map.map[next_rmrr].addr +
>> memory_map.map[next_rmrr].size;
>> +            base = (resource->base  + bar_sz - 1) & ~(uint64_t)(bar_sz -
>> 1);
>> +            next_rmrr=find_next_rmrr(base);
>> +        }
>> +
>>           bar_data |= (uint32_t)base;
>>           bar_data_upper = (uint32_t)(base >> 32);
>>           base += bar_sz;
>>
>
> Actually this chunk of codes are really similar as what we did in my
> previous revisions from RFC ~ v3. It's just trying to skip and then
> allocate, right? As Jan pointed out, there are two key problems:
>
> #1. All skipping action probably cause a result of no sufficient MMIO to
> allocate all devices as before.
>
> #2. Another is that alignment issue. When the original "base" change to
> align to rdm_end, some spaces are wasted. Especially, these spaces could be
> allocated to other smaller bars.

Just to be pedantic: #2 is really just an extension of #1 -- i.e., it
doesn't matter if space is "wasted" if all the MMIO regions still fit;
the only reason #2 matters is that it makes #1 worse.

In any case, I know it's not perfect -- the point was to get something
that 1) was relatively simple to implement 2) worked out-of-the-box
for many cases, and 3) had a work-around which the user could use in
other cases.

Given that if we run out of MMIO space, all that happens is that some
devices will not really work, I think this solution is really no worse
than the "disable devices on conflict" solution; and it's better,
because you can actually work around it by increasing the MMIO hole
size.  But I'll leave it to  Jan and others to determine which (if
any) would be suitable to check in at this point.

 -George