Problems with 2.5.14 PCI reorg and non-PCI architectures

Hi All,

You've moved arch/i386/kernel/pci-dma.c under your pci subdirectory. This means that it is now compiled in only when CONFIG_PCI is defined whereas previously it was always compiled.

This file contains all of the DMA memory manipulation functions (like pci_alloc_consistent et al.) which you need for device driver memory mapping even in a non PCI bus machine.

I think the solution is to move it back up to the i386/kernel level and make it always compiled in (perhaps keeping the name as dma.c, though).

James

arch/i386/pci/dma.c now only contains pci_alloc_consistent() and pci_free_consistent(). What kind of configuration are you using that works without CONFIG_PCI and yet calls those functions? Is it a ISA_PNP type configuration? Do you have a .config that this fails on?

I'd be glad to move it back, but I'd like to understand who is using those functions outside of the pci and isa_pnp drivers.

thanks,

greg k-h

The problem is that this is not necessarily PCI related on other platforms.

My cross platform SCSI driver, 53c700.c, uses pci_alloc_consistent because it has to work on parisc archs as well (which do have consistent memory even though a few of them don't have PCI busses). It was failing a test compile. I can manipulate the #ifdefs so that it doesn't use the consistent allocation functions on x86, but I think, in principle, cross platform drivers should be able to use these functions.

Yes, please. If you look at a lot of the non-x86 arch drivers, some of them also use pci_alloc_consistent. I think the only other x86 example I can come up with is aic7xxx_old which also supports the 7770 chip which is used for SCSI in the intel xpress motherboard (pure EISA).

James

But parisc has it's own implementation of pci_alloc_consistent(), so you're ok on that platform. And it looks like only 2 scsi drivers use the 53c700.c code, lasi700.c and NCR_D700.c. The NCR driver looks to need pci, and the lasi700 driver doesn't look like it will even compile on i386.

No wait, the NCR driver needs Microchannel, is that true?

I would like to push back and ask why you are calling a pci_* function from a driver that does not need pci. Yes, I know it's a nice, generic function, but that hasn't stopped people from rewriting that same function a number of times in different forms in different places in the tree :)

In a perfect world, we should probably create a function like:

	void *alloc_consistent (int flags, size_t size, dma_addr_t *dma_handle);

to solve everyone's needs, but I'm not volunteering to do that :)

I'll go move the file and send the changeset to Linus.

thanks,

greg k-h

Correct. the two drivers (lasi700 and NCR_D700) both use 53c700 to drive the chip core, but they take care of interfacing to the local bus, whatever it is.
The chip core (which is bus independent) still has to allocate consistent
memory for the chip mailbox (although I suppose I could alter the bus drivers to pass in a pointer to a pre-allocated region). I can't get away without using pci_sync_single et al. in the bus independent driver, though.

The 53c700 core must be able to use synchronous memory (if it can) on parisc. The only global call for this is pci_alloc_consistent (and its use is advised in Documentation/DMA-mapping.txt). Obviously, x86 is fully synchronous anyway, so it only needs to support the call as a type of nop.

