On 09-Nov-18 5:57 AM, tone.zhang wrote:
With a larger PAGE_SIZE it is possible for the MSI table to very
close to the end of the BAR s.t. when we align the start and end
of the MSI table to the PAGE_SIZE, the end offset of the MSI
table is out of the PCI BAR boundary.
This patch addresses the issue by comparing both the start and the
end offset of the MSI table with the BAR size, and skip the mapping
if it is out of Bar scope.
The patch fixes the debug log as below:
EAL: Skipping BAR0
Signed-off-by: tone.zhang <tone.zh...@arm.com>
Reviewed-by: Gavin Hu <gavin...@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>
Reviewed-by: Steve Capper <steve.cap...@arm.com>
Reviewed-by: Burakov Anatoly <anatoly.bura...@intel.com>
In the future, please don't include my Reviewed tag unless i actually
sent one :)
---
drivers/bus/pci/linux/pci_vfio.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index 305cc06..9a0affe 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -445,9 +445,11 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct
mapped_pci_resource *vfio_res,
struct pci_msix_table *msix_table = &vfio_res->msix_table;
struct pci_map *bar = &vfio_res->maps[bar_index];
- if (bar->size == 0)
+ if (bar->size == 0) {
/* Skip this BAR */
+ RTE_LOG(INFO, EAL, "Skipping BAR%d\n", bar_index);
return 0;
+ }
if (msix_table->bar_index == bar_index) {
/*
@@ -456,8 +458,22 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct
mapped_pci_resource *vfio_res,
*/
uint32_t table_start = msix_table->offset;
uint32_t table_end = table_start + msix_table->size;
- table_end = (table_end + ~PAGE_MASK) & PAGE_MASK;
- table_start &= PAGE_MASK;
+ table_end = RTE_ALIGN(table_end, PAGE_SIZE);
+ table_start = RTE_ALIGN(table_start, PAGE_SIZE);
+ /* after rounding to PAGE_SIZE, it is over the bar->size,
+ * fall back to the MSI-X table offset in the bar and
+ * align with PAGE_SIZE.
+ */
Minor nitpick - wording of comment could be better, for example:
if page-aligned start of MSI-X table is beyond BAR size, shrink the
mapping size to MSI-X table start address.
Also, probably needs newline before comment.
+ if (table_start >= bar->size) {
+ table_start = RTE_ALIGN_FLOOR(msix_table->offset,
+ PAGE_SIZE);
+ /* after aligning with PAGE_SIZE, if it is less than
+ * the MSI-X table offset, continue falling back to
+ * the actual MSI-X table offset in the bar.
+ */
Same here, wording could probably be improved. Suggested rewording:
If MSI-X table address, floor-aligned by page size, is lower than actual
MSI-X table offset, fall back to using MSI-X table offset as table start.
Now that i think of it, this could really be expressed like this:
uint32_t aligned = RTE_ALIGN_FLOOR(msix_table->offset, PAGE_SIZE);
table_start = RTE_MAX(aligned, msix_table_offset);
I believe this would be much clearer.
+ if (table_start < msix_table->offset)
+ table_start = msix_table->offset;
+ }
if (table_start == 0 && table_end >= bar->size) {
/* Cannot map this BAR */
@@ -469,8 +485,18 @@ pci_vfio_mmap_bar(int vfio_dev_fd, struct
mapped_pci_resource *vfio_res,
memreg[0].offset = bar->offset;
memreg[0].size = table_start;
- memreg[1].offset = bar->offset + table_end;
- memreg[1].size = bar->size - table_end;
+ if (bar->size < table_end) {
+ /*
+ * after rounding to PAGE_SIZE we don't have any space
+ * left after the MSI table, so don't try and map it.
+ */
Suggested rewording:
If MSI-X table end is beyond BAR end, don't attempt to perform second
mapping.
+ memreg[1].offset = 0;
+ memreg[1].size = 0;
+ }
+ else {
+ memreg[1].offset = bar->offset + table_end;
+ memreg[1].size = bar->size - table_end;
+ }
RTE_LOG(DEBUG, EAL,
"Trying to map BAR%d that contains the MSI-X "
However, the patch can go in as is if needed, so
Reviewed-by: Anatoly Burakov <anatoly.bura...@intel.com>
--
Thanks,
Anatoly
