Messages in this thread Patch in this message |  | | Date | Thu, 06 Sep 2001 15:57:44 -0500 | From | Todd Inglett <> | Subject | pci_read_bridge_bases bug and sign extend problems |
| |
In 2.4.9-ac7 I've found the following two problems in pci_read_bridge_bases(). First, the code is testing "base" for the PCI_IO_RANGE_TYPE_32 when it should be testing "io_base_lo" (base is already shifted up 8 bits). The tests on the mem bases are ok.
Second, the code does some shifting which is not safe when sizeof(long) < sizeof(int) (i.e. 64 bit). As I understand it (and observe) in the assignment like "base = (mem_base_lo << 16)" the u16 mem_base_lo gets promoted to int before the assignment. The resulting signed int gets sign extended to fit into the unsigned long base. The solution is to cast the u16's to unsigned long.
Finally, it seems odd that resource[1] here is handled differently than resource[2]. I'm not a pci wizard, though, so I may be missing something. But recently this code has been patched to handle io range types better and it seems resource[1] was missed. I don't include anything in the patch for this, though.
Patch attached (relative to 2.4.9-ac7 though ac9 appears the same). -- -todd--- drivers/pci/pci.c 27 Aug 2001 19:06:08 -0000 1.8 +++ drivers/pci/pci.c 6 Sep 2001 19:23:57 -0000 @@ -967,10 +967,10 @@ res = child->resource[0]; pci_read_config_byte(dev, PCI_IO_BASE, &io_base_lo); pci_read_config_byte(dev, PCI_IO_LIMIT, &io_limit_lo); - base = (io_base_lo & PCI_IO_RANGE_MASK) << 8; - limit = (io_limit_lo & PCI_IO_RANGE_MASK) << 8; + base = (unsigned long)(io_base_lo & PCI_IO_RANGE_MASK) << 8; + limit = (unsigned long)(io_limit_lo & PCI_IO_RANGE_MASK) << 8; - if ((base & PCI_IO_RANGE_TYPE_MASK) == PCI_IO_RANGE_TYPE_32) { + if ((io_base_lo & PCI_IO_RANGE_TYPE_MASK) == PCI_IO_RANGE_TYPE_32) { u16 io_base_hi, io_limit_hi; pci_read_config_word(dev, PCI_IO_BASE_UPPER16, &io_base_hi); pci_read_config_word(dev, PCI_IO_LIMIT_UPPER16, &io_limit_hi); @@ -995,8 +995,8 @@ res = child->resource[1]; pci_read_config_word(dev, PCI_MEMORY_BASE, &mem_base_lo); pci_read_config_word(dev, PCI_MEMORY_LIMIT, &mem_limit_lo); - base = (mem_base_lo & PCI_MEMORY_RANGE_MASK) << 16; - limit = (mem_limit_lo & PCI_MEMORY_RANGE_MASK) << 16; + base = (unsigned long)(mem_base_lo & PCI_MEMORY_RANGE_MASK) << 16; + limit = (unsigned long)(mem_limit_lo & PCI_MEMORY_RANGE_MASK) << 16; if (base && base <= limit) { res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM; res->start = base; @@ -1011,16 +1011,16 @@ res = child->resource[2]; pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo); pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo); - base = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16; - limit = (mem_limit_lo & PCI_PREF_RANGE_MASK) << 16; + base = (unsigned long)(mem_base_lo & PCI_PREF_RANGE_MASK) << 16; + limit = (unsigned long)(mem_limit_lo & PCI_PREF_RANGE_MASK) << 16; if ((mem_base_lo & PCI_PREF_RANGE_TYPE_MASK) == PCI_PREF_RANGE_TYPE_64) { u32 mem_base_hi, mem_limit_hi; pci_read_config_dword(dev, PCI_PREF_BASE_UPPER32, &mem_base_hi); pci_read_config_dword(dev, PCI_PREF_LIMIT_UPPER32, &mem_limit_hi); #if BITS_PER_LONG == 64 - base |= ((long) mem_base_hi) << 32; - limit |= ((long) mem_limit_hi) << 32; + base |= ((unsigned long) mem_base_hi) << 32; + limit |= ((unsigned long) mem_limit_hi) << 32; #else if (mem_base_hi || mem_limit_hi) { printk(KERN_ERR "PCI: Unable to handle 64-bit address space for %s\n", child->name); |  |