pci_alloc_consistent usage

pci_alloc_consistent usage

[Wayne Gowcher]

I am working on a driver that uses PCI DMA transfers
from system memory to the PCI device. On the 2.4.18
kernel it worked OK, but now when it is recompiled for
2.4.20 it doesn't. Digging into things I have found
that the function virt_to_phys() has been changed from
:
return PHYSADDR(address)

to
return (unsigned long)address - PAGE_OFFSET

Where PAGE_OFFSET is 0x8000 0000, and where PHYSADDR
would AND the address against 0x1FFF FFFF. As far as I
can tell the problem comes from pci_alloc_consistent
doing :

ret = UNCAC_ADDR(ret) 

which converts a 0x8xxx address to 0xAxxx, and then
when you pass this 0xAxxx_xxxx address through
virt_to_phys() you get an address of the form
0x2xxx_xxxx. This 0x2xxx_xxxx is passed to the dma
controller as the physical address to where it must
read / write data, and because it is 0x2xxx_xxxx and
not 0x0xxx_xxxx an exception occurs.

At first I just tried AND'ing out the 0xA.. like
PHYSADDR used to do it, but with that change i no
longer get the exception, but the driver does not dma
the data across - it just sits there.

I read DMA-mapping.txt and it says virt_to_phys() will
be phased out, and should be used, but doesn't
elaborate any further (like how you should do it now
).

So after that long intro, my question is :

Anybody know where I'm going wrong and how to fix
things ?

Also any tips on what drivers to look at for good
examples would also be appreciated.

TIA


__________________________________
Do you Yahoo!?
The New Yahoo! Search - Faster. Easier. Bingo.
http://search.yahoo.com

Re: pci_alloc_consistent usage

[Ralf Baechle]

On Fri, May 23, 2003 at 02:59:35PM -0700, Wayne Gowcher wrote:

> Where PAGE_OFFSET is 0x8000 0000, and where PHYSADDR
> would AND the address against 0x1FFF FFFF. As far as I
> can tell the problem comes from pci_alloc_consistent
> doing :
> 
> ret = UNCAC_ADDR(ret) 
> 
> which converts a 0x8xxx address to 0xAxxx, and then
> when you pass this 0xAxxx_xxxx address through
> virt_to_phys() you get an address of the form
> 0x2xxx_xxxx. This 0x2xxx_xxxx is passed to the dma
> controller as the physical address to where it must
> read / write data, and because it is 0x2xxx_xxxx and
> not 0x0xxx_xxxx an exception occurs.

The change was partly done to catch broken code, partly because a
subtraction is potencially faster on some processors.

> At first I just tried AND'ing out the 0xA.. like
> PHYSADDR used to do it, but with that change i no
> longer get the exception, but the driver does not dma
> the data across - it just sits there.
> 
> I read DMA-mapping.txt and it says virt_to_phys() will
> be phased out, and should be used, but doesn't
> elaborate any further (like how you should do it now).

Use the value returned by pci_alloc_consistent in *dma_handle instead
of trying to do any conversions with of pci_alloc_consistent's return
value.

  Ralf

Re: pci_alloc_consistent usage

[Atsushi Nemoto]

>>>>> On Tue, 27 May 2003 11:17:40 +0200, Ralf Baechle <ralf@linux-mips.org> said:
ralf> Use the value returned by pci_alloc_consistent in *dma_handle
ralf> instead of trying to do any conversions with of
ralf> pci_alloc_consistent's return value.

How about virt_to_page()?

Currently, many sound drivers (including ALSA) pass a
pci_alloc_consistent's return value to virt_to_page.  While
virt_to_page is defined as below, we can not pass KSEG1 address
(returned by pci_alloc_consistent) to virt_to_page.

#define __pa(x)		((unsigned long) (x) - PAGE_OFFSET)
#define virt_to_page(kaddr)	(mem_map + (__pa(kaddr) >> PAGE_SHIFT))

Following definition can handle this case properly,

#define virt_to_page(kaddr)	(mem_map + (PHYSADDR(kaddr) >> PAGE_SHIFT))

But as ralf said, PHYSADDR() may be is a bit slower then __pa().

Is it worth to change?  Or should we fix the driver using virt_to_page
with pci_alloc_consistent?

---
Atsushi Nemoto

Re: pci_alloc_consistent usage

[Christoph Hellwig]

