PowerPC Linux and PCI

PowerPC Linux and PCI

[James L Peterson]

I apologize if this message is directed incorrectly; any hints you could
give on who else to ask on this would be appreciated.

I'm trying to get Linux running on a 405GP system with PCI.  Actually
it's on the mambo simulator and I'm writing the PCI support.  My
information about PCI comes from the PCI System Architecture
book by MindShare; also the PPC405GP User's Manual. My problem is
basically a big-endian/little-endian problem.  All of the documents
agree that the Vendor ID is in bytes 0 and 1, and the Device ID
is in bytes 2 and 3.  The header file include/linux/pci.h agrees:

#define PCI_VENDOR_ID  0x00 /* 16 bits */
#define PCI_DEVICE_ID  0x02 /* 16 bits */

But in drivers/pci/pci.c, when we are trying to discover
a particular device, in pci_scan_device, the code
reads out a dword (a 32-bit quantity) and then masks
out the two subfields:

if (pci_read_config_dword(temp, PCI_VENDOR_ID, &l))
  return NULL;
     ....
  memcpy(dev, temp, sizeof(*dev));
 dev->vendor = l & 0xffff;
 dev->device = (l >> 16) & 0xffff;

It seems to me this is incorrect for a big-endian machine
(like PowerPC).  If we read the two 16-bit parts out of the
first 32-bit part, we will end up with:

        0       1            2        3
        vendor-id       device-id

with a big-endian machine, but

       0        1           2         3
      device-id         vendor-id

for a little endian machine.  This would make the mask and
shift definition to the vendor and device field correct for little
endian, but swapped for big-endian.

There is a similar problem for the read of the Class Code
and Revision ID dword in pci_setup_device, where, again,
they read the dword and then shift to get the "upper 3  bytes"
while on a big-endian machine, you would actually want the
"lower 3 bytes" for the 3-byte Class Code field.

The fixes to this code is fairly simple -- always read the size
of field that is defined from the bytes where it is defined to be --
but I am more confused by why this has not shown up before.
This seems a fundamental problem -- incorrectly defining the
device and vendor fields for a PCI device -- which would have
prevented PCI devices from working on PowerPC systems
(or any other big-endian system), but I don't see #ifdef's for
it, or platform-specific code to correct it or anything else.
Yet since these fields are defined by hardware manufacturers,
I would expect they are always as defined by the PCI document,
that is always little-endian.  So it seems that the code can't work,
and yet ...

I'd appreciate any comments or recommendations.

jim

James Peterson
IBM Austin Research Lab
Austin, Texas

Re: PowerPC Linux and PCI

[David S. Miller]

   From: James L Peterson <peterson@austin.ibm.com>
   Date: Fri, 19 Apr 2002 16:37:03 -0500

   if (pci_read_config_dword(temp, PCI_VENDOR_ID, &l))
     return NULL;
        ....
     memcpy(dev, temp, sizeof(*dev));
    dev->vendor = l & 0xffff;
    dev->device = (l >> 16) & 0xffff;
   
   It seems to me this is incorrect for a big-endian machine
   (like PowerPC).  If we read the two 16-bit parts out of the
   first 32-bit part, we will end up with:

pci_read_config_dword should do the byte swapping on &l for
the caller, fix your pci_{read,write}_config_*() arch implementation.

Re: PowerPC Linux and PCI

[Paul Mackerras]

David S. Miller writes:

>    From: James L Peterson <peterson@austin.ibm.com>
>    Date: Fri, 19 Apr 2002 16:37:03 -0500
> 
>    if (pci_read_config_dword(temp, PCI_VENDOR_ID, &l))
>      return NULL;
>         ....
>      memcpy(dev, temp, sizeof(*dev));
>     dev->vendor = l & 0xffff;
>     dev->device = (l >> 16) & 0xffff;
>    
>    It seems to me this is incorrect for a big-endian machine
>    (like PowerPC).  If we read the two 16-bit parts out of the
>    first 32-bit part, we will end up with:
> 
> pci_read_config_dword should do the byte swapping on &l for
> the caller, fix your pci_{read,write}_config_*() arch implementation.

