| Message ID | 1503875366-14572-1-git-send-email-xiong.y.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 28.08.17 at 01:09, <xiong.y.zhang@intel.com> wrote: > 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 doesn'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 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> Reviewed-by: Jan Beulich <jbeulich@suse.com> albeit ... > --- a/tools/firmware/hvmloader/pci.c > +++ b/tools/firmware/hvmloader/pci.c > @@ -84,7 +84,6 @@ void pci_setup(void) > uint32_t vga_devfn = 256; > uint16_t class, vendor_id, device_id; > unsigned int bar, pin, link, isa_irq; > - int next_rmrr; > > /* Resources assignable to PCI devices via BARs. */ > struct resource { > @@ -405,8 +404,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 +461,21 @@ 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); > + int next_rmrr; > + > next_rmrr = find_next_rmrr(base); ... this should have become the initializer of the variable now. If I end up committing this, I'll try to remember to do the adjustment. Jan
diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c index f4288a3..1858d7f 100644 --- a/tools/firmware/hvmloader/pci.c +++ b/tools/firmware/hvmloader/pci.c @@ -84,7 +84,6 @@ void pci_setup(void) uint32_t vga_devfn = 256; uint16_t class, vendor_id, device_id; unsigned int bar, pin, link, isa_irq; - int next_rmrr; /* Resources assignable to PCI devices via BARs. */ struct resource { @@ -405,8 +404,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 +461,21 @@ 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); + int next_rmrr; + 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 doesn'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 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> --- Changes since v1: - Reduce the scope of next_rmrr to just this if() --- tools/firmware/hvmloader/pci.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)