| Message ID | 1503352419-2851-1-git-send-email-xiong.y.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 21.08.17 at 23:53, <xiong.y.zhang@intel.com> wrote: > @@ -464,15 +462,19 @@ void pci_setup(void) > 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) ) > + if ( resource == &mem_resource) > { > - base = memory_map.map[next_rmrr].addr + memory_map.map[next_rmrr].size; > - base = (base + bar_sz - 1) & ~(bar_sz - 1); > next_rmrr = find_next_rmrr(base); > + while ( 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 = (base + bar_sz - 1) & ~(bar_sz - 1); > + next_rmrr = find_next_rmrr(base); > + } > } Looks good, but please reduce the scope of next_rmrr to just this if() (afaict it's no longer used anywhere else). Jan
diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c index f4288a3..16fccbf 100644 --- a/tools/firmware/hvmloader/pci.c +++ b/tools/firmware/hvmloader/pci.c @@ -405,8 +405,6 @@ void pci_setup(void) io_resource.base = 0xc000; io_resource.max = 0x10000; - next_rmrr = find_next_rmrr(pci_mem_start); - /* Assign iomem and ioport resources in descending order of size. */ for ( i = 0; i < nr_bars; i++ ) { @@ -464,15 +462,19 @@ void pci_setup(void) 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) ) + if ( resource == &mem_resource) { - base = memory_map.map[next_rmrr].addr + memory_map.map[next_rmrr].size; - base = (base + bar_sz - 1) & ~(bar_sz - 1); next_rmrr = find_next_rmrr(base); + while ( 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 = (base + bar_sz - 1) & ~(bar_sz - 1); + next_rmrr = find_next_rmrr(base); + } } bar_data |= (uint32_t)base;
find_next_rmrr(base) is used to find the lowest RMRR ending above base but below 4G. Current method couldn't cover the following situation: a. two rmrr exist, small gap between them b. pci_mem_start and mem_resource.base is below the first rmrr.base c. find_next_rmrr(pci_mem_start) will find the first rmrr d. After aligning mem_resource.base to bar size, first_rmrr.end < new_base < second_rmrr.base and new_base + bar_sz > second_rmrr.base. So the new bar will overlap with the second rmrr and don't overlap with the first rmrr. But the next_rmrr point to the first rmrr, then check_overlap() couldn't find the overlap. Finally assign a wrong bar address to bar. This patch using aligned new base to find the next rmrr, could fix the above case and find all the overlapped rmrr with new base. Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.com> --- tools/firmware/hvmloader/pci.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)