diff mbox series

[RFC] PCI: Fix prefetchable range broken in pci_bridge_check_ranges

Message ID 1506151482-113560-1-git-send-email-wangzhou1@hisilicon.com
State Not Applicable
Headers show
Series [RFC] PCI: Fix prefetchable range broken in pci_bridge_check_ranges | expand

Commit Message

Zhou Wang Sept. 23, 2017, 7:24 a.m. UTC 
When double checking 64bit prefetch range, we will change the prefetch range
in a time slot by writing 0xffffffff to PCI_PREF_BASE_UPPER32. This may break
transfers through related bridge at that time.

E.g. if we have below PCIe topology:

-[0000:00]-+-00.0-[01-02]--+-00.0  Device 8086:10fb
           |               \-00.1  Device 8086:10fb
           \-08.0-[03]----00.0  Device 8086:0953

When rescan 00:08.0, it will call:

        pci_rescan_bus
                -> pci_assign_unassigned_bus_resources
                        -> __pci_bus_size_bridges
                                -> pci_bridge_check_ranges
                                        -> pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, 0xffffffff)

This will change the prefetch range of 00:00.0 in a time slot, so traffic of
01:00.0 or 01:00.1 may be broken.

In fact, we can get if one bridge supports 64bit range by the bottom 4bits of
prefetchable memory base/limit. Honestly speaking, I don't know why 1f82de10d6b1
("PCI/86: don't assume prefetchable ranges are 64bit") has added the double
check code.

So Can we remove the double checking of prefetchable range to avoid this problem?

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 drivers/pci/setup-bus.c | 14 --------------
 1 file changed, 14 deletions(-)

Comments

Zhou Wang Sept. 30, 2017, 1:15 a.m. UTC | #1 
On 2017/9/23 15:24, Zhou Wang wrote:
> When double checking 64bit prefetch range, we will change the prefetch range
> in a time slot by writing 0xffffffff to PCI_PREF_BASE_UPPER32. This may break
> transfers through related bridge at that time.
> 
> E.g. if we have below PCIe topology:
> 
> -[0000:00]-+-00.0-[01-02]--+-00.0  Device 8086:10fb
>            |               \-00.1  Device 8086:10fb
>            \-08.0-[03]----00.0  Device 8086:0953
> 
> When rescan 00:08.0, it will call:
> 
>         pci_rescan_bus
>                 -> pci_assign_unassigned_bus_resources
>                         -> __pci_bus_size_bridges
>                                 -> pci_bridge_check_ranges
>                                         -> pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, 0xffffffff)
> 
> This will change the prefetch range of 00:00.0 in a time slot, so traffic of
> 01:00.0 or 01:00.1 may be broken.
> 
> In fact, we can get if one bridge supports 64bit range by the bottom 4bits of
> prefetchable memory base/limit. Honestly speaking, I don't know why 1f82de10d6b1
> ("PCI/86: don't assume prefetchable ranges are 64bit") has added the double
> check code.
> 
> So Can we remove the double checking of prefetchable range to avoid this problem?
> 
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> ---
>  drivers/pci/setup-bus.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 958da7d..23010a9 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -778,20 +778,6 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>  			b_res[2].flags |= PCI_PREF_RANGE_TYPE_64;
>  		}
>  	}
> -
> -	/* double check if bridge does support 64 bit pref */
> -	if (b_res[2].flags & IORESOURCE_MEM_64) {
> -		u32 mem_base_hi, tmp;
> -		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
> -					 &mem_base_hi);
> -		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
> -					       0xffffffff);
> -		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp);
> -		if (!tmp)
> -			b_res[2].flags &= ~IORESOURCE_MEM_64;
> -		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
> -				       mem_base_hi);
> -	}
>  }
>  
>  /* Helper function for sizing routines: find first available
>

Any idea about this problem?

Best Regards，
Zhou
Yinghai Lu Oct. 2, 2017, 4:17 a.m. UTC | #2 
On Sat, Sep 23, 2017 at 03:24:42PM +0800, Zhou Wang wrote:
>                                         -> pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, 0xffffffff)
> 
> This will change the prefetch range of 00:00.0 in a time slot, so traffic of
> 01:00.0 or 01:00.1 may be broken.
> 
> In fact, we can get if one bridge supports 64bit range by the bottom 4bits of
> prefetchable memory base/limit. Honestly speaking, I don't know why 1f82de10d6b1
> ("PCI/86: don't assume prefetchable ranges are 64bit") has added the double
> check code.

some chip even that flags say that 64bit is support from that bits, but its upper 32 bits
actually can not be changed.

> 
> So Can we remove the double checking of prefetchable range to avoid this problem?
> 
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> ---
>  drivers/pci/setup-bus.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 958da7d..23010a9 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -778,20 +778,6 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>  			b_res[2].flags |= PCI_PREF_RANGE_TYPE_64;
>  		}
>  	}
> -
> -	/* double check if bridge does support 64 bit pref */
> -	if (b_res[2].flags & IORESOURCE_MEM_64) {
> -		u32 mem_base_hi, tmp;
> -		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
> -					 &mem_base_hi);
> -		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
> -					       0xffffffff);
> -		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp);
> -		if (!tmp)
> -			b_res[2].flags &= ~IORESOURCE_MEM_64;
> -		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
> -				       mem_base_hi);
> -	}
>  }
>  
>  /* Helper function for sizing routines: find first available

Maybe we can try this: only touch upper 32bits after we touched low 32bits ?

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 958da7db9033..2ac4d20e5c11 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -744,6 +744,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
 	u32 pmem;
 	struct pci_dev *bridge = bus->self;
 	struct resource *b_res;
+	int pref_memory_base_touched = 0;
 
 	b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
 	b_res[1].flags |= IORESOURCE_MEM;
@@ -769,6 +770,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
 					       0xffe0fff0);
 		pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem);
 		pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0);
+		pref_memory_base_touched = 1;
 	}
 	if (pmem) {
 		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH;
@@ -780,7 +782,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
 	}
 
 	/* double check if bridge does support 64 bit pref */
