Bjorn Helgaas wrote:
> On Tuesday 01 December 2009 12:03:57 am Yinghai Lu wrote:
>> Alex found one system that one pci bridge pref mmio 64 is not set correctly.
>> aka, the upper32 base/limit is not cleaned.
>> he found that bridge is supporting 64 bit pref mmio, but device under that
>> does not support that. so that IORESOURCE_MEM_64 get cleared in pbus_size_mem()
>
> I think it's wrong that pbus_size_mem() fiddles with IORESOURCE_MEM_64
> in bus resources based on where BARs of devices on that bus live. That
> feels fragile.
yes. need more clean up.
>
> The question of whether the bridge supports 64-bit apertures is
> strictly a hardware property of the bridge. It has nothing to do
> with where we place downstream devices.
>
> Is there really a problem with writing to PCI_PREF_BASE_UPPER32
> unconditionally? As Alex pointed out, per 3.2.5.10 of the bridge
> spec, the UPPER32 registers are read-only if only 32-bit apertures
> are supported. If you mentioned a problem with doing this
> unconditionally, I missed it.
remember that some x2apic registers say they are reserved, and read them could cause GP error.
and for pci devices, if it is read-only, for good design, then write it should be ok.
but if there is some design problem in devices, and they could say those are read-only.
why kernel write value to it?
>
> The only place we test IORESOURCE_MEM_64 for a bus resource is when
> we're programming PCI_PREF_BASE_UPPER32. If we think it's important
> to program it conditionally, why don't we skip IORESOURCE_MEM_64
IORESOURCE_MEM_64 is used to control if we can use MMIO > 4g.
> altogether, and just look at the bits in PCI_PREF_MEMORY_BASE directly?
> E.g., something like this:
>
> pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &l);
> if ((l & PCI_PREF_RANGE_TYPE_MASK) == PCI_PREF_RANGE_TYPE_64) {
> pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
> pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
> }
>
> Then we don't have to maintain flags at all, and it's easy to verify
> that the code corresponds to the spec.
that will have several extra read, and also we already store that bit in pci_read_bridge_bases()
but forget to set that in pci_check_bus_range()
YH
YH
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