On Tue, May 27, 2003 at 07:07:49PM +0900, Atsushi Nemoto wrote:
> >>>>> On Tue, 27 May 2003 11:17:40 +0200, Ralf Baechle <ralf@linux-mips.org> said:
> ralf> Use the value returned by pci_alloc_consistent in *dma_handle
> ralf> instead of trying to do any conversions with of
> ralf> pci_alloc_consistent's return value.
> 
> How about virt_to_page()?
> 
> Currently, many sound drivers (including ALSA) pass a
> pci_alloc_consistent's return value to virt_to_page.

You are not allow to do so.  Any driver doing this is broken.

Re: pci_alloc_consistent usage

[Ralf Baechle]

On Tue, May 27, 2003 at 11:53:22AM +0100, Christoph Hellwig wrote:

> > ralf> Use the value returned by pci_alloc_consistent in *dma_handle
> > ralf> instead of trying to do any conversions with of
> > ralf> pci_alloc_consistent's return value.
> > 
> > How about virt_to_page()?
> > 
> > Currently, many sound drivers (including ALSA) pass a
> > pci_alloc_consistent's return value to virt_to_page.
> 
> You are not allow to do so.  Any driver doing this is broken.

That however is exactly what's documented in DMA-mapping.txt:

[...]
portably refer to any piece of memory.  If you have a cpu pointer
(which may be validly DMA'd too) you may easily obtain the page
and offset using something like this:
                                                                                
        struct page *page = virt_to_page(ptr);
        unsigned long offset = ((unsigned long)ptr & ~PAGE_MASK);
[...]

While it's officially documented I still don't like it.  A double
conversion such as page_address(virt_to_page(ptr)) would accidently
turn a pointer of an uncached mapping into one to a cached area for the
same object - that will almost certainly not work as expected on a
non-coherent machine.

  Ralf

Re: pci_alloc_consistent usage

[Christoph Hellwig]

On Tue, May 27, 2003 at 01:22:37PM +0200, Ralf Baechle wrote:
> [...]
> portably refer to any piece of memory.  If you have a cpu pointer
> (which may be validly DMA'd too) you may easily obtain the page
> and offset using something like this:
>                                                                                 
>         struct page *page = virt_to_page(ptr);
>         unsigned long offset = ((unsigned long)ptr & ~PAGE_MASK);
> [...]
> 
> While it's officially documented I still don't like it.

Hmm, I remembered that some ports used vmalloc-like allocators for
this and virt_to_page doesn't work for those..

Re: pci_alloc_consistent usage

[Ralf Baechle]

On Tue, May 27, 2003 at 12:33:29PM +0100, Christoph Hellwig wrote:

> On Tue, May 27, 2003 at 01:22:37PM +0200, Ralf Baechle wrote:
> > [...]
> > portably refer to any piece of memory.  If you have a cpu pointer
> > (which may be validly DMA'd too) you may easily obtain the page
> > and offset using something like this:
> >                                                                                 
> >         struct page *page = virt_to_page(ptr);
> >         unsigned long offset = ((unsigned long)ptr & ~PAGE_MASK);
> > [...]
> > 
> > While it's officially documented I still don't like it.
> 
> Hmm, I remembered that some ports used vmalloc-like allocators for
> this and virt_to_page doesn't work for those..

There's at least one MIPS system which we don't support anymore but which
would have to be supported by something like this.

Whatever - virt_to_page should then be considered a a legacy API which we
have to try to support as well as possible in the hope it's going to fade
away ...

  Ralf

Re: pci_alloc_consistent usage

[Atsushi Nemoto]

>>>>> On Tue, 27 May 2003 13:22:37 +0200, Ralf Baechle <ralf@linux-mips.org> said:
ralf> While it's officially documented I still don't like it.  A
ralf> double conversion such as page_address(virt_to_page(ptr)) would
ralf> accidently turn a pointer of an uncached mapping into one to a
ralf> cached area for the same object - that will almost certainly not
ralf> work as expected on a non-coherent machine.

Yes, it's dangerous, I agree.

ralf> Whatever - virt_to_page should then be considered a a legacy API
ralf> which we have to try to support as well as possible in the hope
ralf> it's going to fade away ...

I hope so.  For now we can do something like:

     buf = pci_alloc_consistent(dev, size, &dmabuf);
#if defined(__mips__) && defined(CONFIG_NONCOHERENT_IO)
     page = virt_to_page(CAC_ADDR(buf));
#else
     page = virt_to_page(buf);
#endif

Ugly but I do not want to add extra overhead to virt_to_page itself...

---
Atsushi Nemoto

pci_alloc_consistent() usage

[Vadivelan Mani]

hi,

   I'm writing a wireless driver which requires 8 transmit and 8 receive 
buffers each of size 3KB approx.

These buffers should be in dma capable space.

I've allocated them using pci_alloc_consistent().

I've allocated 128KBytes just to make more space. I've got few doubts.

   1. Can pci_alloc_consistent() be used to allocate memory upto 128KBytes?

If this is not possible, how much can it allocate?



2.)   I would also like to know the exact use of this allocated space to 
transmit or receive a packet.

During transmission i do the following. Plz correct me if i'm wrong because 
i'm new to driver writing.

The device has a register which should be loaded with the transmit buffers 
starting address.

I copy the packet coming from the Kernel in the form of sk_buff structure 
into one of the transmit buffers that i've allocated using memcpy().
And i set the register in the device to initiate transmission of the packet.

Where does the dma transfer concept come in this?
There is no mention of the direction of data transfer in 
pci_alloc_consistent().

I also assumed that allocating buffer in dma capable space was itself enough 
to start dma transfers.

Since i do not have the device now i'm not able to test the code. But i 
would like to write the code before i get the device.

Kindly help me in this.

thanking u in advance.

regards,
vadi.

_________________________________________________________________
Marriage by choice.... http://www.shaadi.com/ptnr.php?ptnr=hmltag Log onto 
Shaadi.com

Re: pci_alloc_consistent() usage

[Ralf Baechle]

On Tue, Jul 13, 2004 at 06:31:01PM +0530, Vadivelan Mani wrote:

>   I'm writing a wireless driver which requires 8 transmit and 8 receive 
> buffers each of size 3KB approx.

I hope this device isn't really as simplistic as you make it sound ...

> These buffers should be in dma capable space.
> 
> I've allocated them using pci_alloc_consistent().
> 
> I've allocated 128KBytes just to make more space. I've got few doubts.
> 
>   1. Can pci_alloc_consistent() be used to allocate memory upto 128KBytes?

Yes.  For MIPS MAX_ORDER defaults to 11 so you even do alloc_page (which
is the underlying allocator of pci_alloc_consistent) upto 2^11 page that
is 8MB.  Downside - memory allocation is making such large allocations
unreliable; the more unreliable the larger the allocation.  In general
try to stick to allocations of order 0 that is PAGE_SIZE which atm is
4k on MIPS.  They're ok for permanent allocations such as rx/tx rings
which only happen rarely.

> 2.)   I would also like to know the exact use of this allocated space to 
> transmit or receive a packet.

pci_alloc_consistent() is meant to be used for permanent allocations
such as rx/tx rings.  It's not suitable for allocating skbufs; there are
other mechanisms available for that.

> During transmission i do the following. Plz correct me if i'm wrong because 
> i'm new to driver writing.
> 
> The device has a register which should be loaded with the transmit buffers 
> starting address.
> 
> I copy the packet coming from the Kernel in the form of sk_buff structure 
> into one of the transmit buffers that i've allocated using memcpy().

That's usually an idea only for very small packets.  pci_alloc_consistent
allocates uncached memory on a system withour hardware coherency so this
copy operation would be very slow.  In any case you should experiment to
find the breakeven point.

For larger packets you should use pci_map_single() in the start_xmit()
method of the driver, then pci_unmap_single() later when cleaning that is
typically in an interrupt handler.  Reception of packets would work
fairly similar.

> And i set the register in the device to initiate transmission of the packet.
> 
> Where does the dma transfer concept come in this?
> There is no mention of the direction of data transfer in 
> pci_alloc_consistent().

pci_alloc_consistent will allocate consistent memory that is it's suitable
for transfers in both directions.  On the typical MIPS processor which
doesn't maintain coherency in hardware this means it will return
uncached memory.  Obviously that will work for any direction of transfer.
But: uncached memory is slow - so avoid copying packets there.

> I also assumed that allocating buffer in dma capable space was itself 
> enough to start dma transfers.

No.  DMA capable space means some memory that can be accessed somehow by
a DMA engine.  You still have to tell the device to start the operation.

> Since i do not have the device now i'm not able to test the code. But i 
> would like to write the code before i get the device.

  Ralf