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
