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

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 25-Sep-20 3:14 AM, Hyong Youb Kim 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>
> ---

LGTM

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly