On Tue, Apr 15, 2014 at 8:29 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
> On Tue, Apr 15, 2014 at 5:09 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
>>> It went wrong at the beginning. Note the error message never considers
>>> 64-bit or not, but BAR 15 here has it MEM_64 flag cleared.
>>
>> BAR 15 is a bridge window. I think its resource flags should reflect
>> the capability of the *window*, even if we disable the window or we
>> happen to assign addresses that are under 4GB. So I think it's wrong
>> that we clear the MEM_64 flag in pbus_size_mem() and the IO flag in
>> pbus_size_io().
>
> We keep PCI_PREF_RANGE_TYPE_64 in the flag.
I think we should stop putting PCI_PREF_RANGE_TYPE_64 in res->flags.
I don't think we ever test for PCI_PREF_RANGE_TYPE_64 today, except
for the trivial usage in pci_read_bridge_mmio_pref(), which can be
easily removed.
Everywhere else, we use IORESOURCE_MEM, IORESOURCE_MEM_64, and
IORESOURCE_PREFETCH to keep track of the properties of the hardware
underlying the struct resource, i.e., the BAR. Why shouldn't we do
the same for bridge windows?
The struct resource flags should tell us the *possible* properties of
the window, e.g., what the bridge can support. These properties are
fixed by the bridge hardware, regardless of what devices happen to be
below the bridge. We discover those properties at enumeration-time,
and we shouldn't change them afterwards.
> We should check that before we touch PCI_PREF_LIMIT_UPPER32 BAR in
> pci_setup_bridge_mmio_pref()
>
> like following:
>
> Subject: [PATCH] PCI: only touch upper 32bit for pref64bit bridge bar
>
> If the bridge pref mmio bar does not support 64bit, we should touch
> upper 32bit.
>
> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
>
> ---
> drivers/pci/setup-bus.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> Index: linux-2.6/drivers/pci/setup-bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/setup-bus.c
> +++ linux-2.6/drivers/pci/setup-bus.c
> @@ -592,23 +592,23 @@ static void pci_setup_bridge_mmio(struct
> static void pci_setup_bridge_mmio_pref(struct pci_bus *bus)
> {
> struct pci_dev *bridge = bus->self;
> - struct resource *res;
> + struct resource *res = bus->resource[2];
> struct pci_bus_region region;
> u32 l, bu, lu;
>
> /* Clear out the upper 32 bits of PREF limit.
> If PCI_PREF_BASE_UPPER32 was non-zero, this temporarily
> disables PREF range, which is ok. */
> - pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, 0);
> + if (res->flags & PCI_PREF_RANGE_TYPE_64)
> + pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, 0);
>
> /* Set up PREF base/limit. */
> bu = lu = 0;
> - res = bus->resource[2];
> pcibios_resource_to_bus(bridge->bus, ®ion, res);
> if (res->flags & IORESOURCE_PREFETCH) {
> l = (region.start >> 16) & 0xfff0;
> l |= region.end & 0xfff00000;
> - if (res->flags & IORESOURCE_MEM_64) {
> + if (res->flags & PCI_PREF_RANGE_TYPE_64) {
> bu = upper_32_bits(region.start);
> lu = upper_32_bits(region.end);
> }
> @@ -619,8 +619,10 @@ static void pci_setup_bridge_mmio_pref(s
> pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, l);
>
> /* Set the upper 32 bits of PREF base & limit. */
> - pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, bu);
> - pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, lu);
> + if (res->flags & 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);
> + }
> }
>
> static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type)
--
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
