[parisc-linux] tulip DMA mapping

[parisc-linux] tulip DMA mapping

[Grant Grundler]

Hi all,
I see a "bug" in tulip's usage of mapping services.
It's not the bug I was looking for unfortunately.

In line 217 of drivers/net/tulip/interrupt.c:

	if (tp->tx_buffers[entry].mapping)
		pci_unmap_single(tp->pdev,
				tp->tx_buffers[entry].mapping,
				sizeof(tp->setup_frame),
				PCI_DMA_TODEVICE);

0 is a valid pci_map_single() return value when the system has an IO MMU.

The system will panic before pci_map_single() will fail.
The driver needs to remember some other way if a buffer was mapped or not.
Or the Documentation/DMA-mapping.txt should be changed - ie add this
to the interface definition and I can reserve the 1st mapping
entry so no-one uses it.

Should I be mailing Jeff Garzik <jgarzik@mandrakesoft.com> directly?
Or can someone who knows Jeff point this out to him?

thanks,
grant

Re: [parisc-linux] tulip DMA mapping

[David S. Miller]

   Date: Fri, 10 Nov 2000 10:18:08 +0000
   From: Matthew Wilcox <matthew@wil.cx>

   > Should I be mailing Jeff Garzik <jgarzik@mandrakesoft.com> directly?
   > Or can someone who knows Jeff point this out to him?

   i've cc'd jeff & dave miller on this.

In 2.4.x there is _NO_ error return from the PCI dma functions except
the consistent DMA mapping ones.

This was an explicit design decision, the dynamic mapping functions
should never fail, and if they do it is a hard error.

Therefore no drivers need to check for failure, as far as they are
concerned, there is no failure.

So what is the issue?  In 2.5.x I'll add an error return facility
(BTW: -1 ie. 0xfffffff would probably work as an error value on all
 platforms :-)

Later,
David S. Miller
davem@redhat.com

Re: [parisc-linux] tulip DMA mapping

[Matthew Wilcox]

On Thu, Nov 09, 2000 at 12:12:25PM -0800, Grant Grundler wrote:
> 0 is a valid pci_map_single() return value when the system has an IO MMU.

Oh dear.  You can bet tulip won't be the only driver which assumes it
isn't a valid return value.  Can't our IOMMU code be limited in such a
way that 0 is not a valid return value?  Say, constrain all allocated
addresses to the top half of the device bus?

(um, just check me on this, map_single returns a device bus address,
not a processor bus address, right?)

> The system will panic before pci_map_single() will fail.
> The driver needs to remember some other way if a buffer was mapped or not.
> Or the Documentation/DMA-mapping.txt should be changed - ie add this
> to the interface definition and I can reserve the 1st mapping
> entry so no-one uses it.

we should probably have a BAD_DMA_ADDR define which that array should be
initialised to.  it's a little late in 2.4 to go through and audit all
the drivers again.

> Should I be mailing Jeff Garzik <jgarzik@mandrakesoft.com> directly?
> Or can someone who knows Jeff point this out to him?

i've cc'd jeff & dave miller on this.

-- 
Revolutions do not require corporate support.

Re: [parisc-linux] tulip DMA mapping

[Richard Hirst]

I've quoted the whole of Grants message below, so you can see the
context.  It looks like tulip is treating zero as meaning it
doesn't have anything to pci_unmap...

Grant Grundler wrote:
> Hi all,
> I see a "bug" in tulip's usage of mapping services.
> It's not the bug I was looking for unfortunately.
> 
> In line 217 of drivers/net/tulip/interrupt.c:
> 
>         if (tp->tx_buffers[entry].mapping)
>                 pci_unmap_single(tp->pdev,
>                                 tp->tx_buffers[entry].mapping,
>                                 sizeof(tp->setup_frame),
>                                 PCI_DMA_TODEVICE);
> 
> 0 is a valid pci_map_single() return value when the system has an IO MMU.
> 
> The system will panic before pci_map_single() will fail.
> The driver needs to remember some other way if a buffer was mapped or not.
> Or the Documentation/DMA-mapping.txt should be changed - ie add this
> to the interface definition and I can reserve the 1st mapping
> entry so no-one uses it.

Richard


