Problems with 2.5.14 PCI reorg and non-PCI architectures

Problems with 2.5.14 PCI reorg and non-PCI architectures

[James Bottomley]

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

Re: Problems with 2.5.14 PCI reorg and non-PCI architectures

[Greg KH]

On Wed, May 08, 2002 at 07:11:10PM -0400, James Bottomley wrote:
> 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.

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 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).

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

Re: Problems with 2.5.14 PCI reorg and non-PCI architectures

[James Bottomley]

greg@kroah.com said:
> 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?

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.

> 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. 

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

Re: Problems with 2.5.14 PCI reorg and non-PCI architectures

[Greg KH]

On Thu, May 09, 2002 at 09:00:28AM -0400, James Bottomley wrote:
> greg@kroah.com said:
> > 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?
> 
> 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.

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

Re: Problems with 2.5.14 PCI reorg and non-PCI architectures

[James Bottomley]

greg@kroah.com said:
> No wait, the NCR driver needs Microchannel, is that true? 

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.

> 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:) 

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.

> 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 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.

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

Thanks!

James

[BK PATCH] PCI reorg fix

[Greg KH]

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



ChangeSet@1.557, 2002-05-09 10:35:57-07:00, greg@kroah.com
  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(-)

Re: [BK PATCH] PCI reorg fix

[Patrick Mochel]

> 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()?

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

Re: [BK PATCH] PCI reorg fix

[Greg KH]

On Thu, May 09, 2002 at 11:06:45AM -0700, Patrick Mochel wrote:
> 
> > 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()?
> 
> 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. 

That seems reasonable.

> 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...

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

greg k-h

Re: [BK PATCH] PCI reorg fix

[James Bottomley]

mochel@osdl.org said:
> Implementing the generic code is ~5 minutes work. However, it will
> break  everything. OTOH, it would be the best motivation for
> modernizing these  drivers... 

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

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

Re: [BK PATCH] PCI reorg fix

[Kai Germaschewski]

On Thu, 9 May 2002, Patrick Mochel wrote:

> 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...

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

Re: [BK PATCH] PCI reorg fix

[Patrick Mochel]

On Thu, 9 May 2002, Kai Germaschewski wrote:

> On Thu, 9 May 2002, Patrick Mochel wrote:
> 
> > 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...
> 
> 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?

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

Re: [BK PATCH] PCI reorg fix

[Kai Germaschewski]

On Thu, 9 May 2002, Patrick Mochel wrote:

> 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. 

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

Re: [BK PATCH] PCI reorg fix

[Martin Dalecki]

Uz.ytkownik Greg KH napisa?:
> 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()?

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...

Re: [BK PATCH] PCI reorg fix

[James Bottomley]

dalecki@evision-ventures.com said:
> 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

Re: [BK PATCH] PCI reorg fix

[Martin Dalecki]

Uz.ytkownik James Bottomley napisa?:
> dalecki@evision-ventures.com said:
> 
>>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.

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.

> 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 
...
> 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.


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 ;-).