I agree with this. Debate around this naming issue came up in the parisc groups (again because we need consistent allocations and some legacy machines don't have PCI busses). The official response them was use pci_alloc_consistent and don't worry about it seeming to be a pci specific function. Really, it is only legacy machines that are non-pci, so I suppose it does make some sense to have them as special cases of pci specific functions.

Thanks!

James

Linus,

James pointed out that pci_alloc_consistent() and pci_free_consistent() are allowed to be called, even if CONFIG_PCI is not enabled. So this changeset moves these calls back into the arch/i386/kernel directory.

Pull from: bk://linuxusb.bkbits.net/linux-2.5-pci

As a side note, I don't think that any pci_* function should be able to be called by non-pci drivers. Is it worth spending the time now in 2.5 to make these two functions not rely on 'struct pci_dev' and fix up all of the drivers and architectures and documentation to reflect this? Possible names would be alloc_consistent() and free_consistent()?

thanks,

greg k-h

[email protected], 2002-05-09 10:35:57-07:00, [email protected]
moved the pci_alloc_consistent() and pci_free_consistent() functions back into arch/i386/kernel as they are needed even if CONFIG_PCI is not enabled.

 arch/i386/pci/dma.c        |   37 -------------------------------------
 arch/i386/kernel/Makefile  |    2 +-
 arch/i386/kernel/pci-dma.c |   37 +++++++++++++++++++++++++++++++++++++
 arch/i386/pci/Makefile     |    2 +-
 4 files changed, 39 insertions(+), 39 deletions(-)

I would suggest something like:

void *
dev_alloc_consistent(struct device * dev, size_t size, dma_addr_t * dma_handle);

and moving dma_mask to struct device.

To handle differences in arch-specific implementations, we could have a callback that the generic code calls.

Implementing the generic code is ~5 minutes work. However, it will break everything. OTOH, it would be the best motivation for modernizing these drivers...

-pat

Certainly sounds reasonable. I'd guess it's trivial enough to provide wrappers for pci_alloc_consistent(), pci_set_dma_mask() etc., so I don't see why everything would break?

--Kai

Actually, the point _is_ to break everything.

The fact that there are wrappers emulating old PCI behavior is the reason the SCSI drivers don't use the 2.4 interface. If we provide wrappers for alloc_consistent, they'll never change those, either.

At some point, you have to bite the bullet and break the API. It'll be the only way to get drivers to convert. The statement was more of "When we do this, it'll be painful" rather than "If we do it..." :)

-pat

I surely can see your point there. However, new-style and old-style PCI init are conceptually different, and there's no easy way to emulate the old behavior on top of the new one. That is equivalent to saying that converting drivers is in many cases non-trivial. It's painful, and that's why it doesn't happen (or only slowly), unless you force people to do so.

However, the issue here is IMO different. Not having the wrappers only means lots of trivial changes to the drivers, along the lines of

pci_set_dma_mask(pdev,) -> device_set_dma_mask(&pdev->dev,)

which is pointless IMO. Actually, as far as I understood things, the plan with the device tree is not expose it to drivers, but let them still act on pci_dev / usb_dev / whatever, and only base the implementation of struct pci_dev / usb/dev / ... on struct device.

So keeping these pci_* functions makes sense to me, only base the implementation on struct device.

--Kai

That seems reasonable.

Eeek, the scsi drivers? They haven't even started moving to the > 2 years old pci interface yet! :)

greg k-h

It took several years to eliminate the old error handler, too...

If something's already broken, does it really matter how many pieces it's in?

There a siesmic changes coming to the scsi layer anyway, particularly if we want to implement the new tag queuing code. The consistent alloc is virtually a simple conversion recipe, so it shouldn't be too difficult to tack this small change on to a set of much bigger ones.

James

Uz.ytkownik Greg KH napisa?:

If your are at it: I was always itching my had what pci_alloc_inconsistent and pci_free_inconsistent is supposed to be? Negating semantically the consistent attribute shows nicely that the _consistent is a bad nomenclatures. Perhaps something more related to the purpose of it would help. Like

ioalloc() and iofree()

Could be even abstracted from the bus implementation.

And of course much less typing...

I'm all for less typing, but "consistent" memory has a definite meaning to some non-x86 architectures (and inconsistent also has a definite and semantically opposite meaning).

The x86 is nice because it is fully coherent architecture: the CPUs snoop the I/O busses and do CPU cache invalidations for data transferred from an I/O device directly to memory and CPU cache flushes when a device reads directly from memory.

If the CPU doesn't snoop I/O transfers, you have to manually invalidate or flush the necessary CPU cache lines. The pci_sync_... functions are for doing the cache invalidations and flushes correctly. Sometimes you can obtain a region of memory which is "consistent" meaning that you no longer need to do the invalidations/flushes manually because the CPU will operate coherently on this region. "inconsistent" means that you must control the CPU cache yourself. In an incoherent memory architecture, the normal memory allocations give you "inconsistent" regions.

The royal pain on some CPUs is that they only have small consistent regions, so pci_alloc_consistent can fail and you have to know this and fall back to doing all the manual cache operations. For this reason, some cross platform drivers simply choose not to bother with consistent memory at all because the cache manipulation operations are optimised away on fully consistent platforms.

I'd like to say that this is totally unrelated to the bus type, but some architectures place MMUs on their busses which means that memory consistency (and even memory addressability) can indeed be bus specific depending on what the MMU actually does.

James

Uz.ytkownik James Bottomley napisa?:

So what you are requesting is memmory which is suitable to be run for IO. All you are talking about is io. I think ioalloc() and iofree() fit it nice as names.

...

Thank you finally for explaining that the _consistant is about well coherency and caches... This should have been put up in to the documentation in question... becouse beleve me or not - I know well about the "MESIs of the world", but I still wasn't able to make any sense out of this _consistent term in first place.

And I still have the feeling that the nomenclature is bad.
What are you going to do if on some silly VLSI new slow CPU invention at some time the need of doing pciIV_alloc_asynchronous() araises? or pciXI_alloc_remote() or whatever? Are you going to request all the driver writers to adjust to it again!?

(Something like this could for example just happen very urgently if Transmeta decided to reveal native access to the hardware and tools in question I guess ;-).