All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] PCI: Fix prefetchable range broken in pci_bridge_check_ranges
@ 2017-09-23  7:24 Zhou Wang
  2017-09-30  1:15 ` Zhou Wang
  2017-10-02  4:17 ` Yinghai Lu
  0 siblings, 2 replies; 7+ messages in thread
From: Zhou Wang @ 2017-09-23  7:24 UTC (permalink / raw)
  To: Bjorn Helgaas, yinghai; +Cc: linux-pci, Zhou Wang

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
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] PCI: Fix prefetchable range broken in pci_bridge_check_ranges
  2017-09-23  7:24 [RFC PATCH] PCI: Fix prefetchable range broken in pci_bridge_check_ranges Zhou Wang
@ 2017-09-30  1:15 ` Zhou Wang
  2017-10-02  4:17 ` Yinghai Lu
  1 sibling, 0 replies; 7+ messages in thread
From: Zhou Wang @ 2017-09-30  1:15 UTC (permalink / raw)
  To: Bjorn Helgaas, yinghai; +Cc: linux-pci

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] PCI: Fix prefetchable range broken in pci_bridge_check_ranges
  2017-09-23  7:24 [RFC PATCH] PCI: Fix prefetchable range broken in pci_bridge_check_ranges Zhou Wang
  2017-09-30  1:15 ` Zhou Wang
@ 2017-10-02  4:17 ` Yinghai Lu
  2017-10-02 20:38   ` Bjorn Helgaas
  2017-10-09 10:40   ` Zhou Wang
  1 sibling, 2 replies; 7+ messages in thread
From: Yinghai Lu @ 2017-10-02  4:17 UTC (permalink / raw)
  To: Zhou Wang; +Cc: Bjorn Helgaas, linux-pci

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);

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] PCI: Fix prefetchable range broken in pci_bridge_check_ranges
  2017-10-02  4:17 ` Yinghai Lu
@ 2017-10-02 20:38   ` Bjorn Helgaas
  2017-10-10  8:16     ` Zhou Wang
  2017-10-09 10:40   ` Zhou Wang
  1 sibling, 1 reply; 7+ messages in thread
From: Bjorn Helgaas @ 2017-10-02 20:38 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Zhou Wang, Bjorn Helgaas, linux-pci, Lorenzo Pieralisi

[+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);
> 
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] PCI: Fix prefetchable range broken in pci_bridge_check_ranges
  2017-10-02  4:17 ` Yinghai Lu
  2017-10-02 20:38   ` Bjorn Helgaas
@ 2017-10-09 10:40   ` Zhou Wang
  1 sibling, 0 replies; 7+ messages in thread
From: Zhou Wang @ 2017-10-09 10:40 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Bjorn Helgaas, linux-pci

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

> 
> 
> 
> .
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] PCI: Fix prefetchable range broken in pci_bridge_check_ranges
  2017-10-02 20:38   ` Bjorn Helgaas
@ 2017-10-10  8:16     ` Zhou Wang
  2017-10-17  7:00       ` Yinghai Lu
  0 siblings, 1 reply; 7+ messages in thread
From: Zhou Wang @ 2017-10-10  8:16 UTC (permalink / raw)
  To: Bjorn Helgaas, Yinghai Lu; +Cc: Bjorn Helgaas, linux-pci, Lorenzo Pieralisi

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);
>>
>>
>>
> 
> .
> 

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH] PCI: Fix prefetchable range broken in pci_bridge_check_ranges
  2017-10-10  8:16     ` Zhou Wang
@ 2017-10-17  7:00       ` Yinghai Lu
  0 siblings, 0 replies; 7+ messages in thread
From: Yinghai Lu @ 2017-10-17  7:00 UTC (permalink / raw)
  To: Zhou Wang; +Cc: Bjorn Helgaas, Bjorn Helgaas, linux-pci, Lorenzo Pieralisi

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-10-17  7:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-23  7:24 [RFC PATCH] PCI: Fix prefetchable range broken in pci_bridge_check_ranges Zhou Wang
2017-09-30  1:15 ` Zhou Wang
2017-10-02  4:17 ` Yinghai Lu
2017-10-02 20:38   ` Bjorn Helgaas
2017-10-10  8:16     ` Zhou Wang
2017-10-17  7:00       ` Yinghai Lu
2017-10-09 10:40   ` Zhou Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.