It does, that's why it all works. :)  James Peterson seems to have
missed this fact, hence his confusion.

Paul.

Re: PowerPC Linux and PCI

[James L Peterson]

I haven't overlooked the fact that the pci_read_config_dword will
do the byte swapping.  But it will do the byte_swapping for a
dword (32-bits), not for two words (16-bits), and the byte-swapping
for 32-bits is not the same as for two words.

Specifically:

Little-endian dword:

          3    2      1    0
         80  86    71 11

when read in by the pci_read_config_dword,  will swap it to :

        0    1      2     3
        11 71     86  80

For little endian you get device id (bytes 2,3) as 0x8086,
and vendor id of 0x7111.  For big endian you get device
id of 0x7111 and vendor id of 0x8086.

The pci_read_config_dword works just fine for dword
data types, like the base addresses.  But the fundamental
rule of big-endian/little-endian code is that you have to
know the size of the data type that you are working with,
so that you can byte swap properly.  Reading a dword
(and byte-swapping for a dword) is not the same as
reading two words (and byte-swapping them as words).

jim



Contrast this with replacing the read_config_dword with
reading two words:

Instead of:

if (pci_read_config_dword(temp, PCI_VENDOR_ID, &l))
  return NULL;
     ....
  memcpy(dev, temp, sizeof(*dev));
 dev->vendor = l & 0xffff;
 dev->device = (l >> 16) & 0xffff;

It would seem that we need:

 if (pci_read_config_word(temp, PCI_VENDOR_ID, &vendor))
  return NULL;
 if (pci_read_config_word(temp, PCI_DEVICE_ID, &device))
  return NULL;



Paul Mackerras wrote:

> David S. Miller writes:
>
> >    From: James L Peterson <peterson@austin.ibm.com>
> >    Date: Fri, 19 Apr 2002 16:37:03 -0500
> >
> >    if (pci_read_config_dword(temp, PCI_VENDOR_ID, &l))
> >      return NULL;
> >         ....
> >      memcpy(dev, temp, sizeof(*dev));
> >     dev->vendor = l & 0xffff;
> >     dev->device = (l >> 16) & 0xffff;
> >
> >    It seems to me this is incorrect for a big-endian machine
> >    (like PowerPC).  If we read the two 16-bit parts out of the
> >    first 32-bit part, we will end up with:
> >
> > pci_read_config_dword should do the byte swapping on &l for
> > the caller, fix your pci_{read,write}_config_*() arch implementation.
>
> It does, that's why it all works. :)  James Peterson seems to have
> missed this fact, hence his confusion.
>
> Paul.

Re: PowerPC Linux and PCI

[Paul Mackerras]

James L Peterson writes:

> I haven't overlooked the fact that the pci_read_config_dword will
> do the byte swapping.  But it will do the byte_swapping for a
> dword (32-bits), not for two words (16-bits), and the byte-swapping
> for 32-bits is not the same as for two words.

The host bridge should preserve the byte numbering, i.e. byte address
0 on the host side should access byte 0 on the PCI side, not byte 3.
In your example:

> Specifically:
> 
> Little-endian dword:
> 
>           3    2      1    0
>          80  86    71 11

So we have byte[0] = 0x11, byte[1] = 0x71, byte[2] = 86, byte[3] =
0x80, right?  If we read individual bytes we will get the same results
on a little-endian or a big-endian platform.

> when read in by the pci_read_config_dword,  will swap it to :
> 
>         0    1      2     3
>         11 71     86  80
> 
> For little endian you get device id (bytes 2,3) as 0x8086,
> and vendor id of 0x7111.  For big endian you get device
> id of 0x7111 and vendor id of 0x8086.

