@@ -331,12 +331,15 @@ void pci_scan_bars(struct pci_dev *dev)
int i;
for (i = 0; i < PCI_BAR_NUM; i++) {
- if (!pci_bar_is_valid(dev, i))
- continue;
- dev->resource[i] = pci_bar_get_addr(dev, i);
- if (pci_bar_is64(dev, i)) {
- i++;
- dev->resource[i] = (phys_addr_t)0;
+ if (pci_bar_size(dev, i)) {
+ dev->resource[i] = pci_bar_get_addr(dev, i);
+ if (pci_bar_is64(dev, i)) {
+ assert(i + 1 < PCI_BAR_NUM);
+ dev->resource[i + 1] = dev->resource[i];
+ i++;
+ }
+ } else {
+ dev->resource[i] = INVALID_PHYS_ADDR;
}
}
}
Array pci_dev::resource[] is ambiguous wrt what is actually stored in its elements and how to interpret the index into the array. It is simple in case a device has only 32-bit BARs - an element pci_dev::resource[bar_num] contains the decoded address of BAR # bar_num. But what if a device has 64-bit BAR starting at bar_num? Curretnly pci_dev::resource[bar_num] contains the decoded address of the BAR, while pci_dev::resource[bar_num + 1] contains 0. That makes meaning of (bar_num + 1) index difficult to understand. On the one hand a testcase should not address a 64-bit BAR using high part BAR number. Particularly, when it tries to obtan BAR size or address. On the other hand a testcase should be able to access a low and high parts separately if it needs to for whatever reason. The rest of the API allows that as well. pci_dev::resource[] contains decoded 32/64-bit BAR addresses, not low/high parts of underlying 32-bit device BARs. Yet, indexes into this array correspond to raw 32-bit BARs in the PCI device configuration space. Thus, a question arises - what should be stored in array elements that correspond to high-parts of 64-bit BARs? Zero is particularly bad choice, because: - it is a valid address in PIO address space, so it can not stand for "no value" or NULL or whatever marker could be used to indicate a high part; - the high part of underlying 64-bit address is (could be) non-zero. So there is inconsistency also; By placing the same 64-bit address in both bar_num and (bar_num + 1) elements the ambiguity is less striking, since: - the meaning of bar_num kept consistent with the rest of the functions (where it refers 32-bit BAR in terms of the device configuration address space); - pci_dev::resource[bar_num + 1] contains a valid address rather than vague value of 0. - both bar_num and (bar_num + 1) indexes refer to the same 64-bit BAR and therefore return the same address; The notion of low and high parts of a 64-bit address is ignored, but that is fine, since pci_dev::resource[] contain only full addresses; Cc: Thomas Huth <thuth@redhat.com> Cc: Andrew Jones <drjones@redhat.com> Cc: Peter Xu <peterx@redhat.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Signed-off-by: Alexander Gordeev <agordeev@redhat.com> --- lib/pci.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)