2018-07-13 20:08 GMT+09:00 Burakov, Anatoly <anatoly.bura...@intel.com>: > On 13-Jul-18 12:00 PM, Burakov, Anatoly wrote: >> >> On 13-Jul-18 11:11 AM, Takeshi Yoshimura wrote: >>> >>> The workaround of BAR0 mapping gives up and immediately returns an >>> error if it cannot map around the MSI-X. However, recent version >>> of VFIO allows MSIX mapping (*). >>> >>> I fixed not to return immediately but try mapping. In old Linux, mmap >>> just fails and returns the same error as the code before my fix . In >>> recent Linux, mmap succeeds and this patch enables running DPDK in >>> specific environments (e.g., ppc64le with HGST NVMe) >>> >>> (*): "vfio-pci: Allow mapping MSIX BAR", >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ >>> commit/id=a32295c612c57990d17fb0f41e7134394b2f35f6 >>> >>> Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables") >>> >>> Signed-off-by: Takeshi Yoshimura <t.yoshimura8...@gmail.com> >>> --- >>> >>> Thanks, Anatoly. >>> >>> I updated the patch not to affect behaviors of older Linux and >>> other environments as well as possible. This patch adds another >>> chance to mmap BAR0. >>> >>> I noticed that the check at line 350 already includes the check >>> of page size, so this patch does not fix the check. >>> >>> Regards, >>> Takeshi >> >> >> Hi Takeshi, >> >> Please correct me if i'm wrong, but i'm not sure the old behavior is kept. >> >> Let's say we're running an old kernel, which doesn't allow mapping MSI-X >> BARs. If MSI-X starts at beginning of the BAR (floor-aligned to page size), >> and ends at or beyond end of BAR (ceiling-aligned to page size). In that >> situation, old code just skipped the BAR and returned 0. >> >> We then exited the function, and there's a check for return value right >> after pci_vfio_mmap_bar() that stop continuing if we fail to map something. >> In the old code, we would continue as we went, and finish the rest of our >> mappings. With your new code, you're attempting to map the BAR, it fails, >> and you will return -1 on older kernels. >> >> I believe what we really need here is the following: >> >> 1) If this is a BAR containing MSI-X vector, first try mapping the entire >> BAR. If it succeeds, great - that would be your new kernel behavior. >> 2) If we failed on step 1), check to see if we can map around the BAR. If >> we can, try to map around it like the current code does. If we cannot map >> around it (i.e. if MSI-X vector, page aligned, occupies entire BAR), then we >> simply return 0 and skip the BAR. >> >> That, i would think, would keep the old behavior and enable the new one. >> >> Does that make sense? >> > > I envision this to look something like this: > > bool again = false; > do { > if (again) { > // set up mmap-around > if (cannot map around) > return 0; > } > // try mapping > if (map_failed && msix_table->bar_index == bar_index) { > again = true; > continue; > } > if (map_failed) > return -1; > break/return 0; > } while (again); > > -- > Thanks, > Anatoly
That makes sense. The return code was not same as old one in some paths. I wrote a code based on your idea. It works at least in my ppc64 and x86 machines, but I am concerned that the error messages for pci_map_resource() confuse users in old Linux. I saw a message like this (even if I could mmap): EAL: pci_map_resource(): cannot mmap(15, 0x728ee3a30000, 0x4000, 0x0): Invalid argument (0xffffffffffffffff) But anyway, I send it in the next email, and please check if there is any other problems in the code. Thanks, Takeshi
