Re: [RFC PATCH 04/15] lib/scatterlist: Add flag for indicating P2PDMA segments in an SGL
On 2020-12-09 9:04 p.m., Dan Williams wrote: > On Wed, Dec 9, 2020 at 6:07 PM Logan Gunthorpe wrote: >> >> >> >> On 2020-12-09 6:22 p.m., Dan Williams wrote: >>> On Mon, Nov 9, 2020 at 8:47 AM Logan Gunthorpe wrote: On 2020-11-09 2:12 a.m., Christoph Hellwig wrote: > On Fri, Nov 06, 2020 at 10:00:25AM -0700, Logan Gunthorpe wrote: >> We make use of the top bit of the dma_length to indicate a P2PDMA >> segment. > > I don't think "we" can. There is nothing limiting the size of a SGL > segment. Yes, I expected this would be the unacceptable part. Any alternative ideas? >>> >>> Why is the SG_P2PDMA_FLAG needed as compared to checking the SGL >>> segment-pages for is_pci_p2pdma_page()? >> >> Because the DMA and page segments in the SGL aren't necessarily aligned... >> >> The IOMMU implementations can coalesce multiple pages into fewer DMA >> address ranges, so the page pointed to by sg->page_link may not be the >> one that corresponds to the address in sg->dma_address for a given segment. >> >> If that makes sense -- it's not the easiest thing to explain. > > It does... > > Did someone already grab, or did you already consider the 3rd > available bit in page_link? AFAICS only SG_CHAIN and SG_END are > reserved. However, if you have a CONFIG_64BIT dependency for > user-directed p2pdma that would seem to allow SG_P2PDMA_FLAG to be > (0x4) in page_link. Hmm, I half considered that, but I had came to the conclusion that given the mis-alignment I shouldn't be using the page side of the SGL. However, reconsidering now, that might actually be a reasonable option. However, the CONFIG_64BIT dependency would have to be on all P2PDMA, because we'd need to replace pci_p2pdma_map_sg() in all cases. I'm not sure if this would be a restriction people care about. Thanks, Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 04/15] lib/scatterlist: Add flag for indicating P2PDMA segments in an SGL
On Wed, Dec 9, 2020 at 6:07 PM Logan Gunthorpe wrote: > > > > On 2020-12-09 6:22 p.m., Dan Williams wrote: > > On Mon, Nov 9, 2020 at 8:47 AM Logan Gunthorpe wrote: > >> > >> > >> > >> On 2020-11-09 2:12 a.m., Christoph Hellwig wrote: > >>> On Fri, Nov 06, 2020 at 10:00:25AM -0700, Logan Gunthorpe wrote: > We make use of the top bit of the dma_length to indicate a P2PDMA > segment. > >>> > >>> I don't think "we" can. There is nothing limiting the size of a SGL > >>> segment. > >> > >> Yes, I expected this would be the unacceptable part. Any alternative ideas? > > > > Why is the SG_P2PDMA_FLAG needed as compared to checking the SGL > > segment-pages for is_pci_p2pdma_page()? > > Because the DMA and page segments in the SGL aren't necessarily aligned... > > The IOMMU implementations can coalesce multiple pages into fewer DMA > address ranges, so the page pointed to by sg->page_link may not be the > one that corresponds to the address in sg->dma_address for a given segment. > > If that makes sense -- it's not the easiest thing to explain. It does... Did someone already grab, or did you already consider the 3rd available bit in page_link? AFAICS only SG_CHAIN and SG_END are reserved. However, if you have a CONFIG_64BIT dependency for user-directed p2pdma that would seem to allow SG_P2PDMA_FLAG to be (0x4) in page_link. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 04/15] lib/scatterlist: Add flag for indicating P2PDMA segments in an SGL
On 2020-12-09 6:22 p.m., Dan Williams wrote: > On Mon, Nov 9, 2020 at 8:47 AM Logan Gunthorpe wrote: >> >> >> >> On 2020-11-09 2:12 a.m., Christoph Hellwig wrote: >>> On Fri, Nov 06, 2020 at 10:00:25AM -0700, Logan Gunthorpe wrote: We make use of the top bit of the dma_length to indicate a P2PDMA segment. >>> >>> I don't think "we" can. There is nothing limiting the size of a SGL >>> segment. >> >> Yes, I expected this would be the unacceptable part. Any alternative ideas? > > Why is the SG_P2PDMA_FLAG needed as compared to checking the SGL > segment-pages for is_pci_p2pdma_page()? Because the DMA and page segments in the SGL aren't necessarily aligned... The IOMMU implementations can coalesce multiple pages into fewer DMA address ranges, so the page pointed to by sg->page_link may not be the one that corresponds to the address in sg->dma_address for a given segment. If that makes sense -- it's not the easiest thing to explain. Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 04/15] lib/scatterlist: Add flag for indicating P2PDMA segments in an SGL
On Mon, Nov 9, 2020 at 8:47 AM Logan Gunthorpe wrote: > > > > On 2020-11-09 2:12 a.m., Christoph Hellwig wrote: > > On Fri, Nov 06, 2020 at 10:00:25AM -0700, Logan Gunthorpe wrote: > >> We make use of the top bit of the dma_length to indicate a P2PDMA > >> segment. > > > > I don't think "we" can. There is nothing limiting the size of a SGL > > segment. > > Yes, I expected this would be the unacceptable part. Any alternative ideas? Why is the SG_P2PDMA_FLAG needed as compared to checking the SGL segment-pages for is_pci_p2pdma_page()? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 04/15] lib/scatterlist: Add flag for indicating P2PDMA segments in an SGL
On 2020-11-09 2:12 a.m., Christoph Hellwig wrote: > On Fri, Nov 06, 2020 at 10:00:25AM -0700, Logan Gunthorpe wrote: >> We make use of the top bit of the dma_length to indicate a P2PDMA >> segment. > > I don't think "we" can. There is nothing limiting the size of a SGL > segment. Yes, I expected this would be the unacceptable part. Any alternative ideas? Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 04/15] lib/scatterlist: Add flag for indicating P2PDMA segments in an SGL
On 2020-11-09 09:12, Christoph Hellwig wrote:
On Fri, Nov 06, 2020 at 10:00:25AM -0700, Logan Gunthorpe wrote:
We make use of the top bit of the dma_length to indicate a P2PDMA
segment.
I don't think "we" can. There is nothing limiting the size of a SGL
segment.
Right, the story behind ab2cbeb0ed30 ("iommu/dma: Handle SG length
overflow better") comes immediately to mind, for one. If all the P2P
users can agree to be in on the game then by all means implement this in
the P2P code, but I don't think it belongs in the generic top-level
scatterlist API.
Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 04/15] lib/scatterlist: Add flag for indicating P2PDMA segments in an SGL
On Fri, Nov 06, 2020 at 10:00:25AM -0700, Logan Gunthorpe wrote: > We make use of the top bit of the dma_length to indicate a P2PDMA > segment. I don't think "we" can. There is nothing limiting the size of a SGL segment. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[RFC PATCH 04/15] lib/scatterlist: Add flag for indicating P2PDMA segments in an SGL
We make use of the top bit of the dma_length to indicate a P2PDMA
segment. Code that uses this will need to use sg_dma_p2pdma_len() to
get the length and ensure no lengths exceed 2GB.
An sg_dma_is_p2pdma() helper is included to check if a particular
segment is p2pdma().
Signed-off-by: Logan Gunthorpe
---
include/linux/scatterlist.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 36c47e7e66a2..e738159d56f9 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -39,6 +39,10 @@ struct scatterlist {
#define sg_dma_len(sg) ((sg)->length)
#endif
+#define SG_P2PDMA_FLAG (1U << 31)
+#define sg_dma_p2pdma_len(sg) (sg_dma_len(sg) & ~SG_P2PDMA_FLAG)
+#define sg_dma_is_p2pdma(sg) (sg_dma_len(sg) & SG_P2PDMA_FLAG)
+
struct sg_table {
struct scatterlist *sgl;/* the list */
unsigned int nents; /* number of mapped entries */
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