-	if (b_res[2].flags & IORESOURCE_MEM_64) {
+	if (pref_memory_base_touched && b_res[2].flags & IORESOURCE_MEM_64) {
 		u32 mem_base_hi, tmp;
 		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
 					 &mem_base_hi);
Bjorn Helgaas Oct. 2, 2017, 8:38 p.m. UTC | #3 
[+cc Lorenzo]

On Sun, Oct 01, 2017 at 09:17:01PM -0700, Yinghai Lu wrote:
> On Sat, Sep 23, 2017 at 03:24:42PM +0800, Zhou Wang wrote:
> >                                         -> pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, 0xffffffff)
> > 
> > This will change the prefetch range of 00:00.0 in a time slot, so
> > traffic of 01:00.0 or 01:00.1 may be broken.
> > 
> > In fact, we can get if one bridge supports 64bit range by the
> > bottom 4bits of prefetchable memory base/limit. Honestly speaking,
> > I don't know why 1f82de10d6b1 ("PCI/86: don't assume prefetchable
> > ranges are 64bit") has added the double check code.

I agree this is a problem.  We shouldn't be changing the window while
devices below the bridge are active.

> some chip even that flags say that 64bit is support from that bits,
> but its upper 32 bits actually can not be changed.

We should figure this out at enumeration-time, before we enable any
devices below the bridge.  Maybe it could be done in the
pci_setup_device() path.

This is sort of related to the pci_read_bridge_bases() path, which is
currently done by the arch-specific pcibios_fixup_bus(), even though
it really isn't arch-specific.  Somebody tried to fix that recently,
but I don't remember the issues.

I think it would be nice if pci_setup_device() could read bridge
window info the same way it currently reads BAR info.  Maybe it would
make dmesg more intelligible, too, e.g., we could print the bridge
info before we print info about devices below the bridge.

Bjorn