On Fri, Nov 10, 2000 at 02:16:11AM -0800, David S. Miller wrote:
>    Date: Fri, 10 Nov 2000 10:18:08 +0000
>    From: Matthew Wilcox <matthew@wil.cx>
> 
>    > Should I be mailing Jeff Garzik <jgarzik@mandrakesoft.com> directly?
>    > Or can someone who knows Jeff point this out to him?
> 
>    i've cc'd jeff & dave miller on this.
> 
> In 2.4.x there is _NO_ error return from the PCI dma functions except
> the consistent DMA mapping ones.
> 
> This was an explicit design decision, the dynamic mapping functions
> should never fail, and if they do it is a hard error.
> 
> Therefore no drivers need to check for failure, as far as they are
> concerned, there is no failure.
> 
> So what is the issue?  In 2.5.x I'll add an error return facility
> (BTW: -1 ie. 0xfffffff would probably work as an error value on all
>  platforms :-)
> 
> Later,
> David S. Miller
> davem@redhat.com
> 
> ---------------------------------------------------------------------------
> To unsubscribe: send e-mail to parisc-linux-request@thepuffingroup.com with
> `unsubscribe' as the subject.
> 
> 

Re: [parisc-linux] tulip DMA mapping

[David S. Miller]

   Date: Fri, 10 Nov 2000 11:12:20 +0000
   From: Richard Hirst <rhirst@linuxcare.com>

   I've quoted the whole of Grants message below, so you can see the
   context.  It looks like tulip is treating zero as meaning it
   doesn't have anything to pci_unmap...

Thank you for the clarification.

Jeff, this is in fact a bug, please fix :-)

Later,
David S. Miller
davem@redhat.com

Re: [parisc-linux] tulip DMA mapping

[Jeff Garzik]

"David S. Miller" wrote:
> 
>    Date: Fri, 10 Nov 2000 11:12:20 +0000
>    From: Richard Hirst <rhirst@linuxcare.com>
> 
>    I've quoted the whole of Grants message below, so you can see the
>    context.  It looks like tulip is treating zero as meaning it
>    doesn't have anything to pci_unmap...
> 
> Thank you for the clarification.
> 
> Jeff, this is in fact a bug, please fix :-)

np.  Like Matthew(?) hinted, this is not the only place that needs
fixing.  I'll take care of it.

	Jeff


-- 
Jeff Garzik             |
Building 1024           | Would you like a Twinkie?
MandrakeSoft            |

Re: [parisc-linux] tulip DMA mapping

[Grant Grundler]

Mathew,
I think the situation is clarified. This is just FYI.

Matthew Wilcox wrote:
> On Thu, Nov 09, 2000 at 12:12:25PM -0800, Grant Grundler wrote:
> > 0 is a valid pci_map_single() return value when the system has an IO MMU.
> 
> Oh dear.  You can bet tulip won't be the only driver which assumes it
> isn't a valid return value.

Well, then:
1) the driver writer made a wrong assumption that the "spec"
   does not support.
2) that wasn't the case here - 0 was used as a flag to indicate
   a mapping had (or hadn't rather) been done - not that the
   mapping call had failed.

> Can't our IOMMU code be limited in such a
> way that 0 is not a valid return value?  Say, constrain all allocated
> addresses to the top half of the device bus?

It could pretty easily by reserving the dma_map[0] entry during
init time. Dave Miller already made it clear that's not desirable.

> (um, just check me on this, map_single returns a device bus address,
> not a processor bus address, right?)

Yes. It's the address a device must use to master DMA transactions.
pci_map_single() input parameters are "struct pci_dev *", virtual host
memory address, and buffer size. The return is a device specific I/O
DMA address - ie this mapping cannot be shared with other devices.

FWIW, "I/O DMA address" are called IOVAs in HPUX (I/O Virtual Address).
The HW guys prefer another name but this one stuck.

grant

Grant Grundler
Unix Systems Enablement Lab
+1.408.447.7253

Re: [parisc-linux] tulip DMA mapping

[Jeff Garzik]

"David S. Miller" wrote:
> 
>    Date: Fri, 10 Nov 2000 11:12:20 +0000
>    From: Richard Hirst <rhirst@linuxcare.com>
> 
>    I've quoted the whole of Grants message below, so you can see the
>    context.  It looks like tulip is treating zero as meaning it
>    doesn't have anything to pci_unmap...
> 
> Thank you for the clarification.
> 
> Jeff, this is in fact a bug, please fix :-)

Fixed in the latest driver (which Alan has).

Some other drivers need fixing for this, too...

-- 
Jeff Garzik       | "You see, in this world there's two kinds of
Building 1024     |  people, my friend: Those with loaded guns
MandrakeSoft      |  and those who dig. You dig."  --Blondie