Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1624403 > unrolled thread
| Started by | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| First post | 2017-04-16 17:50 +0200 |
| Last post | 2017-04-20 02:10 +0200 |
| Articles | 14 on this page of 74 — 7 participants |
Back to article view | Back to linux.kernel
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-16 17:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-16 18:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-17 00:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-17 07:20 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-17 09:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-17 19:00 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-17 19:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 07:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jerome Glisse <jglisse@redhat.com> - 2017-04-17 20:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 08:20 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-17 23:20 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 07:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-18 08:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-17 00:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 18:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 19:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 20:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 20:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 03:20 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 01:00 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 02:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 20:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 21:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 21:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 21:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 22:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 21:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jerome Glisse <jglisse@redhat.com> - 2017-04-18 22:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 22:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 22:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 03:20 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 23:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 23:20 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-18 23:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-18 23:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 00:20 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 00:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 00:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 01:00 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 01:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 03:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 00:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 01:00 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 01:00 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 01:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 03:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-18 23:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 00:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 01:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 04:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-04-19 03:30 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 18:00 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 18:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 19:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jerome Glisse <jglisse@redhat.com> - 2017-04-19 19:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 19:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 20:20 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 20:20 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 20:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-19 20:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 20:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-20 22:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory "Stephen Bates" <sbates@raithlin.com> - 2017-04-21 01:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@intel.com> - 2017-04-21 07:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory "Stephen Bates" <sbates@raithlin.com> - 2017-04-20 22:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 19:20 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 20:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 20:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 21:10 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 21:40 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-19 21:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-19 22:50 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Logan Gunthorpe <logang@deltatee.com> - 2017-04-20 01:00 +0200
Re: [RFC 0/8] Copy Offload with Peer-to-Peer PCI Memory Dan Williams <dan.j.williams@gmail.com> - 2017-04-20 02:10 +0200
Page 4 of 4 — ← Prev page 1 2 3 [4]
| From | Logan Gunthorpe <logang@deltatee.com> |
|---|---|
| Date | 2017-04-19 20:50 +0200 |
| Message-ID | <ty2PT-6TL-11@gated-at.bofh.it> |
| In reply to | #1626690 |
On 19/04/17 12:30 PM, Dan Williams wrote: > Letting others users do the container_of() arrangement means that > struct page_map needs to become public and move into struct > dev_pagemap directly. Ah, yes, I got a bit turned around by that and failed to notice that page_map and dev_pagemap are different. Why is it that dev_pagemap contains pretty much the exact same information as page_map? The only thing gained that I can see is that the struct resource gains const protection... > ...I think that encapsulation loss is worth it for the gain of clearly > separating the HMM-case from the base case. Agreed. Logan
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-04-20 22:50 +0200 |
| Message-ID | <tyrbA-59o-21@gated-at.bofh.it> |
| In reply to | #1626645 |
On Thu, Apr 20, 2017 at 1:43 PM, Stephen Bates <sbates@raithlin.com> wrote: > >> Yes, this makes sense I think we really just want to distinguish host >> memory or not in terms of the dev_pagemap type. > > I would like to see mutually exclusive flags for host memory (or not) and persistence (or not). > Why persistence? It has zero meaning to the mm.
[toc] | [prev] | [next] | [standalone]
| From | "Stephen Bates" <sbates@raithlin.com> |
|---|---|
| Date | 2017-04-21 01:10 +0200 |
| Message-ID | <tytn3-6Ho-7@gated-at.bofh.it> |
| In reply to | #1627812 |
>> Yes, this makes sense I think we really just want to distinguish host >> memory or not in terms of the dev_pagemap type. > >> I would like to see mutually exclusive flags for host memory (or not) and persistence (or not). >> > > Why persistence? It has zero meaning to the mm. I like the idea of having properties of the memory in one place. While mm might not use persistence today it may make use certain things that persistence implies (like finite endurance and/or higher write latency) in the future. Also the persistence of the memory must have issues for mm security? Again not addressed today but useful in the future. In addition I am not sure where else would be an appropriate place to put something like a persistence property flag. I know the NVDIMM section of the kernel uses things like NFIT to describe properties of the memory but we don’t yet (to my knowledge) have something similar for IO memory. Stephen
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2017-04-21 07:10 +0200 |
| Message-ID | <tyyZr-1MT-7@gated-at.bofh.it> |
| In reply to | #1627863 |
On Thu, Apr 20, 2017 at 4:07 PM, Stephen Bates <sbates@raithlin.com> wrote: >>> Yes, this makes sense I think we really just want to distinguish host >>> memory or not in terms of the dev_pagemap type. >> >>> I would like to see mutually exclusive flags for host memory (or not) and persistence (or not). >>> >> >> Why persistence? It has zero meaning to the mm. > > I like the idea of having properties of the memory in one place. We do have memory type data in the global iomem_resource tree, see IORES_DESC_PERSISTENT_MEMORY. > While mm might not use persistence today it may make use certain things that > persistence implies (like finite endurance and/or higher write latency) in the future. A persistence flag does not convey endurance or latency information. > Also the persistence of the memory must have issues for mm security? Not for the mm, data at rest security might be a property of the device, but that's not the mm's concern. >Again not addressed today but useful in the future. Maybe, but to me "Useful for the future" == "don't add it to the kernel until that future arrives". > In addition I am not sure where else would be an appropriate place to put something like a persistence property flag. I know the NVDIMM section of the kernel uses things like NFIT to describe properties of the memory but we don’t yet (to my knowledge) have something similar for IO memory. Do the IORES_DESC flags give you what you need?
[toc] | [prev] | [next] | [standalone]
| From | "Stephen Bates" <sbates@raithlin.com> |
|---|---|
| Date | 2017-04-20 22:50 +0200 |
| Message-ID | <tyrbA-59o-23@gated-at.bofh.it> |
| In reply to | #1626645 |
> Yes, this makes sense I think we really just want to distinguish host > memory or not in terms of the dev_pagemap type. I would like to see mutually exclusive flags for host memory (or not) and persistence (or not). Stephen
[toc] | [prev] | [next] | [standalone]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2017-04-19 19:20 +0200 |
| Message-ID | <ty1qO-6bq-29@gated-at.bofh.it> |
| In reply to | #1626510 |
On Wed, Apr 19, 2017 at 10:48:51AM -0600, Logan Gunthorpe wrote:
> The pci_enable_p2p_bar function would then just need to call
> devm_memremap_pages with the dma_map callback set to a function that
> does the segment check and the offset calculation.
I don't see a use for the dma_map function pointer at this point..
It doesn't make alot of sense for the completor of the DMA to provide
a mapping op, the mapping process is *path* specific, not specific to
a completer/initiator.
So, I would suggest more like this:
static inline struct device *get_p2p_src(struct page *page)
{
struct device *res;
struct dev_pagemap *pgmap;
if (!is_zone_device_page(page))
return NULL;
pgmap = get_dev_pagemap(page_to_pfn(page), NULL);
if (!pgmap || pgmap->type != MEMORY_DEVICE_P2P)
/* For now ZONE_DEVICE memory that is not P2P is
assumed to be configured for DMA the same as CPU
memory. */
return ERR_PTR(-EINVAL);
res = pgmap->dev;
device_get(res);
put_dev_pagemap(pgmap);
return res;
}
dma_addr_t pci_p2p_same_segment(struct device *initator,
struct device *completer,
struct page *page)
{
if (! PCI initiator & completer)
return ERROR;
if (!same segment initiator & completer)
return ERROR;
// Translate page directly to the value programmed into the BAR
return (Completer's PCI BAR base address) + (offset of page within BAR);
}
// dma_sg_map
for (each sgl) {
struct page *page = sg_page(s);
struct device *p2p_src = get_p2p_src(page);
if (IS_ERR(p2p_src))
// fail dma_sg
if (p2p_src) {
bool needs_iommu = false;
pa = pci_p2p_same_segment(dev, p2p_src, page);
if (pa == ERROR)
pa = arch_p2p_cross_segment(dev, p2psrc, page, &needs_iommui);
device_put(p2p_src);
if (pa == ERROR)
// fail
if (!needs_iommu) {
// Insert PA directly into the result SGL
sg++;
continue;
}
}
else
// CPU memory
pa = page_to_phys(page);
To me it looks like the code duplication across the iommu stuff comes
from just duplicating the basic iommu algorithm in every driver.
To clean that up I think someone would need to hoist the overall sgl
loop and use more ops callbacks eg allocate_iommu_range,
assign_page_to_rage, dealloc_range, etc. This is a problem p2p makes
worse, but isn't directly causing :\
Jason
[toc] | [prev] | [next] | [standalone]
| From | Logan Gunthorpe <logang@deltatee.com> |
|---|---|
| Date | 2017-04-19 20:10 +0200 |
| Message-ID | <ty2dc-6Gz-13@gated-at.bofh.it> |
| In reply to | #1626614 |
On 19/04/17 11:14 AM, Jason Gunthorpe wrote:
> I don't see a use for the dma_map function pointer at this point..
Yes, it is kind of like designing for the future. I just find it a
little odd calling the pci functions in the iommu.
> It doesn't make alot of sense for the completor of the DMA to provide
> a mapping op, the mapping process is *path* specific, not specific to
> a completer/initiator.
I'm just spit balling here but if HMM wanted to use unaddressable memory
as a DMA target, it could set that function to create a window ine gpu
memory, then call the pci_p2p_same_segment and return the result as the
dma address.
> dma_addr_t pci_p2p_same_segment(struct device *initator,
> struct device *completer,
> struct page *page)
I'm not sure I like the name pci_p2p_same_segment. It reads as though
it's only checking if the devices are not the same segment. It also may
be that, in the future, it supports devices on different segments. I'd
call it more like pci_p2p_dma_map.
> for (each sgl) {
Thanks, this code fleshes things out nicely
> To me it looks like the code duplication across the iommu stuff comes
> from just duplicating the basic iommu algorithm in every driver.
Yes, this is true.
> To clean that up I think someone would need to hoist the overall sgl
> loop and use more ops callbacks eg allocate_iommu_range,
> assign_page_to_rage, dealloc_range, etc. This is a problem p2p makes
> worse, but isn't directly causing :\
Yup.
Logan
[toc] | [prev] | [next] | [standalone]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2017-04-19 20:40 +0200 |
| Message-ID | <ty2Gg-6Qx-71@gated-at.bofh.it> |
| In reply to | #1626667 |
On Wed, Apr 19, 2017 at 12:01:39PM -0600, Logan Gunthorpe wrote:
> I'm just spit balling here but if HMM wanted to use unaddressable memory
> as a DMA target, it could set that function to create a window ine gpu
> memory, then call the pci_p2p_same_segment and return the result as the
> dma address.
Not entirely, it would have to call through the whole process
including the arch_p2p_cross_segment()..
Maybe we can start down the road of using ops for more iommu steps
with something like this as the helper:
dma_addr_t dma_map_pa(struct device *initiator, struct page *page,
void *data)
{
struct device *completer = get_p2p_completer(page);
dma_addr_t pa;
if (IS_ERR(completer))
return SYSTEM_MEMORY;
// Or maybe ?
return init_ops->dma_map_pa(..);
// Try the generic method
pa = pci_p2p_same_segment(dev, p2p_src, page);
if (pa != ERROR)
goto out;
// Try the arch specific helper
const struct dma_map_ops *comp_ops = get_dma_ops(completer);
const struct dma_map_ops *init_ops = get_dma_ops(initiator);
/* FUTURE: Let something translate a HMM page into a DMA'ble
page, eg by mapping it into a GPU window. Maybe this
callback lives in devmap ? */
page = comp_ops->translate_dma_page(completer, page);
/* New dma_map_op is the same as arch_p2p_cross_segment in
prior version. Any arch specific data needed to program
the iommu flows through data */
pa = init_ops->p2p_cross_segment_map(completer, inititator, page, data);
out:
device_put(completer);
return pa;
}
// map_sg op:
for (each sgl) {
struct page *page = sg_page(s);
struct arch_iommu_data data = {}; // pass through to ops->p2p_cross_segment
dma_addr_t pa;
pa = dma_map_pa(dev, page, &data)
if (pa == ERROR)
// fail
if (!data.needs_iommu) {
// Insert PA directly into the result SGL
sg++;
continue;
}
// Else pa & data describe how to setup the iommu
}
> > dma_addr_t pci_p2p_same_segment(struct device *initator,
> > struct device *completer,
> > struct page *page)
>
> I'm not sure I like the name pci_p2p_same_segment. It reads as though
> it's only checking if the devices are not the same segment.
Well, that is exactly what it is doing. If it succeeds then the caller
knows the DMA will not flow outside the segment and no iommu setup/etc
is required.
That function cannot be expanded to include generic cross-segment
traffic, a new function would be needed..
Jason
[toc] | [prev] | [next] | [standalone]
| From | Logan Gunthorpe <logang@deltatee.com> |
|---|---|
| Date | 2017-04-19 21:10 +0200 |
| Message-ID | <ty39g-7fq-19@gated-at.bofh.it> |
| In reply to | #1626699 |
On 19/04/17 12:32 PM, Jason Gunthorpe wrote: > On Wed, Apr 19, 2017 at 12:01:39PM -0600, Logan Gunthorpe wrote: > Not entirely, it would have to call through the whole process > including the arch_p2p_cross_segment().. Hmm, yes. Though it's still not clear what, if anything, arch_p2p_cross_segment would be doing. In my experience, if you are going between host bridges, the CPU address (or PCI address -- I'm not sure which seeing they are the same on my system) would still work fine -- it just _may_ be a bad idea because of performance. Similarly if you are crossing via a QPI bus or similar, I'd expect the CPU address to work fine. But here the performance is even less likely to be any good. > // Try the arch specific helper > const struct dma_map_ops *comp_ops = get_dma_ops(completer); > const struct dma_map_ops *init_ops = get_dma_ops(initiator); So, in this case, what device does the completer point to? The PCI device or a more specific GPU device? If it's the former, who's responsible for setting the new dma_ops? Typically the dma_ops are arch specific but now you'd be adding ones that are tied to hmm or the gpu. >> I'm not sure I like the name pci_p2p_same_segment. It reads as though >> it's only checking if the devices are not the same segment. > > Well, that is exactly what it is doing. If it succeeds then the caller > knows the DMA will not flow outside the segment and no iommu setup/etc > is required. It appears to me like it's calculating the DMA address, and the check is just a side requirement. It reads as though it's only doing the check. Logan
[toc] | [prev] | [next] | [standalone]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2017-04-19 21:40 +0200 |
| Message-ID | <ty3Ci-7ov-5@gated-at.bofh.it> |
| In reply to | #1626716 |
On Wed, Apr 19, 2017 at 01:02:49PM -0600, Logan Gunthorpe wrote: > > > On 19/04/17 12:32 PM, Jason Gunthorpe wrote: > > On Wed, Apr 19, 2017 at 12:01:39PM -0600, Logan Gunthorpe wrote: > > Not entirely, it would have to call through the whole process > > including the arch_p2p_cross_segment().. > > Hmm, yes. Though it's still not clear what, if anything, > arch_p2p_cross_segment would be doing. Sets up the iommu for arches that place a iommu between the pci root port and other pci root ports. > In my experience, if you are going between host bridges, the CPU > address (or PCI address -- I'm not sure which seeing they are the > same on my system) would still work fine Try it with VT-D turned on. It shouldn't work or there is a notable security hole in your platform.. > > const struct dma_map_ops *comp_ops = get_dma_ops(completer); > > const struct dma_map_ops *init_ops = get_dma_ops(initiator); > > So, in this case, what device does the completer point to? The PCI > device or a more specific GPU device? If it's the former, who's > responsible for setting the new dma_ops? Typically the dma_ops are arch > specific but now you'd be adding ones that are tied to hmm or the gpu. Donno, that is for GPU folks to figure out :) But.. it could point to a GPU and the GPU struct device could have a proxy dma_ops like Dan pointed out. > >> I'm not sure I like the name pci_p2p_same_segment. It reads as though > >> it's only checking if the devices are not the same segment. > > > > Well, that is exactly what it is doing. If it succeeds then the caller > > knows the DMA will not flow outside the segment and no iommu setup/etc > > is required. > > It appears to me like it's calculating the DMA address, and the check is > just a side requirement. It reads as though it's only doing the check. pci_p2p_same_segment_get_pa() then? Jason
[toc] | [prev] | [next] | [standalone]
| From | Logan Gunthorpe <logang@deltatee.com> |
|---|---|
| Date | 2017-04-19 21:50 +0200 |
| Message-ID | <ty3LY-7rV-25@gated-at.bofh.it> |
| In reply to | #1626727 |
On 19/04/17 01:31 PM, Jason Gunthorpe wrote: > Try it with VT-D turned on. It shouldn't work or there is a notable > security hole in your platform.. Ah, ok. >>> const struct dma_map_ops *comp_ops = get_dma_ops(completer); >>> const struct dma_map_ops *init_ops = get_dma_ops(initiator); >> >> So, in this case, what device does the completer point to? The PCI >> device or a more specific GPU device? If it's the former, who's >> responsible for setting the new dma_ops? Typically the dma_ops are arch >> specific but now you'd be adding ones that are tied to hmm or the gpu. > > Donno, that is for GPU folks to figure out :) > > But.. it could point to a GPU and the GPU struct device could have a > proxy dma_ops like Dan pointed out. Seems a bit awkward to me that in order for the intended use case, you have to proxy the dma_ops. I'd probably still suggest throwing a couple ops for things like this in the dev_pagemap. >> It appears to me like it's calculating the DMA address, and the check is >> just a side requirement. It reads as though it's only doing the check. > > pci_p2p_same_segment_get_pa() then? Ok, I think that's a bit clearer. Logan
[toc] | [prev] | [next] | [standalone]
| From | Jason Gunthorpe <jgunthorpe@obsidianresearch.com> |
|---|---|
| Date | 2017-04-19 22:50 +0200 |
| Message-ID | <ty4I1-80K-3@gated-at.bofh.it> |
| In reply to | #1626743 |
On Wed, Apr 19, 2017 at 01:41:49PM -0600, Logan Gunthorpe wrote: > > But.. it could point to a GPU and the GPU struct device could have a > > proxy dma_ops like Dan pointed out. > > Seems a bit awkward to me that in order for the intended use case, you > have to proxy the dma_ops. I'd probably still suggest throwing a couple > ops for things like this in the dev_pagemap. Another option is adding a new 'struct completer_dma_ops *' to struct device for this use case. Seems like a waste to expand dev_pagemap when we only need a unique value per struct device? Jason
[toc] | [prev] | [next] | [standalone]
| From | Logan Gunthorpe <logang@deltatee.com> |
|---|---|
| Date | 2017-04-20 01:00 +0200 |
| Message-ID | <ty6JP-O3-11@gated-at.bofh.it> |
| In reply to | #1626786 |
On 19/04/17 02:48 PM, Jason Gunthorpe wrote: > On Wed, Apr 19, 2017 at 01:41:49PM -0600, Logan Gunthorpe wrote: > >>> But.. it could point to a GPU and the GPU struct device could have a >>> proxy dma_ops like Dan pointed out. >> >> Seems a bit awkward to me that in order for the intended use case, you >> have to proxy the dma_ops. I'd probably still suggest throwing a couple >> ops for things like this in the dev_pagemap. > > Another option is adding a new 'struct completer_dma_ops *' to struct > device for this use case. > > Seems like a waste to expand dev_pagemap when we only need a unique > value per struct device? I feel like expanding dev_pagemap has a much lower impact than expanding struct device... dev_pagemap is only one instance per zone device region so expanding it shouldn't be a huge issue. Expanding struct device means every device struct in the system gets bigger. Logan
[toc] | [prev] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@gmail.com> |
|---|---|
| Date | 2017-04-20 02:10 +0200 |
| Message-ID | <ty7Pz-1FO-9@gated-at.bofh.it> |
| In reply to | #1626857 |
On Wed, Apr 19, 2017 at 3:55 PM, Logan Gunthorpe <logang@deltatee.com> wrote: > > > On 19/04/17 02:48 PM, Jason Gunthorpe wrote: >> On Wed, Apr 19, 2017 at 01:41:49PM -0600, Logan Gunthorpe wrote: >> >>>> But.. it could point to a GPU and the GPU struct device could have a >>>> proxy dma_ops like Dan pointed out. >>> >>> Seems a bit awkward to me that in order for the intended use case, you >>> have to proxy the dma_ops. I'd probably still suggest throwing a couple >>> ops for things like this in the dev_pagemap. >> >> Another option is adding a new 'struct completer_dma_ops *' to struct >> device for this use case. >> >> Seems like a waste to expand dev_pagemap when we only need a unique >> value per struct device? > > I feel like expanding dev_pagemap has a much lower impact than expanding > struct device... dev_pagemap is only one instance per zone device region > so expanding it shouldn't be a huge issue. Expanding struct device means > every device struct in the system gets bigger. Especially since we expect a very small subset of devices will ever support p2p.
[toc] | [prev] | [standalone]
Page 4 of 4 — ← Prev page 1 2 3 [4]
Back to top | Article view | linux.kernel
csiph-web