On Fri, Sep 25, 2020 at 4:15 AM Hyong Youb Kim <[email protected]> wrote: > > When the BAR contains MSI-X table, pci_vfio_mmap_bar() tries to skip > the table and map the rest. "map around it" is the phrase used in the > source. The function splits the BAR into two regions: the region > before the table (first part or memreg[0]) and the region after the > table (second part or memreg[1]). > > For hardware that has MSI-X vector table offset 0, the first part does > not exist (memreg[0].size == 0). > > Capabilities: [60] MSI-X: Enable- Count=48 Masked- > Vector table: BAR=2 offset=00000000 > PBA: BAR=2 offset=00001000 > > The mapping part of the function maps the first part, if it > exists. Then, it maps the second part, if it exists and "if mapping the > first part succeeded". > > The recent change that replaces MAP_FAILED with NULL breaks the "if > mapping the first part succeeded" condition (1) in the snippet below. > > void *map_addr = NULL; > if (memreg[0].size) { > /* actual map of first part */ > map_addr = pci_map_resource(...); > } > > /* if there's a second part, try to map it */ > if (map_addr != NULL // -- (1) > && memreg[1].offset && memreg[1].size) { > [...] > } > > if (map_addr == NULL) { > RTE_LOG(ERR, EAL, "Failed to map pci BAR%d\n", > bar_index); > return -1; > } > > When the first part does not exist, (1) sees map_addr is still NULL, > and the function fails. This behavior is a regression and fails > probing hardware with vector table offset 0. > > Previously, (1) was "map_addr != MAP_FAILED", which meant > pci_map_resource() was actually attempted and failed. So, expand (1) > to check if the first part exists as well, to match the semantics of > MAP_FAILED. > > Bugzilla ID: 539 > Fixes: e200535c1ca3 ("mem: drop mapping API workaround") > > Signed-off-by: Hyong Youb Kim <[email protected]> Acked-by: Anatoly Burakov <[email protected]>
Applied, thanks. -- David Marchand