On a little endian platform, pci_read_config_dword will return
0x80867111, since byte 0 is the least-significant byte.  Thus we have
vendor id = 0x7111 and device-id = 0x8086.

On a big-endian platform a 32-bit read will return 0x11718680 (since
byte 0 is the most significant byte), and pci_read_config_dword will
byte-swap that to 0x80867111.  Thus we once again have vendor id =
0x7111 and device-id = 0x8086.

> The pci_read_config_dword works just fine for dword
> data types, like the base addresses.  But the fundamental
> rule of big-endian/little-endian code is that you have to
> know the size of the data type that you are working with,
> so that you can byte swap properly.  Reading a dword
> (and byte-swapping for a dword) is not the same as
> reading two words (and byte-swapping them as words).

You do in fact get the same effect because of the invariance of the
individual byte addresses.

Doesn't the fact that people have been successfully using PCI devices
in PowerPC machines since 1995 or 1996 suggest to you that it might be
your understanding that is faulty rather than the code? :)

Paul.

Re: PowerPC Linux and PCI

[David S. Miller]

   From: Paul Mackerras <paulus@samba.org>
   Date: Tue, 23 Apr 2002 18:00:47 +1000 (EST)
   
   Doesn't the fact that people have been successfully using PCI devices
   in PowerPC machines since 1995 or 1996 suggest to you that it might be
   your understanding that is faulty rather than the code? :)

And sparc64 :-)

An important point to mention is that big endian systems need to do
byte twisting in the PCI controller for all the byte-lane issues to
work out properly.

Maybe this guys box has a broken PCI host bridge implementation that
doesn't do the byte-twisting and we should consider that in our
analysis of his problems :-)

Re: PowerPC Linux and PCI

[James L Peterson]

What does this mean?  This suggests that PCI controller for
big-endian systems are not interchangable with PCI controllers
for little-endian systems, because the controller itself does
byte swapping (is that what you mean by "byte twisting"?)

jim



"David S. Miller" wrote:

> An important point to mention is that big endian systems need to do
> byte twisting in the PCI controller for all the byte-lane issues to
> work out properly.
>

Re: PowerPC Linux and PCI

[James L Peterson]

Paul Mackerras wrote:

> > Little-endian dword:
> >
> >           3    2      1    0
> >          80  86    71 11
>
> So we have byte[0] = 0x11, byte[1] = 0x71, byte[2] = 86, byte[3] =
> 0x80, right?  If we read individual bytes we will get the same results
> on a little-endian or a big-endian platform.
>
> On a little endian platform, pci_read_config_dword will return
> 0x80867111, since byte 0 is the least-significant byte.  Thus we have
> vendor id = 0x7111 and device-id = 0x8086.
>
> On a big-endian platform a 32-bit read will return 0x11718680 (since
> byte 0 is the most significant byte), and pci_read_config_dword will
> byte-swap that to 0x80867111.  Thus we once again have vendor id =
> 0x7111 and device-id = 0x8086.
>

Actually, in our case, the first dword was 0x80867111 (little-endian),
so when we read the first dword, we got 0x80867111 (big-endian),
since we were reading a dword, not individual bytes. If we read the
individual bytes, we do get the right bytes, but the PCI subsystem
returns the same bytes to us for either a little-endian or big-endian
dword read, since it doesn't know if the processor is big-endian
or little-endian  (The PowerPC can be both, or different processors
on the same bus can be different at the same time).


> Doesn't the fact that people have been successfully using PCI devices
> in PowerPC machines since 1995 or 1996 suggest to you that it might be
> your understanding that is faulty rather than the code? :)

Well, when you have new hardware, a new compiler (with known bugs),
and a new processor chip, the fact that it works for others doesn't mean
that it's right.  It hardly seems wise to assume there are no problems
left
in the software.  My original question was "How can this work (since it
clearly must be working) when it does not appear to be correct?"
It's not as if the code is clearly documented that it depends upon the
PCI hardware knowing if it should return big-endian or little-endian
data.

