RE: [Patch] PCI: check szhi when sz is 0 for 64 bit pref mem

RE: [Patch] PCI: check szhi when sz is 0 for 64 bit pref mem

[Lu, Yinghai]

-----Original Message-----
From: Andrew Morton [mailto:akpm@osdl.org] 

>I've basically given up in exhaustion on that patch.  Maybe when I have
a
>burst of extra energy I'll go back and take the time to understand it,
>or maybe when Greg comes back he'll save me.

Please 
http://lkml.org/lkml/2006/11/6/341

that would be more clean.

Thanks

Yinghai Lu

Re: [Patch] PCI: check szhi when sz is 0 for 64 bit pref mem

[Greg KH]

On Wed, Nov 08, 2006 at 11:15:59AM -0800, Lu, Yinghai wrote:
> -----Original Message-----
> From: Andrew Morton [mailto:akpm@osdl.org] 
> 
> >I've basically given up in exhaustion on that patch.  Maybe when I have
> a
> >burst of extra energy I'll go back and take the time to understand it,
> >or maybe when Greg comes back he'll save me.
> 
> Please 
> http://lkml.org/lkml/2006/11/6/341
> 
> that would be more clean.

Can you just forward the proper version of this patch to me, as it seems
there are lots of different versions of this change floating around?

thanks,

greg k-h

RE: [Patch] PCI: check szhi when sz is 0 for 64 bit pref mem

[Lu, Yinghai]

-----Original Message-----
From: Andrew Morton [mailto:akpm@osdl.org] 
>I don't really understand what this patch does.
>We have a PCI device with a 64-bit BAR and the size is also 64-bit and
is
>larger than 4G, yes?

Yes

>But the code appears to already be attempting to handle such devices. 
>Confused.

The old code will 
Try to calculate the sz from lo 32 bit addr reg, and sz is 0 if the 64
bit resource size if 4G above, so it will continue can skip that
register, and it will go on try to treat the hi 32bit addr reg as
another 32 bit resource addr reg.

YH

Re: [Patch] PCI: check szhi when sz is 0 for 64 bit pref mem

[Andrew Morton]

On Mon, 6 Nov 2006 14:15:23 -0800
"Lu, Yinghai" <yinghai.lu@amd.com> wrote:

> -----Original Message-----
> From: Andrew Morton [mailto:akpm@osdl.org] 
> >I don't really understand what this patch does.
> >We have a PCI device with a 64-bit BAR and the size is also 64-bit and
> is
> >larger than 4G, yes?
> 
> Yes
> 
> >But the code appears to already be attempting to handle such devices. 
> >Confused.
> 
> The old code will 
> Try to calculate the sz from lo 32 bit addr reg, and sz is 0 if the 64
> bit resource size if 4G above, so it will continue can skip that
> register, and it will go on try to treat the hi 32bit addr reg as
> another 32 bit resource addr reg.
> 

OK...  I still don't know what a "pref" is though.

I reworked the path a bit, as below.  Look OK?


From: "Yinghai Lu" <yinghai.lu@amd.com>

If the PCI device is 64-bit memory and has a size of 0xnnnnnnnn00000000 then
pci_read_bases() will incorrectly assume that it has a size of zero.

Cc: Myles Watson <myles@mouselemur.cs.byu.edu>
Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>
Cc: Greg KH <greg@kroah.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 drivers/pci/probe.c |   23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff -puN drivers/pci/probe.c~pci-check-szhi-when-sz-is-0-for-64-bit-pref-mem drivers/pci/probe.c
--- a/drivers/pci/probe.c~pci-check-szhi-when-sz-is-0-for-64-bit-pref-mem
+++ a/drivers/pci/probe.c
@@ -144,6 +144,14 @@ static u32 pci_size(u32 base, u32 maxbas
 	return size;
 }
 
