From: Logan Gunthorpe <logang@deltatee.com>
To: Bjorn Helgaas <helgaas@kernel.org>,
Dongdong Liu <liudongdong3@huawei.com>
Cc: hch@infradead.org, kw@linux.com, leon@kernel.org,
linux-pci@vger.kernel.org, rajur@chelsio.com,
hverkuil-cisco@xs4all.nl, linux-media@vger.kernel.org,
netdev@vger.kernel.org
Subject: Re: [PATCH V10 6/8] PCI/P2PDMA: Add a 10-Bit Tag check in P2PDMA
Date: Wed, 27 Oct 2021 17:41:07 -0600 [thread overview]
Message-ID: <136155cc-d28c-ef36-c69b-557f7af456be@deltatee.com> (raw)
In-Reply-To: <20211027231134.GA258571@bhelgaas>
On 2021-10-27 5:11 p.m., Bjorn Helgaas wrote:
>> @@ -532,6 +577,9 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
>> map_type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
>> }
>> done:
>> + if (pci_10bit_tags_unsupported(client, provider, verbose))
>> + map_type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
>
> I need to be convinced that this check is in the right spot to catch
> all potential P2PDMA situations. The pci_p2pmem_find() and
> pci_p2pdma_distance() interfaces eventually call
> calc_map_type_and_dist(). But those interfaces don't actually produce
> DMA bus addresses, and I'm not convinced that all P2PDMA users use
> them.
>
> nvme *does* use them, but infiniband (rdma_rw_map_sg()) does not, and
> it calls pci_p2pdma_map_sg().
The rules of the current code is that calc_map_type_and_dist() must be
called before pci_p2pdma_map_sg(). The calc function caches the mapping
type in an xarray. If it was not called ahead of time,
pci_p2pdma_map_type() will return PCI_P2PDMA_MAP_NOT_SUPPORTED, and the
WARN_ON_ONCE will be hit in
pci_p2pdma_map_sg_attrs().
Both NVMe and RDMA (only used in the nvme fabrics code) do the correct
thing here and we can be sure calc_map_type_and_dist() is called before
any pages are mapped.
The patch set I'm currently working on will ensure that
calc_map_type_and_dist() is called before anyone maps a PCI P2PDMA page
with dma_map_sg*().
> amdgpu_dma_buf_attach() calls pci_p2pdma_distance_many() but I don't
> know where it sets up P2PDMA transactions.
The amdgpu driver hacked this in before proper support was done, but at
least it's using pci_p2pdma_distance_many() presumably before trying any
transfer. Though it's likely broken as it doesn't take into account the
mapping type and thus I think it always assumes traffic goes through the
host bridge (seeing it doesn't use pci_p2pdma_map_sg()).
> cxgb4 and qed mention "peer2peer", but I don't know whether they are
> related; they don't seem to use any pci_p2p.* interfaces.
I'm really not sure what these drivers are doing at all. However, I
think this is unrelated based on this old patch description[1]:
Open MPI, Intel MPI and other applications don't support the iWARP
requirement that the client side send the first RDMA message. This
class of application connection setup is called peer-2-peer. Typically
once the connection is setup, _both_ sides want to send data.
This patch enables supporting peer-2-peer over the chelsio rnic by
enforcing this iWARP requirement in the driver itself as part of RDMA
connection setup.
Logan
[1] http://lkml.iu.edu/hypermail/linux/kernel/0804.3/1416.html
next prev parent reply other threads:[~2021-10-27 23:41 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-09 10:49 [PATCH V10 0/8] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
2021-10-09 10:49 ` [PATCH V10 1/8] PCI: Use cached devcap in more places Dongdong Liu
2021-10-09 10:49 ` [PATCH V10 2/8] PCI: Cache Device Capabilities 2 Register Dongdong Liu
2021-10-09 10:49 ` [PATCH V10 3/8] PCI: Add 10-Bit Tag register definitions Dongdong Liu
2021-10-09 10:49 ` [PATCH V10 4/8] PCI/sysfs: Add a 10-Bit Tag sysfs file PCIe Endpoint devices Dongdong Liu
2021-10-27 22:28 ` Bjorn Helgaas
2021-10-28 7:44 ` Dongdong Liu
2021-10-28 17:24 ` Bjorn Helgaas
2021-10-29 7:16 ` Dongdong Liu
2021-10-09 10:49 ` [PATCH V10 5/8] PCI/IOV: Add 10-Bit Tag sysfs files for VF devices Dongdong Liu
2021-10-09 10:49 ` [PATCH V10 6/8] PCI/P2PDMA: Add a 10-Bit Tag check in P2PDMA Dongdong Liu
2021-10-27 21:20 ` Bjorn Helgaas
2021-10-28 7:56 ` Dongdong Liu
2021-10-27 23:11 ` Bjorn Helgaas
2021-10-27 23:41 ` Logan Gunthorpe [this message]
2021-10-28 1:39 ` Bjorn Helgaas
2021-10-28 15:56 ` Logan Gunthorpe
2021-10-09 10:49 ` [PATCH V10 7/8] PCI: Enable 10-Bit Tag support for PCIe Endpoint device Dongdong Liu
2021-10-09 10:49 ` [PATCH V10 8/8] PCI/IOV: Enable 10-Bit Tag support for PCIe VF devices Dongdong Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=136155cc-d28c-ef36-c69b-557f7af456be@deltatee.com \
--to=logang@deltatee.com \
--cc=hch@infradead.org \
--cc=helgaas@kernel.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=kw@linux.com \
--cc=leon@kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=liudongdong3@huawei.com \
--cc=netdev@vger.kernel.org \
--cc=rajur@chelsio.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).