Hi,
I noticed a lot of drivers are setting the PCI_CACHE_LINE_SIZE
themselves, some to L1_CACHE_BYTES/sizeof(u32), others
to arbitrary values (4, 8, 16).
Then I spotted that we have a routine in the PCI subsystem
(pdev_enable_device) that sets all these to L1_CACHE_BYTES/sizeof(u32)
Further digging revealed that this routine was not getting called.
On my Athlon box, I've noticed some devices are getting their
default values set to 8 (where (I think) it should be 16 ?)
Questions:
1. Is there reason for the drivers to be setting this themselves
to hardcoded values ?
2. Why is pdev_device_enable no longer used ?
regards,
Davej.
--
| Dave Jones <[email protected]> http://www.suse.de/~davej
| SuSE Labs
[email protected] writes:
> 2. Why is pdev_device_enable no longer used ?
It is used from pci_assign_unassigned_resources. iirc, its just that
x86 doesn't call this function.
_____
|_____| ------------------------------------------------- ---+---+-
| | Russell King [email protected] --- ---
| | | | http://www.arm.linux.org.uk/personal/aboutme.html / / |
| +-+-+ --- -+-
/ | THE developer of ARM Linux |+| /|\
/ | | | --- |
+-+-+ ------------------------------------------------- /\\\ |
On Sat, Dec 09, 2000 at 11:38:05AM +0000, Russell King wrote:
> [email protected] writes:
> > 2. Why is pdev_device_enable no longer used ?
Right question would be "why is it not used yet?" ;-)
This routine appeared a while ago in one of test12-pre.
> It is used from pci_assign_unassigned_resources. iirc, its just that
> x86 doesn't call this function.
Yes, only alpha, arm and mips are using that code.
Ivan.
On Sat, 9 Dec 2000, Ivan Kokshaysky wrote:
> > It is used from pci_assign_unassigned_resources. iirc, its just that
> > x86 doesn't call this function.
> Yes, only alpha, arm and mips are using that code.
Ok, thanks Ivan/Russell for clearing this up for me.
If/When x86 (or all?) architectures use this, will it make sense to
remove the PCI space cache line setting from drivers ?
Or is there borked hardware out there that require drivers to say
"This cacheline size must be xxx bytes, anything else will break" ?
regards,
Davej.
--
| Dave Jones <[email protected]> http://www.suse.de/~davej
| SuSE Labs
> If/When x86 (or all?) architectures use this, will it make sense to
> remove the PCI space cache line setting from drivers ?
> Or is there borked hardware out there that require drivers to say
> "This cacheline size must be xxx bytes, anything else will break" ?
If there is surely the driver can override it again before enabling the
master bit or talking to the device ?
On Sat, 9 Dec 2000, Alan Cox wrote:
> > Or is there borked hardware out there that require drivers to say
> > "This cacheline size must be xxx bytes, anything else will break" ?
> If there is surely the driver can override it again before enabling the
> master bit or talking to the device ?
The pdev_enable_device() stuff is marked __init, so done once at boottime.
So yes, a driver could then fix up during initialisation if necessary.
I was curious whether there are any known cases, or can the stuff
in the existing drivers be nuked if/when x86 calls pdev_enable_device().
Davej.
--
| Dave Jones <[email protected]> http://www.suse.de/~davej
| SuSE Labs
On Sat, Dec 09, 2000 at 12:53:46PM +0000, Alan Cox wrote:
> If there is surely the driver can override it again before enabling the
> master bit or talking to the device ?
It could be done in PCI_FIXUP_FINAL quirks.
Ivan.
Hi!
> I noticed a lot of drivers are setting the PCI_CACHE_LINE_SIZE
> themselves, some to L1_CACHE_BYTES/sizeof(u32), others
> to arbitrary values (4, 8, 16).
>
> Then I spotted that we have a routine in the PCI subsystem
> (pdev_enable_device) that sets all these to L1_CACHE_BYTES/sizeof(u32)
> Further digging revealed that this routine was not getting called.
>
> On my Athlon box, I've noticed some devices are getting their
> default values set to 8 (where (I think) it should be 16 ?)
>
> Questions:
> 1. Is there reason for the drivers to be setting this themselves
> to hardcoded values ?
Definitely not unless the devices are buggy and need a work-around.
For PC's, we've until now relied on the BIOS setting up cache line sizes
correctly. Are the "8"'s you've spotted due to drivers messing with the
cache line register or do they come from the BIOS?
> 2. Why is pdev_device_enable no longer used ?
I haven't written this function, but I guess it's expected to be called
from the architecture specific device enabling code and that some architectures
have to call it, some don't.
Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> <[email protected]> http://atrey.karlin.mff.cuni.cz/~mj/
bug, n: A son of a glitch.
On Sat, 9 Dec 2000, Alan Cox wrote:
> > If/When x86 (or all?) architectures use this, will it make sense to
> > remove the PCI space cache line setting from drivers ?
> > Or is there borked hardware out there that require drivers to say
> > "This cacheline size must be xxx bytes, anything else will break" ?
>
> If there is surely the driver can override it again before enabling the
> master bit or talking to the device ?
Configuring PCI cacheline size with a value that is a multiple of the
right value should not break. MWIs will still write whole cache lines and
MRL and MRM may prefetch more data but this should be harmless.
But, configuring a device for a value lower that the right value of the
cache line size will break if the hardware actually invalidate cache-lines
on MWI. Bridges that alias MWI to MW will obviously not be harmed by such
a misconfiguration.
As a result, in my opinion:
- A device that requires some non zero cache line size value lower than
the right value for a given system and that actually use MWIs must not be
supported on that system, unless we know that the bridge does alias MWI to
MW. (If such a device can be configured for not using MWI, any value for
the PCI cache line size will not break).
- A driver that blindly shoe-horns some value for the cache-line size must
be fixed. Basically, it should not change the value if it is not zero and,
at least, warn user if it has changed the value because it was zero.
What are the strong reasons that let some POST softwares not fill properly
the cache line size of PCI devices ?
G?rard.
=?ISO-8859-1?Q?G=E9rard_Roudier?= writes:
> As a result, in my opinion:
>
> - A device that requires some non zero cache line size value lower than
> the right value for a given system and that actually use MWIs must not be
> supported on that system, unless we know that the bridge does alias MWI to
> MW. (If such a device can be configured for not using MWI, any value for
> the PCI cache line size will not break).
>
> - A driver that blindly shoe-horns some value for the cache-line size must
> be fixed. Basically, it should not change the value if it is not zero and,
> at least, warn user if it has changed the value because it was zero.
>
> What are the strong reasons that let some POST softwares not fill properly
> the cache line size of PCI devices ?
Erm, stupid observation coming up. Isn't this what the architecture-
specific 'pcibios_set_master' function is for? To do any architecture
specific fiddling with the device.
Surely, writing to the cache line size is not something that a driver
should be doing, but something that the architecture specific code
should be doing. Likewise, fiddling with latency timers without good
reason (eg, broken latency timers) surely is asking for problems.
_____
|_____| ------------------------------------------------- ---+---+-
| | Russell King [email protected] --- ---
| | | | http://www.arm.linux.org.uk/personal/aboutme.html / / |
| +-+-+ --- -+-
/ | THE developer of ARM Linux |+| /|\
/ | | | --- |
+-+-+ ------------------------------------------------- /\\\ |
On Sat, 9 Dec 2000, Martin Mares wrote:
> > Questions:
> > 1. Is there reason for the drivers to be setting this themselves
> > to hardcoded values ?
>
> Definitely not unless the devices are buggy and need a work-around.
Maybe that's the case. The culprits are mostly IDE interfaces. Andre ?
drivers/ide/cmd64x.c: (void) pci_write_config_byte(dev,PCI_CACHE_LINE_SIZE, 0x10);
drivers/ide/cs5530.c: pci_write_config_byte(cs5530_0,PCI_CACHE_LINE_SIZE, 0x04);
drivers/ide/hpt366.c: pci_write_config_byte(dev,PCI_CACHE_LINE_SIZE, 0x08);
drivers/ide/ns87415.c: (void) pci_write_config_byte(dev,PCI_CACHE_LINE_SIZE, 0x10);
drivers/atm/eni.c: pci_write_config_byte(eni_dev->pci_dev,PCI_CACHE_LINE_SIZE, 0x10);
drivers/media/video/planb.c: pci_write_config_byte (pdev,PCI_CACHE_LINE_SIZE, 0x8);
> For PC's, we've until now relied on the BIOS setting up cache line
> sizes correctly. Are the "8"'s you've spotted due to drivers messing
> with the cache line register or do they come from the BIOS?
>From the BIOS. They are the USB controllers, I couldn't see any writes
to the PCI_CACHE_LINE_SIZE in the drivers.
regards,
Davej.
--
| Dave Jones <[email protected]> http://www.suse.de/~davej
| SuSE Labs
> - A driver that blindly shoe-horns some value for the cache-line size must
> be fixed. Basically, it should not change the value if it is not zero and,
> at least, warn user if it has changed the value because it was zero.
>
> What are the strong reasons that let some POST softwares not fill properly
> the cache line size of PCI devices ?
In part because it is not required to, and on x86 because there may be no
PCI configuration before the OS loader completes
Here are a few more:
net/acenic.c: pci_write_config_byte(ap->pdev, PCI_CACHE_LINE_SIZE,
net/gmac.c: PCI_CACHE_LINE_SIZE, 8);
scsi/sym53c8xx.c: printk(NAME53C8XX ": PCI_CACHE_LINE_SIZE set to %d (fix-up).\n",
video/pm2fb.c: WR32(p->pci_config, PCI_CACHE_LINE_SIZE, 0xff00);
-- Jamie
[email protected] wrote:
> > > 1. Is there reason for the drivers to be setting this themselves
> > > to hardcoded values ?
> >
> > Definitely not unless the devices are buggy and need a work-around.
>
> Maybe that's the case. The culprits are mostly IDE interfaces. Andre ?
>
> drivers/ide/cmd64x.c: (void) pci_write_config_byte(dev,PCI_CACHE_LINE_SIZE, 0x10);
> drivers/ide/cs5530.c: pci_write_config_byte(cs5530_0,PCI_CACHE_LINE_SIZE, 0x04);
> drivers/ide/hpt366.c: pci_write_config_byte(dev,PCI_CACHE_LINE_SIZE, 0x08);
> drivers/ide/ns87415.c: (void) pci_write_config_byte(dev,PCI_CACHE_LINE_SIZE, 0x10);
>
> drivers/atm/eni.c: pci_write_config_byte(eni_dev->pci_dev,PCI_CACHE_LINE_SIZE, 0x10);
> drivers/media/video/planb.c: pci_write_config_byte (pdev,PCI_CACHE_LINE_SIZE, 0x8);
On Mon, 11 Dec 2000, Jamie Lokier wrote:
> Here are a few more:
>
> net/acenic.c: pci_write_config_byte(ap->pdev, PCI_CACHE_LINE_SIZE,
Acenic is at least setting it to the correct values, not hardcoding it.
> net/gmac.c: PCI_CACHE_LINE_SIZE, 8);
Ick.
> scsi/sym53c8xx.c: printk(NAME53C8XX ": PCI_CACHE_LINE_SIZE set to %d (fix-up).\n",
**vomit**
On the plus side, they made it arch independant. Shame it's incomplete.
If you look at the x86 path, its missing Pentium 4 support (x86==15).
It also screws up on Athlon where it should be set to 16, but gets 8.
I wouldn't be surprised if the other arch's were missing some definitions
too. The fact that this driver is a port of FreeBSD driver may be the
reason why SMP_CACHE_BYTES wasn't used instead, and the author opted
for that monster. But still, the whole thing is completely unnecessary.
> video/pm2fb.c: WR32(p->pci_config, PCI_CACHE_LINE_SIZE, 0xff00);
Icky.
IMO, these fixups should all get nuked. In a majority of cases, we have
half-arsed solutions that are doing just as much badness on some archs
as the goodness they were intending to provide on others.
Let the PCI layer handle all of these quirks on startup, and be done.
regards,
Davej.
--
| Dave Jones <[email protected]> http://www.suse.de/~davej
| SuSE Labs
On Mon, 11 Dec 2000, Jamie Lokier wrote:
> Here are a few more:
>
> net/acenic.c: pci_write_config_byte(ap->pdev, PCI_CACHE_LINE_SIZE,
> net/gmac.c: PCI_CACHE_LINE_SIZE, 8);
> scsi/sym53c8xx.c: printk(NAME53C8XX ": PCI_CACHE_LINE_SIZE set to %d (fix-up).\n",
For this one, this happens on Intel:
- ONLY if PCI cache line size was configured to ZERO (i.e. not
configured).
AND
- ONLY if user asked for this through the boot command line.
Anyway, the driver WARNs user about if it shoe-horns some value as you can
see above.
Btw, there is a single case where using MWI is a workaround.
Given that all known systems have a known PCI CACHE LINE SIZE for L2/L3,
if POST software + O/S PCI driver are loose enough not to provide the
RIGHT value of the PCI CACHE LINE LINE for devices that support it, what
software drivers can do ?
May-be, they should just refuse to attach the device, at least when this
information _must_ be known in order to work-around a device problem. This
will remove some ugly code for non-Intel plat-forms from the sym53c8xx
source, by the way.
Having to call some pdev_enable_device() to have the cache line size
configured looks like shit to me. After all, the BARs, INT, LATENCY TIMER,
etc.. are configured prior to entering driver probe. Why should the cache
line size be deferred to some call to some obscure mismaned thing ?
[...]
G?rard.
On Mon, 11 Dec 2000 [email protected] wrote:
> On Mon, 11 Dec 2000, Jamie Lokier wrote:
>
> > Here are a few more:
> >
> > net/acenic.c: pci_write_config_byte(ap->pdev, PCI_CACHE_LINE_SIZE,
>
> Acenic is at least setting it to the correct values, not hardcoding it.
>
> > net/gmac.c: PCI_CACHE_LINE_SIZE, 8);
>
> Ick.
>
> > scsi/sym53c8xx.c: printk(NAME53C8XX ": PCI_CACHE_LINE_SIZE set to %d (fix-up).\n",
>
> **vomit**
A BASTARD you are. Linux was born thanks to volunteers that spent
thousands of hours on their free time for helping development. If you
vomit on me, let me shit on you.
> On the plus side, they made it arch independant. Shame it's incomplete.
> If you look at the x86 path, its missing Pentium 4 support (x86==15).
Most of the code in Linux was there years ago prior to the Pentium 4 that,
by the way, looks like the buggiest thing that are ever existed.
> It also screws up on Athlon where it should be set to 16, but gets 8.
Same for this one.
> I wouldn't be surprised if the other arch's were missing some definitions
> too. The fact that this driver is a port of FreeBSD driver may be the
> reason why SMP_CACHE_BYTES wasn't used instead, and the author opted
> for that monster. But still, the whole thing is completely unnecessary.
The driver is back to FreeBSD and is intended to go to other Free O/Ses as
I will find time for.
[...]
G?rard.
Hello Gerard!
> Having to call some pdev_enable_device() to have the cache line size
> configured looks like shit to me. After all, the BARs, INT, LATENCY TIMER,
> etc.. are configured prior to entering driver probe.
Once upon a time, they used to be, but they no longer are. Unfortunately, there
are lots of bogus devices which must be never assigned BARs nor routed
interrupts. You need to call pci_enable_device() after you recognize the device
as handled by your driver to get BARs and interrupts set up. Also, if your
driver uses bus mastering, it should call pci_set_master().
> Why should the cache line size be deferred to some call to some obscure
> mismaned thing ?
See above. I'm also not joyfully jumping when I think of it, but consider
it being a tax on being compatible with the rough world of buggy PCI devices.
Anyway, it's still zillion times better than random drivers modifying such
configuration registers in random manner, knowing nothing about the host
bridge and other such stuff.
(Side note: I'm not saying the method your driver uses was bad at the time
it was designed, I'm only saying that it's wrong wrt. the rest of the kernel
and it should be gone.)
Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> <[email protected]> http://atrey.karlin.mff.cuni.cz/~mj/
CChheecckk yyoouurr dduupplleexx sswwiittcchh..
On Mon, 11 Dec 2000, Martin Mares wrote:
> Hello Gerard!
>
> > Having to call some pdev_enable_device() to have the cache line size
> > configured looks like shit to me. After all, the BARs, INT, LATENCY TIMER,
> > etc.. are configured prior to entering driver probe.
>
> Once upon a time, they used to be, but they no longer are. Unfortunately, there
> are lots of bogus devices which must be never assigned BARs nor routed
> interrupts.
Hmmm... Because of broken devices you move the burden to fine ones. Let me
disagree here.
> You need to call pci_enable_device() after you recognize the device
> as handled by your driver to get BARs and interrupts set up. Also, if your
> driver uses bus mastering, it should call pci_set_master().
I donnot see in what pci_enable_device() knows better than the tailored
software driver about what is to enable for the device and what must not
or should not. And, btw, when pci_enable_device() was born, it just
shoe-horned latency timer 64 if it was lower that 16 and I didn't want to
use so obviously loose in the first place interface.
> > Why should the cache line size be deferred to some call to some obscure
> > mismaned thing ?
>
> See above. I'm also not joyfully jumping when I think of it, but consider
> it being a tax on being compatible with the rough world of buggy PCI devices.
Blacklists are there to preserve simplicity and genericity for compliant
devices and it has the advantage of showing in a single place what and how
these devices are broken.
On the other hand, I donnot see `pci_enable_device()' in latest 2.2
kernels. It seem to beleive that the millions Linux used around the world
are rather 2.2 that 2.4. All these buggy PCI devices breaking millions of
Linux 2.2 kernels should make some noise. What did I miss here?
> Anyway, it's still zillion times better than random drivers modifying such
> configuration registers in random manner, knowing nothing about the host
> bridge and other such stuff.
Making driver for Linux work has been a nightmare for years, especially
PCI-SCSI, due to limitations and brokenness of related kernel services.
> (Side note: I'm not saying the method your driver uses was bad at the time
> it was designed, I'm only saying that it's wrong wrt. the rest of the kernel
> and it should be gone.)
The driver is not calling pci_enable_device() but is does not change
anything by default for Intel (the only one I support personnaly) if it is
provided with a correct configuration. I have used it on various Intel
platforms, even without SDMS BIOS and the only configuration item I have
seen missing sometimes has been the offending PCI cache line size.
When the driver has been tinked (not by me) in order to run on non Intel
platforms, I am been provided with various ugly PCI-related hacks that I
have incorporated with appropriate #ifdef(s), since I couldn't even test
them.
If now, the PCI stuff is claimed to be cleaned up, then _all_ the hacks
have to be removed definitely.
As a result, the driver will not work anymore on Sparc64, neither on PPC
and I am not sure it will still work on Alpha, in my opinion.
G?rard.
Date: Mon, 11 Dec 2000 21:49:52 +0100 (CET)
From: G?rard Roudier <[email protected]>
If now, the PCI stuff is claimed to be cleaned up, then _all_ the
hacks have to be removed definitely. As a result, the driver will
not work anymore on Sparc64, neither on PPC and I am not sure it
will still work on Alpha, in my opinion.
Actually Gerard, in your current 2.4.x NCR53c8xx and SYM53c8XX drivers
only real ifdefs for sparc64 are printf format strings for PCI interrupt
numbers :-)
Really, in 2.4.x sparc64 requires PCI config space hackery no longer.
Later,
David S. Miller
[email protected]
On Mon, 11 Dec 2000, David S. Miller wrote:
> Date: Mon, 11 Dec 2000 21:49:52 +0100 (CET)
> From: G?rard Roudier <[email protected]>
>
> If now, the PCI stuff is claimed to be cleaned up, then _all_ the
> hacks have to be removed definitely. As a result, the driver will
> not work anymore on Sparc64, neither on PPC and I am not sure it
> will still work on Alpha, in my opinion.
>
> Actually Gerard, in your current 2.4.x NCR53c8xx and SYM53c8XX drivers
> only real ifdefs for sparc64 are printf format strings for PCI interrupt
> numbers :-)
>
> Really, in 2.4.x sparc64 requires PCI config space hackery no longer.
Really?
I was thinking about the pcivtophys() alias bus_dvma_to_mem() hackery used
to retrieve the actual BAR address from the so-called pcidev bar cookies.
As you know the driver needs to know the actual values of MEM BARs, since
SCRIPTS may access either the IO registers and/or the on-chip RAM using
non sci-fi but actual BUS adresses (those that are actually used by PCI
transactions and that devices compare against their BARs in order to
claim access they are targetted).
Even for chips that donnot actually master themselves (896 for example),
due to LOAD/STORE and using internal cycles to access the on-chip RAM,
the actual on-chip RAM BAR address we need.
Note that if reading the BARs using pci_read_config_*() interface is
allowed, then the pcivtophys() is and was an useless thing.
About the PPC, it is the memcpy_toio() for the on-chip RAM that does not
work using iomapped bar cookie. The driver has to use SCRIPT that does
self-mastering, but self-mastering is no more compliant with PCI-2.2 as we
know.
About the Alpha. The pcivtobus/bus_dvma_to_mem thing in the driver, is not
defined as just nilpotent, but in fact it is so (d & 0xffffffff) at least
for 32 bit scsi-fi cookies.
G?rard.
Date: Mon, 11 Dec 2000 22:30:59 +0100 (CET)
From: G?rard Roudier <[email protected]>
On Mon, 11 Dec 2000, David S. Miller wrote:
> Really, in 2.4.x sparc64 requires PCI config space hackery no longer.
Really?
I was thinking about the pcivtophys() alias bus_dvma_to_mem() hackery used
to retrieve the actual BAR address from the so-called pcidev bar cookies.
Really :-) This conversation was about drivers making modifications
to PCI config space areas which are being argued to be only modified
by arch-specific PCI support layers. That is the context in which
I made my statements.
Interpreting physical BAR values is another issue altogether. Kernel
wide interfaces for this may be easily added to include/asm/pci.h
infrastructure, please just choose some sane name for it and I will
compose a patch ok? :-)
Later,
David S. Miller
[email protected]
On Mon, 11 Dec 2000, David S. Miller wrote:
> Date: Mon, 11 Dec 2000 22:30:59 +0100 (CET)
> From: G?rard Roudier <[email protected]>
>
> On Mon, 11 Dec 2000, David S. Miller wrote:
>
> > Really, in 2.4.x sparc64 requires PCI config space hackery no longer.
>
> Really?
>
> I was thinking about the pcivtophys() alias bus_dvma_to_mem() hackery used
> to retrieve the actual BAR address from the so-called pcidev bar cookies.
>
> Really :-) This conversation was about drivers making modifications
> to PCI config space areas which are being argued to be only modified
> by arch-specific PCI support layers. That is the context in which
> I made my statements.
Was more general in my opinion. :-)
> Interpreting physical BAR values is another issue altogether. Kernel
> wide interfaces for this may be easily added to include/asm/pci.h
> infrastructure, please just choose some sane name for it and I will
> compose a patch ok? :-)
Really? :-)
It is the bar cookies in pci dev structure that are insane, in my opinion.
If a driver needs BARs values, it needs actual BARs values and not some
stinking cookies. What a driver can do with BAR cookies other than using
them as band-aid for dubiously designed kernel interface.
BUT, a driver does not care about handles passed to read*/write* and
friends and should not have to care. Using cookies, handle or tag or
whatever means 'user should not worry about but just pass them when
needed' is good here.
So, if you want to fix this insane PCI interface:
1) Provide the _actual_ BARs values in the pci dev structure, otherwise
drivers that need them will have to deal with ugly hackery or access
explicitely the PCI configuration space.
2) Provide an interface that accepts the PCI dev and the BAR offset as
input and that return somes cookie for read*/write* interface.
GiveMeSomeCookieForMmIo(pcidev, bar_offset).
G?rard.
Hello!
> It is the bar cookies in pci dev structure that are insane, in my opinion.
>
> If a driver needs BARs values, it needs actual BARs values and not some
> stinking cookies. What a driver can do with BAR cookies other than using
> them as band-aid for dubiously designed kernel interface.
If a driver wants to know the BAR values, it can pick them up in the configuration
space itself. The problem is that these values mean absolutely nothing outside
the bus the device resides on. There exist zillions of translating bridges
and no driver except for the driver for the particular bridge should ever
assume anything about them.
The values in pci_dev->resource[] are not some random cookies, they are
genuine physical addresses as seen by the CPU and as accepted by ioremap().
Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> <[email protected]> http://atrey.karlin.mff.cuni.cz/~mj/
"People disagree with me. I just ignore them." -- Linus Torvalds
Date: Mon, 11 Dec 2000 23:07:01 +0100 (CET)
From: G?rard Roudier <[email protected]>
So, if you want to fix this insane PCI interface:
1) Provide the _actual_ BARs values in the pci dev structure, otherwise
drivers that need them will have to deal with ugly hackery or access
explicitely the PCI configuration space.
Tell me one valid use of this information first :-)
a) If you want to use it to arrive at addresses MEM I/O operations
you need to go through something akin to ioremap() first anyways.
b) If you wish to interpret the BAR values and use them from a BUS
perspective somehow, you still need to go through some interface
because you cannot assume what even the hw BAR values mean.
This is precisely the kind of interface I am suggesting.
Consider even just that top few bits of BAR values on some system
have some special meaning, and must be masked out before used from
PCI device side transactions. Perhaps these bits are interpreted
somehow at the host bridge when CPU accesses to device MEM or I/O
space are made. I argue not that this is compliant behavior, I
argue only that it is something idiots designing hardware will in
fact do. We have seen worse things occur. Now, subsequently, if
we start using raw BARs in drivers these systems (however important
or not important) will become difficult to impossible to support.
Here the blacklists will end up in your driver, which is where I
think both of us will agree they should not be :-)
2) Provide an interface that accepts the PCI dev and the BAR offset as
input and that return somes cookie for read*/write* interface.
GiveMeSomeCookieForMmIo(pcidev, bar_offset).
I do not understand why ioremap() is such a bletcherous interface
for you :-) You take resource in PDEV, add desired offset, and pass
it to ioremap(). What about this sequence requires you to take pain
killers? :-) It seems quite straightforward to me.
We do not want to expose physical BARs because you as a driver have
no way to portably interpret this information. On the other hand
if you tell us "Given PDEV resource X, plus offset Y, give me this
address in BUS space" we can do that and that is the interface that
makes sense and is implementable on all architectures. This is what
I am proposing for adding asm/pci.h
Having people read and intepret BARs is not implementable on all
architecures (see discussion in (b) above).
I guess there is some fundamental reason you do not like the kernel
trying to discourage access to physical BARs. This makes things so
much easier and cleaner, at least to me.
I bet we end up in standstill here and ifdef hacks remain in symbios
drivers :-))) We will see...
Later,
David S. Miller
[email protected]
>>>>> "Dave" == davej <[email protected]> writes:
Dave> Hi, I noticed a lot of drivers are setting the
Dave> PCI_CACHE_LINE_SIZE themselves, some to
Dave> L1_CACHE_BYTES/sizeof(u32), others to arbitrary values (4, 8,
Dave> 16).
Dave> Then I spotted that we have a routine in the PCI subsystem
Dave> (pdev_enable_device) that sets all these to
Dave> L1_CACHE_BYTES/sizeof(u32) Further digging revealed that this
Dave> routine was not getting called.
If it comes to that, it really should based upen SMP_CACHE_BYTES
rather than L1_CACHE_BYTES.
Jes
>>>>> "Dave" == davej <[email protected]> writes:
Dave> On Mon, 11 Dec 2000, Jamie Lokier wrote:
>> Here are a few more:
>>
>> net/acenic.c: pci_write_config_byte(ap->pdev, PCI_CACHE_LINE_SIZE,
Dave> Acenic is at least setting it to the correct values, not
Dave> hardcoding it.
Nod, it's important that it is set to the right value rather than some
random hardcoded noise. I had to add it to the AceNIC driver as I had
a motherboard that didn't set it correctly on all PCI slots.
Jes
"David S. Miller" wrote:
> Interpreting physical BAR values is another issue altogether. Kernel
> wide interfaces for this may be easily added to include/asm/pci.h
> infrastructure, please just choose some sane name for it and I will
> compose a patch ok? :-)
There's a semi-respectable use for BAR values: peer-to-peer mastering.
A new kernel interface could actually make that portable, eg:
int pci_peer_master_address(struct pci_dev *master,
struct pci_dev *target, int resource, unsigned long offset,
unsigned long *address);
Return values PCI_PEER_OK, PCI_PEER_NOWAY, PCI_PEER_FXBUG,
PCI_PEER_YOURE_JOKING_RIGHT, ... Bus address usable by master placed in
address.
Implementing something like this with a single hostbridge is simple.
It's harder on boards like my Intel 840 motherboard here, where the
33MHz and 66MHz buses don't talk to each other. It could eventually grow
into a big list of platform specific workarounds, but at least they'd
all be in one place where we could see them.
- Adrian Cox, AG Electronics
On Tue, 12 Dec 2000, Martin Mares wrote:
> Hello!
>
> > It is the bar cookies in pci dev structure that are insane, in my opinion.
> >
> > If a driver needs BARs values, it needs actual BARs values and not some
> > stinking cookies. What a driver can do with BAR cookies other than using
> > them as band-aid for dubiously designed kernel interface.
>
> If a driver wants to know the BAR values, it can pick them up in the configuration
> space itself. The problem is that these values mean absolutely nothing outside
The return value makes FULL sense on the BUS on which _real_ PCI
transactions will happen for old SYMBIOS devices and will hint recent ones
about using internal cycles instead (that are PCI 2.2 compliants) for
accessing the on chip-RAM.
As seen from the BUS and thus from the PCI device, all the opaque
inventions of O/Ses are just irrelevant sci-fi.
By the way, the hack that used bus_dvma_to_mem() from the BAR cookies is
not from me, but from David S. Miller. This will be fixed as you suggest.
> the bus the device resides on. There exist zillions of translating bridges
> and no driver except for the driver for the particular bridge should ever
> assume anything about them.
You seem to know well PCI but, in my opinion, you still have to learn much
about it and about what reality is.
You should repeat hundred times:
"It is not G?rard neither the sym driver that wants to know about
BARs"
But,
"They are these damned PCI specifications that based everything on
actual BUS address comparators and the NCR/SYMBIOS ingenieers
that based memory related SCRIPTS instructions on actual adresses
as seen from the BUS, and btw, as suggested by the specifications."
> The values in pci_dev->resource[] are not some random cookies, they are
> genuine physical addresses as seen by the CPU and as accepted by ioremap().
These cookies are confusing a lot and useless given a correct design of
related kernel interfaces. There is plenty of room in the pcidev structure
for private informations that would have avoided these stupid cookies.
In fact, these cookies are still there for historical reasons when
MMIO-capable PCI device driver(s) had to use vremap() on actual BAR
addresses. This only worked on Intel.
G?rard.
On Mon, 11 Dec 2000, David S. Miller wrote:
> Date: Mon, 11 Dec 2000 23:07:01 +0100 (CET)
> From: G?rard Roudier <[email protected]>
>
> So, if you want to fix this insane PCI interface:
>
> 1) Provide the _actual_ BARs values in the pci dev structure, otherwise
> drivers that need them will have to deal with ugly hackery or access
> explicitely the PCI configuration space.
>
> Tell me one valid use of this information first :-)
SCRIPTS. Have a look into my kind :-) response to Martin.
By the way, the genuine physical addresses, alias pcidev cookies, as seen
from the CPU have exactly NO USE at all, except as input for ioremap().
Drivers can throw them away after that. So, given correct design they
should not even have to deal with them.
> a) If you want to use it to arrive at addresses MEM I/O operations
> you need to go through something akin to ioremap() first anyways.
ioremap() is the historical successor of vremap(). Without vremap(), it
may well never have existed.
> b) If you wish to interpret the BAR values and use them from a BUS
> perspective somehow, you still need to go through some interface
> because you cannot assume what even the hw BAR values mean.
> This is precisely the kind of interface I am suggesting.
The BAR values make FULL sense on the BUS.
> Consider even just that top few bits of BAR values on some system
> have some special meaning, and must be masked out before used from
> PCI device side transactions. Perhaps these bits are interpreted
> somehow at the host bridge when CPU accesses to device MEM or I/O
> space are made. I argue not that this is compliant behavior, I
> argue only that it is something idiots designing hardware will in
> fact do. We have seen worse things occur. Now, subsequently, if
> we start using raw BARs in drivers these systems (however important
> or not important) will become difficult to impossible to support.
> Here the blacklists will end up in your driver, which is where I
> think both of us will agree they should not be :-)
Read my reply to Martin on that point.
> 2) Provide an interface that accepts the PCI dev and the BAR offset as
> input and that return somes cookie for read*/write* interface.
> GiveMeSomeCookieForMmIo(pcidev, bar_offset).
>
> I do not understand why ioremap() is such a bletcherous interface
> for you :-) You take resource in PDEV, add desired offset, and pass
> it to ioremap(). What about this sequence requires you to take pain
> killers? :-) It seems quite straightforward to me.
I can live perfectly with ioremap(). :-))
> We do not want to expose physical BARs because you as a driver have
> no way to portably interpret this information. On the other hand
> if you tell us "Given PDEV resource X, plus offset Y, give me this
> address in BUS space" we can do that and that is the interface that
> makes sense and is implementable on all architectures. This is what
> I am proposing for adding asm/pci.h
>
> Having people read and intepret BARs is not implementable on all
> architecures (see discussion in (b) above).
>
> I guess there is some fundamental reason you do not like the kernel
> trying to discourage access to physical BARs. This makes things so
> much easier and cleaner, at least to me.
>
> I bet we end up in standstill here and ifdef hacks remain in symbios
> drivers :-))) We will see...
I will wait for your .txt file that describes your idea. Your
documentation about the new DMA mapping had been extremally useful.
Let me thank you again for it.
Bye,
G?rard.
Date: Tue, 12 Dec 2000 20:17:21 +0100 (CET)
From: G?rard Roudier <[email protected]>
On Mon, 11 Dec 2000, David S. Miller wrote:
> Tell me one valid use of this information first :-)
SCRIPTS. Have a look into my kind :-) response to Martin.
Ok, this I understand.
> b) If you wish to interpret the BAR values and use them from a BUS
> perspective somehow, you still need to go through some interface
> because you cannot assume what even the hw BAR values mean.
> This is precisely the kind of interface I am suggesting.
The BAR values make FULL sense on the BUS.
I am saying there may be systems where it does not make any sense,
f.e. actually used bits of BAR depend upon whether CPU, or DEVICE on
that bus, or DEVICE on some other bus make the access.
Forget all the PCI specifications, it is irrelevant here. All your
PCI expertiece means nothing, nor mine. People build dumb machines
with "PCI implementations" and we need to handle them.
I will wait for your .txt file that describes your idea. Your
documentation about the new DMA mapping had been extremally useful.
Let me thank you again for it.
It requires no .txt file :-), it will just be formalization of
existing bus_to_dvma_whatever hack :-) Specify PDEV (device) and
RESNUM (which I/O or MEM resource for that device), returns either
error or address as seen by BUS that PDEV is on. You may offset
this return value as desired, up to the size of that resource.
I could make a more elaborate interface (add new parameter,
PDEV_MASTER which is device which wishes to access area described by
PDEV+RESNUM), allowing full PCI peer-to-peer setup, as described by
someone else in another email of this thread. This version would have
an error return, since there will be peer2peer situations on some
systems which cannot be made. But I feel this is inappropriate until
2.5.x, others can disagree.
Later,
David S. Miller
[email protected]
On Tue, 12 Dec 2000, David S. Miller wrote:
> Date: Tue, 12 Dec 2000 20:17:21 +0100 (CET)
> From: G?rard Roudier <[email protected]>
>
> On Mon, 11 Dec 2000, David S. Miller wrote:
>
> > Tell me one valid use of this information first :-)
>
> SCRIPTS. Have a look into my kind :-) response to Martin.
>
> Ok, this I understand.
>
> > b) If you wish to interpret the BAR values and use them from a BUS
> > perspective somehow, you still need to go through some interface
> > because you cannot assume what even the hw BAR values mean.
> > This is precisely the kind of interface I am suggesting.
>
> The BAR values make FULL sense on the BUS.
>
> I am saying there may be systems where it does not make any sense,
> f.e. actually used bits of BAR depend upon whether CPU, or DEVICE on
> that bus, or DEVICE on some other bus make the access.
>
> Forget all the PCI specifications, it is irrelevant here. All your
> PCI expertiece means nothing, nor mine. People build dumb machines
> with "PCI implementations" and we need to handle them.
Even the dumbest PCI implementation will keep with BAR relevance. Reason
is that PCI devices are using BAR values and corresponding size to make
decision about claiming or not a transaction as target.
You can be as dump as you want with PCI, but not that much. :-)
> I will wait for your .txt file that describes your idea. Your
> documentation about the new DMA mapping had been extremally useful.
> Let me thank you again for it.
>
> It requires no .txt file :-),
No problems, a ".text" file would also fit just fine. :-)
> it will just be formalization of
> existing bus_to_dvma_whatever hack :-) Specify PDEV (device) and
> RESNUM (which I/O or MEM resource for that device), returns either
> error or address as seen by BUS that PDEV is on. You may offset
> this return value as desired, up to the size of that resource.
>
> I could make a more elaborate interface (add new parameter,
> PDEV_MASTER which is device which wishes to access area described by
> PDEV+RESNUM), allowing full PCI peer-to-peer setup, as described by
> someone else in another email of this thread. This version would have
> an error return, since there will be peer2peer situations on some
> systems which cannot be made. But I feel this is inappropriate until
> 2.5.x, others can disagree.
I saw the proposal.
Btw, unlike the person, that proposed it, that will be able to test
peer-to-peer unability only, my current machine will allow to test
peer-to-peer ability only between 2 different PCI BUSes. :-)
For now, my intention is to encapsulate the right interface as seen from
my brain device in macros and forget about it until a new interface will
be provided. I will first implement it on SYM-2 and backport changes to
sym53c8xx later. And since I need the new major driver version to be
tested on non-Intel platforms, this will make full synergy for the
testings. :-)
Bye,
G?rard.
Date: Tue, 12 Dec 2000 21:28:18 +0100 (CET)
From: G?rard Roudier <[email protected]>
You can be as dump as you want with PCI, but not that much. :-)
Your point is well taken.
Btw, unlike the person, that proposed it, that will be able to test
peer-to-peer unability only, my current machine will allow to test
peer-to-peer ability only between 2 different PCI BUSes. :-)
For now, my intention is to encapsulate the right interface as seen from
my brain device in macros and forget about it until a new interface will
be provided. I will first implement it on SYM-2 and backport changes to
sym53c8xx later. And since I need the new major driver version to be
tested on non-Intel platforms, this will make full synergy for the
testings. :-)
Ok, meanwhile I will try to code up the generic interface. It will
work like this: 1) I will code up something I know each port can
implement 2) I will show it to those here and everyone can tell me if
they can make use of it at all :-)
Later,
David S. Miller
[email protected]