> > So Can we remove the double checking of prefetchable range to
> > avoid this problem?
> > 
> > Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> > ---
> >  drivers/pci/setup-bus.c | 14 --------------
> >  1 file changed, 14 deletions(-)
> > 
> > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> > index 958da7d..23010a9 100644
> > --- a/drivers/pci/setup-bus.c
> > +++ b/drivers/pci/setup-bus.c
> > @@ -778,20 +778,6 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
> >  			b_res[2].flags |= PCI_PREF_RANGE_TYPE_64;
> >  		}
> >  	}
> > -
> > -	/* double check if bridge does support 64 bit pref */
> > -	if (b_res[2].flags & IORESOURCE_MEM_64) {
> > -		u32 mem_base_hi, tmp;
> > -		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
> > -					 &mem_base_hi);
> > -		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
> > -					       0xffffffff);
> > -		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp);
> > -		if (!tmp)
> > -			b_res[2].flags &= ~IORESOURCE_MEM_64;
> > -		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
> > -				       mem_base_hi);
> > -	}
> >  }
> >  
> >  /* Helper function for sizing routines: find first available
> 
> Maybe we can try this: only touch upper 32bits after we touched low 32bits ?
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 958da7db9033..2ac4d20e5c11 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -744,6 +744,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>  	u32 pmem;
>  	struct pci_dev *bridge = bus->self;
>  	struct resource *b_res;
> +	int pref_memory_base_touched = 0;
>  
>  	b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
>  	b_res[1].flags |= IORESOURCE_MEM;
> @@ -769,6 +770,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>  					       0xffe0fff0);
>  		pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem);
>  		pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0);
> +		pref_memory_base_touched = 1;
>  	}
>  	if (pmem) {
>  		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH;
> @@ -780,7 +782,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>  	}
>  
>  	/* double check if bridge does support 64 bit pref */
> -	if (b_res[2].flags & IORESOURCE_MEM_64) {
> +	if (pref_memory_base_touched && b_res[2].flags & IORESOURCE_MEM_64) {
>  		u32 mem_base_hi, tmp;
>  		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
>  					 &mem_base_hi);
> 
> 
>
Zhou Wang Oct. 9, 2017, 10:40 a.m. UTC | #4 
On 2017/10/2 12:17, Yinghai Lu wrote:
> On Sat, Sep 23, 2017 at 03:24:42PM +0800, Zhou Wang wrote:
>>                                         -> pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, 0xffffffff)
>>
>> This will change the prefetch range of 00:00.0 in a time slot, so traffic of
>> 01:00.0 or 01:00.1 may be broken.
>>
>> In fact, we can get if one bridge supports 64bit range by the bottom 4bits of
>> prefetchable memory base/limit. Honestly speaking, I don't know why 1f82de10d6b1
>> ("PCI/86: don't assume prefetchable ranges are 64bit") has added the double
>> check code.
> 
> some chip even that flags say that 64bit is support from that bits, but its upper 32 bits
> actually can not be changed.
> 
>>
>> So Can we remove the double checking of prefetchable range to avoid this problem?
>>
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>> ---
>>  drivers/pci/setup-bus.c | 14 --------------
>>  1 file changed, 14 deletions(-)
>>
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index 958da7d..23010a9 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -778,20 +778,6 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>>  			b_res[2].flags |= PCI_PREF_RANGE_TYPE_64;
>>  		}
>>  	}
>> -
>> -	/* double check if bridge does support 64 bit pref */
>> -	if (b_res[2].flags & IORESOURCE_MEM_64) {
>> -		u32 mem_base_hi, tmp;
>> -		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
>> -					 &mem_base_hi);
>> -		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
>> -					       0xffffffff);
>> -		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp);
>> -		if (!tmp)
>> -			b_res[2].flags &= ~IORESOURCE_MEM_64;
>> -		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
>> -				       mem_base_hi);
>> -	}
>>  }
>>  
>>  /* Helper function for sizing routines: find first available
> 
> Maybe we can try this: only touch upper 32bits after we touched low 32bits ?
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 958da7db9033..2ac4d20e5c11 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -744,6 +744,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>  	u32 pmem;
>  	struct pci_dev *bridge = bus->self;
>  	struct resource *b_res;
> +	int pref_memory_base_touched = 0;
>  
>  	b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
>  	b_res[1].flags |= IORESOURCE_MEM;
> @@ -769,6 +770,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>  					       0xffe0fff0);
>  		pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem);
>  		pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0);
> +		pref_memory_base_touched = 1;
>  	}
>  	if (pmem) {
>  		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH;
> @@ -780,7 +782,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>  	}
>  
>  	/* double check if bridge does support 64 bit pref */
> -	if (b_res[2].flags & IORESOURCE_MEM_64) {
> +	if (pref_memory_base_touched && b_res[2].flags & IORESOURCE_MEM_64) {
>  		u32 mem_base_hi, tmp;
>  		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
>  					 &mem_base_hi);
>

I think this workaround can solve the problem I mentioned in commit message:

if we have below PCIe topology:

-[0000:00]-+-00.0-[01-02]--+-00.0  Device 8086:10fb
           |               \-00.1  Device 8086:10fb
           \-08.0-[03]----00.0  Device 8086:0953

When rescan 00:08.0, it will change the prefetch range of 00:00.0 in a time slot,
so traffic of 01:00.0 or 01:00.1 may be broken.

As when it runs pci_bridge_check_ranges for bus 1, PCI_PREF_MEMORY_BASE will not be
0, PCI_PREF_BASE_UPPER32 will not be double checked.

Thanks,
Zhou

> 
> 
> 
> .
>
Zhou Wang Oct. 10, 2017, 8:16 a.m. UTC | #5 
On 2017/10/3 4:38, Bjorn Helgaas wrote:
> [+cc Lorenzo]
> 
> On Sun, Oct 01, 2017 at 09:17:01PM -0700, Yinghai Lu wrote:
>> On Sat, Sep 23, 2017 at 03:24:42PM +0800, Zhou Wang wrote:
>>>                                         -> pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32, 0xffffffff)
>>>
>>> This will change the prefetch range of 00:00.0 in a time slot, so
>>> traffic of 01:00.0 or 01:00.1 may be broken.
>>>
>>> In fact, we can get if one bridge supports 64bit range by the
>>> bottom 4bits of prefetchable memory base/limit. Honestly speaking,
>>> I don't know why 1f82de10d6b1 ("PCI/86: don't assume prefetchable
>>> ranges are 64bit") has added the double check code.
> 
> I agree this is a problem.  We shouldn't be changing the window while
> devices below the bridge are active.
> 
>> some chip even that flags say that 64bit is support from that bits,
>> but its upper 32 bits actually can not be changed.
> 
> We should figure this out at enumeration-time, before we enable any
> devices below the bridge.  Maybe it could be done in the
> pci_setup_device() path.
> 
> This is sort of related to the pci_read_bridge_bases() path, which is
> currently done by the arch-specific pcibios_fixup_bus(), even though
> it really isn't arch-specific.  Somebody tried to fix that recently,
> but I don't remember the issues.
> 
> I think it would be nice if pci_setup_device() could read bridge
> window info the same way it currently reads BAR info.  Maybe it would
> make dmesg more intelligible, too, e.g., we could print the bridge
> info before we print info about devices below the bridge.

