Skip to contentSkip to conversation

PROBLEM: memory corruption with i815 chipset variant

PATCH In review Nicolas Aspert 2 participants 11 messages May 27, 2002 – May 30, 2002 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw
Quoted 3 lines · Alan Cox

That means its actually using the same GART code as the 440BX and friends if I remember rightly (the i815 special stuff is for on board video)

Alessandro reported the problem to me also. I went through the i815 specs and found 2 'strange' things (maybe they are not but...)
1) No 'ERRSTS' register (well... a bus that does no error should be a feature ;-)
2) The ATTBASE register to which the *_configure functions write is different from other Intel chipsets. In the i815, the ATT base adress should be written between bits 12 and 28, whereas in all other Intel chipsets, it should be written between bits 12 and 31 (don't ask me why Intel feels like changing the adresses/specs for registers at each new chipsets....) . Alan, do you think this could cause all those troubles ?

Quoted 7 lines
Quoted 3 lines

Without agpgart module, kernel seems stable. A naive (totally naive, I admit it) interpretation suggests a problem in setting the AGP aperture.

Does the ram survive memtest86 overnight with no errors logged if you boot memtest86 and just leave it ?

From what Alessandro reported, it seems clear that the 'insmod agpgart' triggers the mayhem, including memtest failures.

Best regards

Nicolas.

Signature · 2 lines
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)
#2 lore raw
Quoted 11 lines · Nicolas Aspert

Alessandro reported the problem to me also. I went through the i815 specs and found 2 'strange' things (maybe they are not but...)
1) No 'ERRSTS' register (well... a bus that does no error should be a feature ;-)
2) The ATTBASE register to which the *_configure functions write is different from other Intel chipsets. In the i815, the ATT base adress should be written between bits 12 and 28, whereas in all other Intel chipsets, it should be written between bits 12 and 31 (don't ask me why Intel feels like changing the adresses/specs for registers at each new chipsets....) .
Alan, do you think this could cause all those troubles ?

It certainly could be. If bits 29-31 maybe control things like memory timings then it could do quite horrible things. Fixing it to leave the ERRSTS register alone and keep bits 29-31 is definitely worth trying. If that fixes it then its going to be easy enough to drop a fix into the mainstream code

Alan

#3 lore raw
In reply to a message not in the archive
Quoted 7 lines · Alan Cox

It certainly could be. If bits 29-31 maybe control things like memory timings then it could do quite horrible things. Fixing it to leave the ERRSTS register alone and keep bits 29-31 is definitely worth trying. If that fixes it then its going to be easy enough to drop a fix into the mainstream code

OK, I have a patch almost ready to do that except, I am not sure about what to do for those 3 bits...

The *usual* call is :

	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
			       agp_bridge.gatt_bus_addr);

Where 'gatt_bus_addr' is returned from a 'virt_to_phys' on 'gatt_table_real'.

Should I mask those three bits out when writing or write
'gatt_bus_addr >> 3' instead ? I am not too sure about the assumptions that can be made about what returns 'virt_to_phys' ...

Thanks in advance.

Nicolas.

Signature · 2 lines
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)
#4 lore raw
Quoted 7 lines · Nicolas Aspert

OK, I have a patch almost ready to do that except, I am not sure about what to do for those 3 bits...

The *usual* call is :

	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE,
			       agp_bridge.gatt_bus_addr);
Quoted 6 lines

Where 'gatt_bus_addr' is returned from a 'virt_to_phys' on 'gatt_table_real'.

Should I mask those three bits out when writing or write 'gatt_bus_addr >> 3' instead ? I am not too sure about the assumptions that can be made about what returns 'virt_to_phys' ...

virt_to_phys returns you the CPU physical (after the MMU) address of the memory in question. You'd want to check the documentation but assuming the base is still written as an address in bytes then you'd want to do something like

	if(agp_bridge.gatt_bus_addr & ~0x1FFFFFFF)
		panic("gatt bus addr too high");
	pci_read_config_dword(agp_bridge.dev, INTEL_ATTBASE, &addr);
	addr&=~0x1FFFFFFF;
	addr|=agp_bridge.gatt_bus_addr;
	pci_write_config_dword(agp_bridge.dev, INTEL_ATTBASE, &addr);