jim

RE: PowerPC Linux and PCI

[Ed Vance]

James L Peterson wrote: 
> 
> "David S. Miller" wrote:
> > 
> > An important point to mention is that big endian systems need to do
> > byte twisting in the PCI controller for all the byte-lane issues to
> > work out properly.
> 
> What does this mean?  This suggests that PCI controller for
> big-endian systems are not interchangeable with PCI controllers
> for little-endian systems, because the controller itself does
> byte swapping (is that what you mean by "byte twisting"?)

I think David's reference is to the system's PCI subsystem/interface rather
than to the PCI cards plugged into it.

Ed Vance

---------------------------------------------------------------- 
Ed Vance              edv@macrolink.com
Macrolink, Inc.       1500 N. Kellogg Dr  Anaheim, CA  92807
----------------------------------------------------------------

Re: PowerPC Linux and PCI

[David S. Miller]

   From: Ed Vance <EdV@macrolink.com>
   Date: Tue, 23 Apr 2002 10:16:16 -0700

   James L Peterson wrote: 
   > What does this mean?  This suggests that PCI controller for
   > big-endian systems are not interchangeable with PCI controllers
   > for little-endian systems, because the controller itself does
   > byte swapping (is that what you mean by "byte twisting"?)
   
   I think David's reference is to the system's PCI subsystem/interface rather
   than to the PCI cards plugged into it.

No, I'm talking about the "PCI host controller" the thing that
connects the PCI bus to the system bus :-)

And to address James's concern, yes unmodified such a PCI controller
won't work on a little-endian system, but any sane hardware designed
would add at least a jumper of some sort to switch the behavior of
the endianness features.

All of this has to do with what "byte X" within a word means when it
comes from a little endian vs. a big endian processor.

Can I ask you to have a stare at the following document before
continuing this thread further?

	http://www.sun.com/processors/manuals/802-7835.pdf

In particular, I wish you would read Chapter 10 "endianness support".
it starts on page 115.  Sun's controller does things right, compare it
with yours to see if it deals with the byte lanes correctly when
hooked up to a big endian processor :-)

Note your original problem is an absolutely trite example, if that
code in the kernel doesn't work you are going to have tons of other
problems elsewhere when accessing things on the PCI bus.

RE: PowerPC Linux and PCI

[Ed Vance]

Hi Dave,

David S. Miller wrote:
> 
>    From: Ed Vance <EdV@macrolink.com>
>    Date: Tue, 23 Apr 2002 10:16:16 -0700
> 
>    James L Peterson wrote: 
>    > What does this mean?  This suggests that PCI controller for
>    > big-endian systems are not interchangeable with PCI controllers
>    > for little-endian systems, because the controller itself does
>    > byte swapping (is that what you mean by "byte twisting"?)
>    
>    I think David's reference is to the system's PCI subsystem/interface 
>    rather than to the PCI cards plugged into it. 
> 
> No, I'm talking about the "PCI host controller" the thing that
> connects the PCI bus to the system bus :-)

Hi Dave,

Yup, same thing I was talking about. In technical terms, the thingy that 
marries the processor to the PCI bus. Some SBC's have more than one bus in
front of the PCI bus. Hmmm... I've heard that such marriages are illegal in
Texas. ;-)

Hi Jim,

Any time two unlike buses have a nexus, the nexus hardware has the major 
responsibility to resolve the issues of that nexus, whatever they may be. 
Nothing new here. Ordering of the PCI bus is always the same, as specified
in the spec, regardless of how you get there. ... And it's not always
obvious from the code what is going on in the hardware, especially if it's
_good_ hardware that makes the driver writer's life easier. 

It's all magic ...

Ed

---------------------------------------------------------------- 
Ed Vance              edv@macrolink.com
Macrolink, Inc.       1500 N. Kellogg Dr  Anaheim, CA  92807
----------------------------------------------------------------