So can we move pci_bridge_check_ranges to pci_setup_device? just like:

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a6560c9..61e016d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -366,4 +366,6 @@ int acpi_get_rc_resources(struct device *dev, const char *hid, u16 segment,
 			  struct resource *res);
 #endif

+void pci_bridge_check_ranges(struct pci_dev *dev);
+
 #endif /* DRIVERS_PCI_H */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ff94b69..a5c25e0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1517,6 +1517,7 @@ int pci_setup_device(struct pci_dev *dev)
 		pci_read_irq(dev);
 		dev->transparent = ((dev->class & 0xff) == 1);
 		pci_read_bases(dev, 2, PCI_ROM_ADDRESS1);
+                pci_bridge_check_ranges(dev);
 		set_pcie_hotplug_bridge(dev);
 		pos = pci_find_capability(dev, PCI_CAP_ID_SSVID);
 		if (pos) {
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 958da7d..220e6c8 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -738,11 +738,10 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
 /* Check whether the bridge supports optional I/O and
    prefetchable memory ranges. If not, the respective
    base/limit registers must be read-only and read as 0. */
-static void pci_bridge_check_ranges(struct pci_bus *bus)
+void pci_bridge_check_ranges(struct pci_dev *bridge)
 {
 	u16 io;
 	u32 pmem;
-	struct pci_dev *bridge = bus->self;
 	struct resource *b_res;

 	b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
@@ -1248,7 +1247,6 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 		break;

 	case PCI_CLASS_BRIDGE_PCI:
-		pci_bridge_check_ranges(bus);
 		if (bus->self->is_hotplug_bridge) {
 			additional_io_size  = pci_hotplug_io_size;
 			additional_mem_size = pci_hotplug_mem_size;

Thanks,
Zhou

> 
> Bjorn
> 
>>> So Can we remove the double checking of prefetchable range to
>>> avoid this problem?
>>>
>>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>>> ---
>>>  drivers/pci/setup-bus.c | 14 --------------
>>>  1 file changed, 14 deletions(-)
>>>
>>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>>> index 958da7d..23010a9 100644
>>> --- a/drivers/pci/setup-bus.c
>>> +++ b/drivers/pci/setup-bus.c
>>> @@ -778,20 +778,6 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>>>  			b_res[2].flags |= PCI_PREF_RANGE_TYPE_64;
>>>  		}
>>>  	}
>>> -
>>> -	/* double check if bridge does support 64 bit pref */
>>> -	if (b_res[2].flags & IORESOURCE_MEM_64) {
>>> -		u32 mem_base_hi, tmp;
>>> -		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
>>> -					 &mem_base_hi);
>>> -		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
>>> -					       0xffffffff);
>>> -		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp);
>>> -		if (!tmp)
>>> -			b_res[2].flags &= ~IORESOURCE_MEM_64;
>>> -		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
>>> -				       mem_base_hi);
>>> -	}
>>>  }
>>>  
>>>  /* Helper function for sizing routines: find first available
>>
>> Maybe we can try this: only touch upper 32bits after we touched low 32bits ?
>>
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index 958da7db9033..2ac4d20e5c11 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -744,6 +744,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>>  	u32 pmem;
>>  	struct pci_dev *bridge = bus->self;
>>  	struct resource *b_res;
>> +	int pref_memory_base_touched = 0;
>>  
>>  	b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
>>  	b_res[1].flags |= IORESOURCE_MEM;
>> @@ -769,6 +770,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>>  					       0xffe0fff0);
>>  		pci_read_config_dword(bridge, PCI_PREF_MEMORY_BASE, &pmem);
>>  		pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, 0x0);
>> +		pref_memory_base_touched = 1;
>>  	}
>>  	if (pmem) {
>>  		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH;
>> @@ -780,7 +782,7 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
>>  	}
>>  
>>  	/* double check if bridge does support 64 bit pref */
>> -	if (b_res[2].flags & IORESOURCE_MEM_64) {
>> +	if (pref_memory_base_touched && b_res[2].flags & IORESOURCE_MEM_64) {
>>  		u32 mem_base_hi, tmp;
>>  		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
>>  					 &mem_base_hi);
>>
>>
>>
> 
> .
>
Yinghai Lu Oct. 17, 2017, 7 a.m. UTC | #6 
On Tue, Oct 10, 2017 at 1:16 AM, Zhou Wang <wangzhou1@hisilicon.com> wrote:
>
> So can we move pci_bridge_check_ranges to pci_setup_device? just like:

short answer is no.

As we have other user for __pci_bus_bridges, and before that the flags
could be cleared during resource allocation retries.

-Yinghai
diff mbox series

Patch

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 958da7d..23010a9 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -778,20 +778,6 @@  static void pci_bridge_check_ranges(struct pci_bus *bus)
 			b_res[2].flags |= PCI_PREF_RANGE_TYPE_64;
 		}
 	}
-
-	/* double check if bridge does support 64 bit pref */
-	if (b_res[2].flags & IORESOURCE_MEM_64) {
-		u32 mem_base_hi, tmp;
-		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
-					 &mem_base_hi);
-		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
-					       0xffffffff);
-		pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp);
-		if (!tmp)
-			b_res[2].flags &= ~IORESOURCE_MEM_64;
-		pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
-				       mem_base_hi);
-	}
 }
 
 /* Helper function for sizing routines: find first available