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@xxxxxxxxxx>
Cc: Andrew Jones <drjones@xxxxxxxxxx>
Cc: Peter Xu <peterx@xxxxxxxxxx>
Reviewed-by: Andrew Jones <drjones@xxxxxxxxxx>
Signed-off-by: Alexander Gordeev <agordeev@xxxxxxxxxx>
---
lib/pci.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/lib/pci.c b/lib/pci.c
index 62b1c6b0b7c5..9acc9652cb25 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -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;
}
}
}
--
1.8.3.1
