Re: Boot hangs "Probing PCI hardware"

Dear All:

Thank you for your quick response.

David Mosberger wrote:
>
>  Tatsuya> There are some BAR operations in pci_read_bases() function
>  Tatsuya> of pci.c to get memory and IO space ranges assigned for
>  Tatsuya> each PCI device by writing 0xffffffff on BAR and reading it
>  Tatsuya> again. If an interrupt from SAPIC occurs during BAR
>  Tatsuya> operations, the first PCI Bus hangs.
>
>Interesting.
>
>  Tatsuya> In this case, the video device gets 64MB memory space from
>  Tatsuya> 0xFC000000 to 0xFFFFFFFF improperly, and it conflicts with
>  Tatsuya> System reserve region (0xFCxxxxxx - 0xFExxxxxx) for SAPIC
>  Tatsuya> interrupt messages. After that the video device reacts to
>  Tatsuya> an SAPIC interrupt improperly.
>
>  Tatsuya> According to PCI specifications, it is necessary to disable
>  Tatsuya> Memory IO target function during BAR operations by setting
>  Tatsuya> zero to bit0/1 of PCI_COMMAND, but current pci_read_bases()
>  Tatsuya> doesn't do that.
>
>Makes sense.  Just for completeness (and to save me some time), could
>you point out the section in the PCI spec that requires this?

Please refer the following specification.

PCI Local Bus Specification
Revision 2.2
December 18, 1998

P.204 of 6.2.5.1. Address Maps
It says as follows.
"Implementation Note: Sizing a 32 bit Base Address Register
Example
Decode (I/O or memory) of a register is disabled via the command register
before sizing a Base Address register.
  :
The original value in the Base Address register is restored before
re-enabling decode in the command register of the device."

It is available from http://www.pcisig.com/home

>  Tatsuya> I'd like to provide the following patch, and I've already
>  Tatsuya> found it works well on both RHL7.2 and Kernel-2.4.18.
>
>The patch looks mostly good to me, except that I think it would be better
>to replace:
>
>        pci_write_config_word(dev, PCI_COMMAND, pcicmd_sv & 0xfffc);
>
>with:
>
>        pci_write_config_word(dev, PCI_COMMAND,
>			pcicmd_sv & ~(PCI_COMMAND_IO|PCI_COMMAND_MEMORY));
>
>also, a comment pointing to the appropriate PCI section might be
>helpful.

Thank you for your advise. I'd like to revise them as follows.


*** pci.c.orig  Fri Apr  5 18:16:22 2002
--- pci.c       Fri Apr  5 18:50:26 2002
***************
*** 939,946 ****
--- 939,955 ----
  {
        unsigned int pos, reg, next;
        u32 l, sz;
+       u16 pcicmd_sv;
        struct resource *res;

+       /*
+        * Decode (I/O or memory) of a register is disabled via the command register
+        * before sizing a Base Address register.
+        */
+       pci_read_config_word(dev, PCI_COMMAND, &pcicmd_sv);
+       pci_write_config_word(dev, PCI_COMMAND,
+                       pcicmd_sv & ~(PCI_COMMAND_IO|PCI_COMMAND_MEMORY));
+
        for(pos=0; pos<howmany; pos = next) {
                next = pos+1;
                res = &dev->resource[pos];
***************
*** 1004,1009 ****
--- 1013,1022 ----
                }
                res->name = dev->name;
        }
+       /*
+        * re-enabling decode in the command register of the device.
+        */
+       pci_write_config_word(dev, PCI_COMMAND, pcicmd_sv);
  }

  void __devinit  pci_read_bridge_bases(struct pci_bus *child)


>Are you going to submit this directly to the PCI maintainer?  Even
>though it happens to show on ia64, it's a generic bug.  (And why not
>cc [email protected] while you're at it? ;-)

Actually I've already sent it to the PCI maintainer.
And I'll also send it to [email protected] according your comments.

Best regards,
Tatsuya Tsurukawa
Mitsubishi Electric