From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <t.yoshimura8869@gmail.com>
Received: from mail-pg1-f196.google.com (mail-pg1-f196.google.com
[209.85.215.196]) by dpdk.org (Postfix) with ESMTP id 7C5A4160
for <dev@dpdk.org>; Tue, 17 Jul 2018 10:21:53 +0200 (CEST)
Received: by mail-pg1-f196.google.com with SMTP id k3-v6so132197pgq.5
for <dev@dpdk.org>; Tue, 17 Jul 2018 01:21:53 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
h=mime-version:in-reply-to:references:from:date:message-id:subject:to
:cc; bh=Bl9cdGM4GzgVPwHiJJisA9IWq0pTdPmi11W6dW0EEVQ=;
b=fbAFUsNKD+SHgMprnOuoA79a62bZcTlMPyk/2Fqy9WHSCwcBsNMsNcSv3Jc4jojLYl
Opblm8CI+4mQqg0i2IqdxxEF3oub73QCUk15AKH4GhAx9falxTjB0rCDfNZ0c9ySIsDO
g314VQkZA3wmkYTbqAJP/lUhevTrtp85J/YOxG9MN4wg3weHAOwz6UHWNjYh77xnxbR1
tMjC1UtYVdORlXM3/9WI8r/Hq9uY+kt6FYX/6eh+A/0zDdMClo6uJX236LdzyItfxXtN
HfuuAHVvknNNFIz8N1+GOUhC9X0xn6k3QkilIbh8JRuuRxvBWseQ6uBxMqxibuXWaTVQ
QjGg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20161025;
h=x-gm-message-state:mime-version:in-reply-to:references:from:date
:message-id:subject:to:cc;
bh=Bl9cdGM4GzgVPwHiJJisA9IWq0pTdPmi11W6dW0EEVQ=;
b=GX7l82DGQMire6tHl3+7J+lzWRrSWnb4fUsZLH2mf/xaxDhL7jST/O1KpBPYjD9yd5
s/OvoFh2E26vGwTbCNrUq8ZPag9tap9GfDnINhCbXOjNtLDigI0Q6vrtUD9qtI6rCxH/
RvLmVgry6+p+rYRpUX+PXmDVH3U/ASk6GWbxKHLJ+Kh5DqWfphXxZX4KAkeWO5o+fHLh
W1jumWAaAG6MAl/rJJcDTQtvaoz6B/XY0CT3E8zfeDpqzuqNAxjmtsv5iEz1Hmrbx2xG
B9f9zgvYBbtLAA4O6sz9QNHMuT7EuJaq5nLMqlR2KdriG+RItnqyNp4/Ff3xzwDgMVJi
JNNg==
X-Gm-Message-State: AOUpUlFXNcpoe5AauZzGR2j7QpgwI4v6Fcd5U3H1FvUUkvgQIKXgxgPn
9vx4QMCCMyJPf6ovZp3yl+3SUoDJobs7eCvPzjI=
X-Google-Smtp-Source: AAOMgpc1DpZXXguhHqVBHIA+h2YVqOnbJyJ7zFU3WeYvzRAVWoZ+klkld+XmjjbQd7Y3WfZZEAyKvghFspmPfAhXmCw=
X-Received: by 2002:a63:214f:: with SMTP id
s15-v6mr624420pgm.267.1531815712537;
Tue, 17 Jul 2018 01:21:52 -0700 (PDT)
MIME-Version: 1.0
Received: by 2002:a17:90a:22a8:0:0:0:0 with HTTP; Tue, 17 Jul 2018 01:21:52
-0700 (PDT)
In-Reply-To: <fa6ca8c8-8614-ab5b-ca87-e3e1f1e37248@intel.com>
References: <20180712030833.4887-1-t.yoshimura8869@gmail.com>
<20180713101145.4795-1-t.yoshimura8869@gmail.com>
<4c46da4e-ab67-bb76-b42a-25646c79cd99@intel.com>
<fa6ca8c8-8614-ab5b-ca87-e3e1f1e37248@intel.com>
From: Takeshi Yoshimura <t.yoshimura8869@gmail.com>
Date: Tue, 17 Jul 2018 17:21:52 +0900
Message-ID: <CALBPOTVLpMTf5D1FfrsuEEzeSHf-_VJXzm=amw2JzcMEuHBi+Q@mail.gmail.com>
To: "Burakov, Anatoly" <anatoly.burakov@intel.com>
Cc: dev@dpdk.org
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [dpdk-dev] [PATCH v3] vfio: fix workaround of BAR0 mapping
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 17 Jul 2018 08:21:53 -0000
2018-07-13 20:08 GMT+09:00 Burakov, Anatoly <anatoly.burakov@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.yoshimura8869@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