10/07/2020 12:07, Thomas Monjalon: > 10/07/2020 11:54, David Marchand: > > On Wed, Jul 8, 2020 at 11:26 AM <alvinx.zh...@intel.com> wrote: > > > From: Alvin Zhang <alvinx.zh...@intel.com> > > > > > > When mapping a PCI BAR containing an MSI-X table, some devices do not > > > need to actually map this BAR or only need to map part of them, which > > > may cause the mapping to fail. Now some checks are added and a non-NULL > > > initial value is set to the variable to avoid this situation. [...] > > > --- a/drivers/bus/pci/linux/pci_vfio.c > > > +++ b/drivers/bus/pci/linux/pci_vfio.c > > > @@ -547,6 +547,14 @@ > > > bar_index, > > > memreg[0].offset, memreg[0].size, > > > memreg[1].offset, memreg[1].size); > > > + > > > + if (memreg[0].size == 0 && memreg[1].size == 0) { > > > + /* No need to map this BAR */ > > > + RTE_LOG(DEBUG, EAL, "Skipping BAR%d\n", > > > bar_index); > > > + bar->size = 0; > > > + bar->addr = 0; > > > + return 0; > > > + } > > > > We already have a check on bar size == 0. > > Why would we have this condition? > > Broken hw? > > > > > > > } else { > > > memreg[0].offset = bar->offset; > > > memreg[0].size = bar->size; > > > @@ -556,7 +564,9 @@ > > > bar_addr = mmap(bar->addr, bar->size, 0, MAP_PRIVATE | > > > MAP_ANONYMOUS | additional_flags, -1, 0); > > > if (bar_addr != MAP_FAILED) { > > > - void *map_addr = NULL; > > > + /* Set non NULL initial value for in case of no PCI > > > mapping */ > > > + void *map_addr = bar_addr; > > > + > > > > It took me some time to understand this code... > > Anyway, we have a regression in the librte_pci. > > This is where the fix should be. > > Yes, I am going to send a fix.
Patch sent: https://patches.dpdk.org/patch/73741/ This patch is marked as rejected, but please follow-up on cleanup. > > We can cleanup this code later. > > Yes please, this function isn't understandable and lack of comments. > Anatoly please?