#5 lore raw

PATCH 1/1 Tentative fix for mem. corruption caused by intel 815 AGP

In reply to a message not in the archive

Hi all

Thanks Alan's advice, here is a patch that aims at fixing the mem. corruption problems encountered by some users, when trying to use agpgart with intel 815 chipsets. The patch is against 2.4.19-pre8-ac5

Alessandro, could you please test whether this patch improves things or not ?

Best regards

Signature · 2 lines
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)

Only in agp: Makefile

agp.h +4 -0
diff -u agp.orig/agp.h agp/agp.h
@@ -293,6 +293,10 @@
 /* This one is for I830MP w. an external graphic card */
 #define INTEL_I830_ERRSTS          0x92
 
+/* intel 815 register */
+#define INTEL_815_APCONT        0x51
+#define INTEL_815_ATTBASE_MASK  0x1FFFFFFF
+
 /* intel i820 registers */
 #define INTEL_I820_RDCR     0x51
 #define INTEL_I820_ERRSTS   0xc8
agpgart_be.c +74 -1

100 lines not on this page · raw message

Only in agp: agpgart_fe.c

#6 lore raw

Re: [PATCH,CFT] Tentative fix for mem. corruption caused by intel 815 AGP

On Mon, 2002-05-27 at 10:32, Nicolas Aspert wrote:
p to */

agpgart_be.c line 1514

Quoted 9 lines
agpgart_be.c +9 -0
+	pci_read_config_dword(agp_bridge.dev, INTEL_APBASE, &temp);
+	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
+
+	/* attbase - aperture base */
+        /* the Intel 815 chipset spec. says that bits 29-31 in the
+         * ATTBASE register are reserved -> try not to write them */
+        if (agp_bridge.gatt_bus_addr & (~ INTEL_815_ATTBASE_MASK))
+		panic("gatt bus addr too high");
+	addr = agp_bridge.gatt_bus_addr & INTEL_815_ATTBASE_MASK;

You need to add + temp&~INTEL_815_ATTBASE_MASK ..

#7 lore raw

Re: [PATCH,CFT] Tentative fix for mem. corruption caused by intel 815 AGP

In reply to a message not in the archive
Quoted 17 lines · Alan Cox

On Mon, 2002-05-27 at 10:32, Nicolas Aspert wrote:
p to */

Review

Quoted 9 lines
(quoted diff) +9 -0
+	pci_read_config_dword(agp_bridge.dev, INTEL_APBASE, &temp);
+	agp_bridge.gart_bus_addr = (temp & PCI_BASE_ADDRESS_MEM_MASK);
+
+	/* attbase - aperture base */
+        /* the Intel 815 chipset spec. says that bits 29-31 in the
+         * ATTBASE register are reserved -> try not to write them */
+        if (agp_bridge.gatt_bus_addr & (~ INTEL_815_ATTBASE_MASK))
+		panic("gatt bus addr too high");
+	addr = agp_bridge.gatt_bus_addr & INTEL_815_ATTBASE_MASK;

You need to add + temp&~INTEL_815_ATTBASE_MASK ..

I am not sure to understand... Do you really mean mixing 'APBASE' which is the AGP base aperture adress along with the *gatt* which is the translation table adress ? If yes, I think I need a supplementary explanation...

Best regards.

Signature · 2 lines
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)
#8 lore raw

Re: [PATCH,CFT] Tentative fix for mem. corruption caused by intel 815 AGP

In reply to a message not in the archive
Quoted 4 lines · Alan Cox

Your code isnt reading the top bits from the register and anding them back into it with the address.

OK I got it now... I was confused by the 'temp' thing but I should have read more carefully your initial suggestion (doing 2 things at the same time seems beyond my possibilities today ;-) Anyway, here is take 2 of the patch, hopefully correct this time...

Just another quick thought... in all intel chipsets datasheets, the bits 0-11 of the ATTBASE register are also marked as 'reserved'. So far, all the intel_*_configure routines are writing shamelessly on these bits. Shouldn't we mask those bits out too (though it seems this has not caused any trouble so far) ?

Best regards

