Re: [dpdk-dev] [PATCH] bus/pci: fix mapping BAR containing MSI-X table

[David Marchand <david.marchand@redhat.com>]

On Fri, Sep 25, 2020 at 4:15 AM Hyong Youb Kim <hyonkim@cisco.com> 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 <hyonkim@cisco.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks.

-- 
David Marchand