+static inline bool is_64_bit_memory(u32 v)
+{
+	if ((v & (PCI_BASE_ADDRESS_SPACE|PCI_BASE_ADDRESS_MEM_TYPE_MASK)) ==
+	    (PCI_BASE_ADDRESS_SPACE_MEMORY|PCI_BASE_ADDRESS_MEM_TYPE_64))
+		return true;
+	return false;
+}
+
 static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
 {
 	unsigned int pos, reg, next;
@@ -165,7 +173,11 @@ static void pci_read_bases(struct pci_de
 			l = 0;
 		if ((l & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_MEMORY) {
 			sz = pci_size(l, sz, (u32)PCI_BASE_ADDRESS_MEM_MASK);
-			if (!sz)
+			/*
+			 * For a 64bit BAR, sz could be 0 if the real size is
+			 * bigger than 4G so we need to check szhi for that.
+			 */
+			if (!is_64_bit_memory(l) && !sz)
 				continue;
 			res->start = l & PCI_BASE_ADDRESS_MEM_MASK;
 			res->flags |= l & ~PCI_BASE_ADDRESS_MEM_MASK;
@@ -178,8 +190,7 @@ static void pci_read_bases(struct pci_de
 		}
 		res->end = res->start + (unsigned long) sz;
 		res->flags |= pci_calc_resource_flags(l);
-		if ((l & (PCI_BASE_ADDRESS_SPACE | PCI_BASE_ADDRESS_MEM_TYPE_MASK))
-		    == (PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64)) {
+		if (is_64_bit_memory(l)) {
 			u32 szhi, lhi;
 			pci_read_config_dword(dev, reg+4, &lhi);
 			pci_write_config_dword(dev, reg+4, ~0);
@@ -188,6 +199,12 @@ static void pci_read_bases(struct pci_de
 			szhi = pci_size(lhi, szhi, 0xffffffff);
 			next++;
 #if BITS_PER_LONG == 64
+			if (!sz && !szhi) {
+				res->start = 0;
+				res->end = 0;
+				res->flags = 0;
+				continue;
+			}
 			res->start |= ((unsigned long) lhi) << 32;
 			res->end = res->start + sz;
 			if (szhi) {
_

Re: [Patch] PCI: check szhi when sz is 0 for 64 bit pref mem

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 38 bytes --]

please check version with pci_size64.

[-- Attachment #2: pci_64bit_pref_4g_1106.patch --]
[-- Type: text/x-patch, Size: 3056 bytes --]

[PATCH] PCI: check szhi when sz is 0 when 64 bit iomem bigger than 4G

	If the PCI device is 64-bit memory and has a size of 0xnnnnnnnn00000000 then
	pci_read_bases() will incorrectly assume that it has a size of zero.

	Cc: Myles Watson <myles@mouselemur.cs.byu.edu>
	Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>
	Cc: Greg KH <greg@kroah.com>
	Signed-off-by: Andrew Morton <akpm@osdl.org>

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e159d66..0e2b10c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -144,6 +144,24 @@ static u32 pci_size(u32 base, u32 maxbas
 	return size;
 }
 
+static u64 pci_size64(u64 base, u64 maxbase, u64 mask)
+{
+	u64 size = mask & maxbase;	/* Find the significant bits */
+	if (!size)
+		return 0;
+
+	/* Get the lowest of them to find the decode size, and
+	   from that the extent.  */
+	size = (size & ~(size-1)) - 1;
+
+	/* base == maxbase can be valid only if the BAR has
+	   already been programmed with all 1s.  */
+	if (base == maxbase && ((base | size) & mask) != mask)
+		return 0;
+
+	return size;
+}
+
 static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
 {
 	unsigned int pos, reg, next;
@@ -151,6 +169,7 @@ static void pci_read_bases(struct pci_de
 	struct resource *res;
 
 	for(pos=0; pos<howmany; pos = next) {
+		u64 l64, sz64;
 		next = pos+1;
 		res = &dev->resource[pos];
 		res->name = pci_name(dev);
@@ -164,9 +183,15 @@ static void pci_read_bases(struct pci_de
 		if (l == 0xffffffff)
 			l = 0;
 		if ((l & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_MEMORY) {
+			sz64 = sz;
 			sz = pci_size(l, sz, (u32)PCI_BASE_ADDRESS_MEM_MASK);
-			if (!sz)
-				continue;
+			/* for 64bit pref, sz could be 0, if the real size is bigger than 4G,
+				so need to check szhi for it
+			 */
+			if ((l & (PCI_BASE_ADDRESS_SPACE | PCI_BASE_ADDRESS_MEM_TYPE_MASK))
+			    != (PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64)) 
+				if (!sz)
+					continue;
 			res->start = l & PCI_BASE_ADDRESS_MEM_MASK;
 			res->flags |= l & ~PCI_BASE_ADDRESS_MEM_MASK;
 		} else {
@@ -185,17 +210,21 @@ static void pci_read_bases(struct pci_de
 			pci_write_config_dword(dev, reg+4, ~0);
 			pci_read_config_dword(dev, reg+4, &szhi);
 			pci_write_config_dword(dev, reg+4, lhi);
-			szhi = pci_size(lhi, szhi, 0xffffffff);
+			sz64 |= ((unsigned long) szhi) << 32;
+			l64 = (((unsigned long) lhi) << 32) | l;
+			sz64 = pci_size64(l64, sz64, PCI_BASE_ADDRESS_MEM_MASK);
 			next++;
 #if BITS_PER_LONG == 64
-			res->start |= ((unsigned long) lhi) << 32;
-			res->end = res->start + sz;
-			if (szhi) {
-				/* This BAR needs > 4GB?  Wow. */
-				res->end |= (unsigned long)szhi<<32;
+			if (!sz64) {
+				res->start = 0;
+				res->end = 0;
+				res->flags = 0;
+				continue;
 			}
+			res->start = l64 & PCI_BASE_ADDRESS_MEM_MASK;
+			res->end = res->start + sz64;
 #else
-			if (szhi) {
+			if (sz64>0x100000000ULL) {
 				printk(KERN_ERR "PCI: Unable to handle 64-bit BAR for device %s\n", pci_name(dev));
 				res->start = 0;
 				res->flags = 0;

[Patch] PCI: check szhi when sz is 0 for 64 bit pref mem

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 24 bytes --]

Please check the patch.

[-- Attachment #2: pci_64bit_pref_4g.patch --]
[-- Type: text/x-patch, Size: 1564 bytes --]

[Patch] PCI: check szhi when sz is 0 for 64 bit pref mem

	For co-prcessor with mem installed, the ram will be treated to pref mem.	
	Under 64bit kernel, when 64bit pref mem size is above 4G, sz from pci_size in low bits, will get 0, 
	at this point, we need to check szhi too. Otherwise the pre-set value by firmware can not be read 
	to resrource struct, it will skip that resource, and try to hi 32 bit as another 32bit resource.

	Cc: Myles Watson <myles@mouselemur.cs.byu.edu>
	Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>	


--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -165,8 +165,13 @@ static void pci_read_bases(struct pci_de
 			l = 0;
 		if ((l & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_MEMORY) {
 			sz = pci_size(l, sz, (u32)PCI_BASE_ADDRESS_MEM_MASK);
-			if (!sz)
-				continue;
+			/* for 64bit pref, sz could be 0, if the real size is bigger than 4G,
+				so need to check szhi for it
+			 */
+			if ((l & (PCI_BASE_ADDRESS_SPACE | PCI_BASE_ADDRESS_MEM_TYPE_MASK))
+			    != (PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64)) 
+				if (!sz)
+					continue;
 			res->start = l & PCI_BASE_ADDRESS_MEM_MASK;
 			res->flags |= l & ~PCI_BASE_ADDRESS_MEM_MASK;
 		} else {
@@ -188,6 +193,12 @@ static void pci_read_bases(struct pci_de
 			szhi = pci_size(lhi, szhi, 0xffffffff);
 			next++;
 #if BITS_PER_LONG == 64
+			if( !sz && !szhi) {
+				res->start = 0;
+				res->end = 0;
+				res->flags = 0;
+				continue;
+			}
 			res->start |= ((unsigned long) lhi) << 32;
 			res->end = res->start + sz;
 			if (szhi) {

Re: [Patch] PCI: check szhi when sz is 0 for 64 bit pref mem

[Andrew Morton]

On Fri, 3 Nov 2006 21:27:35 -0800
"Yinghai Lu" <yinghai.lu@amd.com> wrote:

> 	For co-prcessor with mem installed, the ram will be treated to pref mem.	

What is "pref mem"?

> 	Under 64bit kernel, when 64bit pref mem size is above 4G, sz from pci_size in low bits, will get 0, 
> 	at this point, we need to check szhi too. Otherwise the pre-set value by firmware can not be read 
> 	to resrource struct, it will skip that resource, and try to hi 32 bit as another 32bit resource.
> 
> 	Cc: Myles Watson <myles@mouselemur.cs.byu.edu>
> 	Signed-off-by: Yinghai Lu <yinghai.lu@amd.com>	
> 
> 
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -165,8 +165,13 @@ static void pci_read_bases(struct pci_de
>  			l = 0;
>  		if ((l & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_MEMORY) {
>  			sz = pci_size(l, sz, (u32)PCI_BASE_ADDRESS_MEM_MASK);
> -			if (!sz)
> -				continue;
> +			/* for 64bit pref, sz could be 0, if the real size is bigger than 4G,
> +				so need to check szhi for it
> +			 */
> +			if ((l & (PCI_BASE_ADDRESS_SPACE | PCI_BASE_ADDRESS_MEM_TYPE_MASK))
> +			    != (PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64)) 
> +				if (!sz)
> +					continue;
>  			res->start = l & PCI_BASE_ADDRESS_MEM_MASK;
>  			res->flags |= l & ~PCI_BASE_ADDRESS_MEM_MASK;
>  		} else {
> @@ -188,6 +193,12 @@ static void pci_read_bases(struct pci_de
>  			szhi = pci_size(lhi, szhi, 0xffffffff);
>  			next++;
>  #if BITS_PER_LONG == 64
> +			if( !sz && !szhi) {
> +				res->start = 0;
> +				res->end = 0;
> +				res->flags = 0;
> +				continue;
> +			}
>  			res->start |= ((unsigned long) lhi) << 32;
>  			res->end = res->start + sz;
>  			if (szhi) {

I don't really understand what this patch does.

We have a PCI device with a 64-bit BAR and the size is also 64-bit and is
larger than 4G, yes?

But the code appears to already be attempting to handle such devices. 
Confused.

Re: [Patch] PCI: check szhi when sz is 0 for 64 bit pref mem

[Eric W. Biederman]

Andrew Morton <akpm@osdl.org> writes:

> On Fri, 3 Nov 2006 21:27:35 -0800
> "Yinghai Lu" <yinghai.lu@amd.com> wrote:
>
>> For co-prcessor with mem installed, the ram will be treated to pref mem.
>
> What is "pref mem"?

Memory mapped base address registers can be either normal or for prefetchable
sections of memory mapped I/O.  Frequently all prefetchable bars are 64bit.
The prefetchable bars are also frequently ask for the largest amounts of
memory.  So it is easy and worthwhile to place all prefetchable bars about 4G.

The "pref mem" short hand comes from a the LinuxBIOS print statements that
report every bar value and what kind of bar it is, during boot up.

Eric

Re: [Patch] PCI: check szhi when sz is 0 for 64 bit pref mem

[Andrew Morton]

On Wed, 08 Nov 2006 11:19:40 -0700
ebiederm@xmission.com (Eric W. Biederman) wrote:

> Andrew Morton <akpm@osdl.org> writes:
> 
> > On Fri, 3 Nov 2006 21:27:35 -0800
> > "Yinghai Lu" <yinghai.lu@amd.com> wrote:
> >
> >> For co-prcessor with mem installed, the ram will be treated to pref mem.
> >
> > What is "pref mem"?
> 
> Memory mapped base address registers can be either normal or for prefetchable
> sections of memory mapped I/O.  Frequently all prefetchable bars are 64bit.
> The prefetchable bars are also frequently ask for the largest amounts of
> memory.  So it is easy and worthwhile to place all prefetchable bars about 4G.
> 
> The "pref mem" short hand comes from a the LinuxBIOS print statements that
> report every bar value and what kind of bar it is, during boot up.
> 

Ah.  Prefetchable.  Thanks.

I've basically given up in exhaustion on that patch.  Maybe when I have a
burst of extra energy I'll go back and take the time to understand it,
or maybe when Greg comes back he'll save me.