Signature · 2 lines
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)
agp.h +4 -0
diff -u agp.orig/agp.h agp/agp.h
@@ -293,6 +293,10 @@
 /* This one is for I830MP w. an external graphic card */
 #define INTEL_I830_ERRSTS          0x92
 
+/* intel 815 register */
+#define INTEL_815_APCONT        0x51
+#define INTEL_815_ATTBASE_MASK  ~0x1FFFFFFF
+
 /* intel i820 registers */
 #define INTEL_I820_RDCR     0x51
 #define INTEL_I820_ERRSTS   0xc8
agpgart_be.c +76 -1

102 lines not on this page · raw message

#9 lore raw

Re: [PATCH,CFT] Tentative fix for mem. corruption caused by intel 815 AGP

Quoted 5 lines · Nicolas Aspert

Just another quick thought... in all intel chipsets datasheets, the bits 0-11 of the ATTBASE register are also marked as 'reserved'. So far, all the intel_*_configure routines are writing shamelessly on these bits. Shouldn't we mask those bits out too (though it seems this has not caused any trouble so far) ?

We ought to mask and copy the original yes. The number of times we've had Linux driver breakages by not masking and avoiding writes to reserved bits is small but it does happen occasionally.

Alan

#10 lore raw

PATCH 1/1 Tentative fix for agpgart (writing on 'reserved' bits)

In reply to a message not in the archive
Quoted 4 lines · Alan Cox

We ought to mask and copy the original yes. The number of times we've had Linux driver breakages by not masking and avoiding writes to reserved bits is small but it does happen occasionally.

That sounds reasonable. I attached a patch that should do this. According to what was done before (i.e. writing on the reserved bits), the 12 LSB of ATTBASE should be discarded... The patch is against 2.4.19-pre8-ac5

Please test

Best regards

Nicolas.

Signature · 2 lines
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)
agp.h +7 -0
diff -u agp.orig/agp.h agp/agp.h
@@ -274,6 +274,9 @@
 #define INTEL_NBXCFG    0x50
 #define INTEL_ERRSTS    0x91
 
+/* mask to preserve the 12 LSB of the ATTBASE register */
+#define INTEL_ATTBASE_MASK 0x00000FFF
+
 /* intel i830 registers */
 #define I830_GMCH_CTRL             0x52
 #define I830_GMCH_ENABLED          0x4
@@ -293,6 +296,10 @@
 /* This one is for I830MP w. an external graphic card */
 #define INTEL_I830_ERRSTS          0x92
 
+/* intel 815 register */
+#define INTEL_815_APCONT        0x51
+#define INTEL_815_ATTBASE_MASK  0xE0000FFF
+
 /* intel i820 registers */
 #define INTEL_I820_RDCR     0x51
 #define INTEL_I820_ERRSTS   0xc8
agpgart_be.c +108 -15

202 lines not on this page · raw message

#11 lore raw

Re: [PATCH,CFT] Tentative fix for agpgart (writing on 'reserved' bits)

In reply to a message not in the archive
Quoted 4 lines · Alessandro Morelli

Hello
I managed to get rid of customers, in order to test the patches.... Bad news...no change...

At least it is not worse (I hope ...) ;-)

Quoted 1 line

I'm starting to believe I'm mad...my PC works like charm without AGP and gets all mixed when I enabled it...

Does it start messing right after you enable it, or only after you write to it (starting X...) ?

Quoted 6 lines

What kind of information can I provide (/proc readings, tests, etc.) to shed some light?

Could this be a case of interaction between AGP and VM?

tlb-flush weirdness?

Apart from this kinda usual write-on-reserved-bits in the AGP initialization, which has been done before on various chipsets and almost never caused any trouble, I saw no blatant errors that may lead to such things... In fact, we have several PCs with i815 chipsets in the lab. People are using AGP without any troubles. The only difference is that we run an old kernel version (2.4.3-13, thanks the sysadmin for never upgrading....), and we have a different graphic card (Matrox).

Alan, do you have any idea about this one ?

Best regards

Signature · 2 lines
Nicolas Aspert      Signal Processing Institute (ITS)
Swiss Federal Institute of Technology (EPFL)