linux-kernel - Re: pci_map_sg() does not coalesce adjacent physical memory? x86

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 19 Nov 2008 15:58:12 +0900
From:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
To:	leon.woestenberg@...il.com
Cc:	fujita.tomonori@....ntt.co.jp, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org,
	James.Bottomley@...senpartnership.com, tj@...nel.org
Subject: Re: pci_map_sg() does not coalesce adjacent physical memory? x86

On Wed, 19 Nov 2008 07:22:31 +0100
"Leon Woestenberg" <leon.woestenberg@...il.com> wrote:

> Hello,
> 
> On Wed, Nov 19, 2008 at 6:19 AM, FUJITA Tomonori
> <fujita.tomonori@....ntt.co.jp> wrote:
> > On Mon, 17 Nov 2008 19:15:32 -0800
> >> > pci_map_sg() does not coalesce the scattergather list for me on x86.
> >> [ Marked as Bogus Report ]
> >>
> >> > Is this expected? Documentation mentions that coalescing is typically
> >> > done by pci_map_sg().
> >
> > Hm, what document did you read? We might need to fix it.
> >
> http://lxr.linux.no/linux+v2.6.27.6/Documentation/DMA-API.txt
> 
>  316        pci_map_sg(struct pci_dev *hwdev, struct scatterlist *sg,
>  317                int nents, int direction)
>  318
>  319Maps a scatter gather list from the block layer.
>  320
>  321Returns: the number of physical segments mapped (this may be shorter
>  322than <nents> passed in if the block layer determines that some
>  323elements of the scatter/gather list are physically adjacent and thus
>  324may be mapped with a single entry).

Hmm, the description looks confusing. The block layer coalesces
physically adjacent entries before pci_map_sg(). pci_map_sg() could
coalesces physically or virtually adjacent entries.


> Also it is mentioned elsewhere, such a Linux Device Driver 3rd
> Edition, page 451.
> 
> > pci_map_sg() is not a typical place to coalesce the entries of the sg
> > list are physically adjacent. The block layer is the typical place.
> >
> Hmmm, why not?
> 
> In my case I am using the following flow:
> 
> virtual user buffer -> scatterlist -> pci_map_sg -> device specific
> scatterlist table

If you follow what the block does now, you build coalesced
scatterlists from virtual user buffers.


> Of course, I could write my own (tm) and map my pages into PCI using
> pci_map_single, but I thought I was being the good kid by using what
> was there.

I don't think it's not a good place. What pci_map_single should do is
mapping a virtual address to a dma-capable address.

IOMMU dma operations maps a virtual address to an I/O address and
non-IOMMU dma operations do nothing. As you said below, some IOMMU
operations do coalescing but it's virtual mapping (probably their code
can handle physical coalescing too though but the block layer does
physical coalescing before that though). And the virtual mapping is
optional. Not all the IOMMU operations do. As I said, x86 IOMMUs
(VT-d, AMD, and Calgary) except for GART does not do coalescing. Any
non-IOMMU dma operations don't do something complicated.

If you add physical coalescing code into pci_map_sg, block-layer
drivers go though physical coalescing code twice, the block layer and
pci_map_sg meaninglessly. It's not the right approach.


> > The dma operations are free to coalesce the entries that physically
> > and virtually adjacent but there are not many that does.
> >
> > For example, by default, on x86, only AMD GART (x86_64) dma operation
> > coalesces such entries.
> >
> 
> > What's kinda of your driver? If it's a SCSI (or block) driver, you
> > don't need this trick.
> >
> Not a block driver.
> 
> A data acquisition driver through PCI Express.

Hmm, what location will your driver live under drivers/?


> > I don't think that inventing a homegrown function to coalesce sg
> > entries in a driver is a good idea. If you really need this, it's
> > would be better to have a generic function to coalesce sg entries and
> > modify the block layer to use it (and your driver can use it too).
> >
> +1 for this idea, *iff* it does not belong in pci_map_sg.
> 
> However, I think it *does* belong in pci_map_sg, because pci_map_sg
> can perform virtual coalescing exploiting IOMMU's if I am not
> mistaken.

As I wrote above, it's not the right approach.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/