lkml.org 
[lkml]   [2019]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 00/14] PCI/P2PDMA: Support transactions that hit the host bridge
    On Mon, Aug 12, 2019 at 11:30:34AM -0600, Logan Gunthorpe wrote:
    > Bjorn, this is v3 of the patchset. Christophs suggestion's required a
    > bit more rework than could be expressed in incremental patches so
    > I've resent the whole thing. I started with your p2pdma branch though
    > so it should have all the changes you already made. I've included a
    > range-diff from your p2pdma branch below.

    I added Christoph's reviewed-by and replaced my pci/p2pdma branch with
    this series, thanks! This is all headed for v5.4.

    > A git branch is available here:
    >
    > https://github.com/sbates130272/linux-p2pmem/ p2pdma_rc_map_v3
    >
    > --
    >
    > Changes in v3:
    > * Rebase on v5.3-rc3 (No changes)
    > * Bjorn's edits for a bunch of the comments and commit messages
    > * Rework upstream_bridge_distance() changes to split the distance,
    > mapping type and ACS flag into separate pass-by-reference parameters
    > (per Christoph's suggestion)
    > * Jonathan Derrick (Intel) reported the domains can be 32-bits on
    > some machines and thus the map_type_index() needed to be an unsigned long
    > to accomadate this.
    >
    > Changes in v2:
    > * Rebase on v5.3-rc2 (No changes)
    > * Re-introduce the private pagemap structure and move the p2p-specific
    > elements out of the commond dev_pagemap (per Christoph)
    > * Use flags instead of bool in the whitelist (per Jason)
    > * Only store the mapping type in the xarray (instead of the distance
    > with flags) such that a function can return the mapping method
    > with a switch statement to decide how to map. (per Christoph)
    > * Drop find_parent_pci_dev() on the fast path and rely on the fact
    > that the struct device passed to the mapping functions *must* be
    > a PCI device and convert it directly. (per suggestions from
    > Christoph and Jason)
    > * Collected Christian's Reviewed-by's
    >
    > --
    >
    > As discussed on the list previously, in order to fully support the
    > whitelist Christian added with the IOMMU, we must ensure that we
    > map any buffer going through the IOMMU with an aprropriate dma_map
    > call. This patchset accomplishes this by cleaning up the output of
    > upstream_bridge_distance() to better indicate the mapping requirements,
    > caching these requirements in an xarray, then looking them up at map
    > time and applying the appropriate mapping method.
    >
    > After this patchset, it's possible to use the NVMe-of P2P support to
    > transfer between devices without a switch on the whitelisted root
    > complexes. A couple Intel device I have tested this on have also
    > been added to the white list.
    >
    > Most of the changes are contained within the p2pdma.c, but there are
    > a few minor touches to other subsystems, mostly to add support
    > to call an unmap function.
    >
    > --
    >
    > Logan Gunthorpe (14):
    > PCI/P2PDMA: Introduce private pagemap structure
    > PCI/P2PDMA: Add provider's pci_dev to pci_p2pdma_pagemap struct
    > PCI/P2PDMA: Add constants for map type results to
    > upstream_bridge_distance()
    > PCI/P2PDMA: Factor out __upstream_bridge_distance()
    > PCI/P2PDMA: Apply host bridge whitelist for ACS
    > PCI/P2PDMA: Factor out host_bridge_whitelist()
    > PCI/P2PDMA: Whitelist some Intel host bridges
    > PCI/P2PDMA: Add attrs argument to pci_p2pdma_map_sg()
    > PCI/P2PDMA: Introduce pci_p2pdma_unmap_sg()
    > PCI/P2PDMA: Factor out __pci_p2pdma_map_sg()
    > PCI/P2PDMA: Store mapping method in an xarray
    > PCI/P2PDMA: dma_map() requests that traverse the host bridge
    > PCI/P2PDMA: Allow IOMMU for host bridge whitelist
    > PCI/P2PDMA: Update pci_p2pdma_distance_many() documentation
    >
    > drivers/infiniband/core/rw.c | 6 +-
    > drivers/nvme/host/pci.c | 10 +-
    > drivers/pci/p2pdma.c | 374 +++++++++++++++++++++++++----------
    > include/linux/memremap.h | 1 -
    > include/linux/pci-p2pdma.h | 28 ++-
    > 5 files changed, 300 insertions(+), 119 deletions(-)
    >
    > --
    >
    > $ git range-diff bjorn/master..bjorn/pci/p2pdma master..p2pdma_rc_map_v3
    >
    > 1: 5173c82b3571 = 1: 78cc3a5be538 PCI/P2PDMA: Introduce private pagemap structure
    > 2: f33fd6b8da93 = 2: 9fb388dff957 PCI/P2PDMA: Add provider's pci_dev to pci_p2pdma_pagemap struct
    > 3: 767f47b59702 < -: ------------ PCI/P2PDMA: Add constants for not-supported result upstream_bridge_distance()
    > -: ------------ > 3: ed105432f644 PCI/P2PDMA: Add constants for map type results to upstream_bridge_distance()
    > 4: 93ed41974d69 ! 4: 590aaea31997 PCI/P2PDMA: Factor out __upstream_bridge_distance()
    > @@ -17,8 +17,8 @@
    > --- a/drivers/pci/p2pdma.c
    > +++ b/drivers/pci/p2pdma.c
    > @@
    > - P2PDMA_NOT_SUPPORTED = 0x08000000,
    > - };
    > + return false;
    > + }
    >
    > -/*
    > - * Find the distance through the nearest common upstream bridge between
    > @@ -44,31 +44,29 @@
    > - * port of the switch, to the common upstream port, back up to the second
    > - * downstream port and then to Device B.
    > - *
    > -- * Any two devices that cannot communicate using p2pdma will return the
    > -- * distance with the flag P2PDMA_NOT_SUPPORTED.
    > +- * Any two devices that cannot communicate using p2pdma will return
    > +- * PCI_P2PDMA_MAP_NOT_SUPPORTED.
    > - *
    > - * Any two devices that have a data path that goes through the host bridge
    > - * will consult a whitelist. If the host bridges are on the whitelist,
    > -- * the distance will be returned with the flag P2PDMA_THRU_HOST_BRIDGE set.
    > -- * If either bridge is not on the whitelist, the flag P2PDMA_NOT_SUPPORTED will
    > -- * be set.
    > +- * this function will return PCI_P2PDMA_MAP_THRU_HOST_BRIDGE.
    > +- *
    > +- * If either bridge is not on the whitelist this function returns
    > +- * PCI_P2PDMA_MAP_NOT_SUPPORTED.
    > - *
    > -- * If a bridge which has any ACS redirection bits set is in the path
    > -- * this function will flag the result with P2PDMA_ACS_FORCES_UPSTREAM.
    > -- * In this case, a list of all infringing bridge addresses will be
    > -- * populated in acs_list (assuming it's non-null) for printk purposes.
    > +- * If a bridge which has any ACS redirection bits set is in the path,
    > +- * acs_redirects will be set to true. In this case, a list of all infringing
    > +- * bridge addresses will be populated in acs_list (assuming it's non-null)
    > +- * for printk purposes.
    > - */
    > --static int upstream_bridge_distance(struct pci_dev *provider,
    > -- struct pci_dev *client,
    > -- struct seq_buf *acs_list)
    > -+static int __upstream_bridge_distance(struct pci_dev *provider,
    > -+ struct pci_dev *client,
    > -+ struct seq_buf *acs_list)
    > + static enum pci_p2pdma_map_type
    > +-upstream_bridge_distance(struct pci_dev *provider, struct pci_dev *client,
    > ++__upstream_bridge_distance(struct pci_dev *provider, struct pci_dev *client,
    > + int *dist, bool *acs_redirects, struct seq_buf *acs_list)
    > {
    > struct pci_dev *a = provider, *b = client, *bb;
    > - int dist_a = 0;
    > @@
    > - return dist_a + dist_b;
    > + return PCI_P2PDMA_MAP_BUS_ADDR;
    > }
    >
    > +/*
    > @@ -95,27 +93,29 @@
    > + * port of the switch, to the common upstream port, back up to the second
    > + * downstream port and then to Device B.
    > + *
    > -+ * Any two devices that cannot communicate using p2pdma will return the
    > -+ * distance with the flag P2PDMA_NOT_SUPPORTED.
    > ++ * Any two devices that cannot communicate using p2pdma will return
    > ++ * PCI_P2PDMA_MAP_NOT_SUPPORTED.
    > + *
    > + * Any two devices that have a data path that goes through the host bridge
    > + * will consult a whitelist. If the host bridges are on the whitelist,
    > -+ * the distance will be returned with the flag P2PDMA_THRU_HOST_BRIDGE set.
    > -+ * If either bridge is not on the whitelist, the flag P2PDMA_NOT_SUPPORTED will
    > -+ * be set.
    > ++ * this function will return PCI_P2PDMA_MAP_THRU_HOST_BRIDGE.
    > + *
    > -+ * If a bridge which has any ACS redirection bits set is in the path
    > -+ * this function will flag the result with P2PDMA_ACS_FORCES_UPSTREAM.
    > -+ * In this case, a list of all infringing bridge addresses will be
    > -+ * populated in acs_list (assuming it's non-null) for printk purposes.
    > ++ * If either bridge is not on the whitelist this function returns
    > ++ * PCI_P2PDMA_MAP_NOT_SUPPORTED.
    > ++ *
    > ++ * If a bridge which has any ACS redirection bits set is in the path,
    > ++ * acs_redirects will be set to true. In this case, a list of all infringing
    > ++ * bridge addresses will be populated in acs_list (assuming it's non-null)
    > ++ * for printk purposes.
    > + */
    > -+static int upstream_bridge_distance(struct pci_dev *provider,
    > -+ struct pci_dev *client,
    > -+ struct seq_buf *acs_list)
    > ++static enum pci_p2pdma_map_type
    > ++upstream_bridge_distance(struct pci_dev *provider, struct pci_dev *client,
    > ++ int *dist, bool *acs_redirects, struct seq_buf *acs_list)
    > +{
    > -+ return __upstream_bridge_distance(provider, client, acs_list);
    > ++ return __upstream_bridge_distance(provider, client, dist,
    > ++ acs_redirects, acs_list);
    > +}
    > +
    > - static int upstream_bridge_distance_warn(struct pci_dev *provider,
    > - struct pci_dev *client)
    > - {
    > + static enum pci_p2pdma_map_type
    > + upstream_bridge_distance_warn(struct pci_dev *provider, struct pci_dev *client,
    > + int *dist)
    > 5: f975e51cf5df < -: ------------ PCI/P2PDMA: Apply host bridge whitelist for ACS
    > -: ------------ > 5: 336e968f075b PCI/P2PDMA: Apply host bridge whitelist for ACS
    > 6: 59b6507ac07c ! 6: 3f565ce6e1d4 PCI/P2PDMA: Factor out host_bridge_whitelist()
    > @@ -58,15 +58,16 @@
    > + return false;
    > +}
    > +
    > - enum {
    > - /*
    > - * These arbitrary offset are or'd onto the upstream distance
    > + static enum pci_p2pdma_map_type
    > + __upstream_bridge_distance(struct pci_dev *provider, struct pci_dev *client,
    > + int *dist, bool *acs_redirects, struct seq_buf *acs_list)
    > @@
    > - if (!(dist & P2PDMA_THRU_HOST_BRIDGE))
    > - return dist;
    > + acs_redirects, acs_list);
    >
    > -- if (root_complex_whitelist(provider) && root_complex_whitelist(client))
    > -+ if (host_bridge_whitelist(provider, client))
    > - return dist;
    > + if (map_type == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE) {
    > +- if (!root_complex_whitelist(provider) ||
    > +- !root_complex_whitelist(client))
    > ++ if (!host_bridge_whitelist(provider, client))
    > + return PCI_P2PDMA_MAP_NOT_SUPPORTED;
    > + }
    >
    > - return dist | P2PDMA_NOT_SUPPORTED;
    > 7: 5ecec567445f = 7: 7b8701dd45e0 PCI/P2PDMA: Whitelist some Intel host bridges
    > 8: 68a37758d5cf = 8: b52771ae7f95 PCI/P2PDMA: Add attrs argument to pci_p2pdma_map_sg()
    > 9: 4b821298d4f7 = 9: 88a554eb39c9 PCI/P2PDMA: Introduce pci_p2pdma_unmap_sg()
    > 10: 3f2dac803737 = 10: 604260bd186a PCI/P2PDMA: Factor out __pci_p2pdma_map_sg()
    > 11: fc402d621534 ! 11: b55fc423a41a PCI/P2PDMA: Store mapping method in an xarray
    > @@ -18,14 +18,10 @@
    > #include <linux/seq_buf.h>
    > #include <linux/iommu.h>
    > +#include <linux/xarray.h>
    > -+
    > -+enum pci_p2pdma_map_type {
    > -+ PCI_P2PDMA_MAP_UNKNOWN = 0,
    > -+ PCI_P2PDMA_MAP_NOT_SUPPORTED,
    > -+ PCI_P2PDMA_MAP_BUS_ADDR,
    > -+ PCI_P2PDMA_MAP_THRU_IOMMU,
    > -+};
    >
    > + enum pci_p2pdma_map_type {
    > + PCI_P2PDMA_MAP_UNKNOWN = 0,
    > +@@
    > struct pci_p2pdma {
    > struct gen_pool *pool;
    > bool p2pmem_published;
    > @@ -51,10 +47,10 @@
    > if (!p2p->pool)
    > goto out;
    > @@
    > - return dist_a + dist_b;
    > + return PCI_P2PDMA_MAP_BUS_ADDR;
    > }
    >
    > -+static int map_types_idx(struct pci_dev *client)
    > ++static unsigned long map_types_idx(struct pci_dev *client)
    > +{
    > + return (pci_domain_nr(client->bus) << 16) |
    > + (client->bus->number << 8) | client->devfn;
    > @@ -64,37 +60,17 @@
    > * Find the distance through the nearest common upstream bridge between
    > * two PCI devices.
    > @@
    > - struct pci_dev *client,
    > - struct seq_buf *acs_list)
    > - {
    > -+ enum pci_p2pdma_map_type map_type;
    > - int dist;
    > -
    > - dist = __upstream_bridge_distance(provider, client, acs_list);
    >
    > -- if (!(dist & P2PDMA_THRU_HOST_BRIDGE))
    > -- return dist;
    > -+ if (!(dist & P2PDMA_THRU_HOST_BRIDGE)) {
    > -+ map_type = PCI_P2PDMA_MAP_BUS_ADDR;
    > -+ goto store_map_type_and_return;
    > -+ }
    > -+
    > -+ if (host_bridge_whitelist(provider, client)) {
    > -+ map_type = PCI_P2PDMA_MAP_THRU_IOMMU;
    > -+ } else {
    > -+ dist |= P2PDMA_NOT_SUPPORTED;
    > -+ map_type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
    > -+ }
    > + if (map_type == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE) {
    > + if (!host_bridge_whitelist(provider, client))
    > +- return PCI_P2PDMA_MAP_NOT_SUPPORTED;
    > ++ map_type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
    > + }
    >
    > -- if (host_bridge_whitelist(provider, client))
    > -- return dist;
    > -+store_map_type_and_return:
    > + if (provider->p2pdma)
    > + xa_store(&provider->p2pdma->map_types, map_types_idx(client),
    > + xa_mk_value(map_type), GFP_KERNEL);
    > -
    > -- return dist | P2PDMA_NOT_SUPPORTED;
    > -+ return dist;
    > ++
    > + return map_type;
    > }
    >
    > - static int upstream_bridge_distance_warn(struct pci_dev *provider,
    > 12: c51eb851e9da ! 12: 94e4c8633459 PCI/P2PDMA: dma_map() requests that traverse the host bridge
    > @@ -44,7 +44,7 @@
    > + client = to_pci_dev(dev);
    > +
    > + switch (pci_p2pdma_map_type(p2p_pgmap->provider, client)) {
    > -+ case PCI_P2PDMA_MAP_THRU_IOMMU:
    > ++ case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
    > + return dma_map_sg_attrs(dev, sg, nents, dir, attrs);
    > + case PCI_P2PDMA_MAP_BUS_ADDR:
    > + return __pci_p2pdma_map_sg(p2p_pgmap, dev, sg, nents);
    > @@ -71,7 +71,7 @@
    > +
    > + map_type = pci_p2pdma_map_type(p2p_pgmap->provider, client);
    > +
    > -+ if (map_type == PCI_P2PDMA_MAP_THRU_IOMMU)
    > ++ if (map_type == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE)
    > + dma_unmap_sg_attrs(dev, sg, nents, dir, attrs);
    > }
    > EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_sg_attrs);
    > 13: 0a3468f51621 = 13: f067cdb5b963 PCI/P2PDMA: Allow IOMMU for host bridge whitelist
    > 14: 20c0cf9f4c9c = 14: 87c84b001dfa PCI/P2PDMA: Update pci_p2pdma_distance_many() documentation
    >
    > --
    > 2.20.1

    \
     
     \ /
      Last update: 2019-08-16 15:46    [W:2.377 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site