Skip to contentSkip to conversation

kdb getting healthier? (PCI guru needed, that's you willy!)

parisc-linux Carlos O'Donell Jr. 3 participants 11 messages Jun 6, 2002 – Jun 7, 2002

lore
Share & tools
lore
#0 lore raw

Randolph,

I think I was able to get kdb to stop dying inside unwind_table_init.

unwind.c Thu Jun 6 08:51:49 2002 renamed from unwind.c.orig Thu Jun 6 08:51:02 2002 +1 -1
@@ -106,7 +106,7 @@
        table->base_addr = base_addr;
        table->gp = gp;
        table->start = base_addr + start[0].region_start;
-       table->end = base_addr + end[-1].region_end;
+       table->end = base_addr + end[0].region_end - sizeof(struct unwind_table_entry);
        table->table = (struct unwind_table_entry *)table_start;
        table->length = end - start;
        table->next = NULL;

Is this a gcc-ism we're seeing?
Note-to-self: Look at .S produced from compiling unwind.c

On another note, we have no code that calls unwind_table_add so I've #if'd it out. Which makes me wonder... which change was the fix? Time to go back to square one and try them out independantly.

The kernel still goes into an infinite loop of unknown origin:
---
PCI or EISA Bus Adapter 0 is not registered...
---
(see ./arch/parisc/kernel/pci.c' -> PCI_PORT_IN) Don't know where this gets called from, or how the ugly macro expands :)

Sadly I think I zapped my VGA out on the 712/60 (A common occurence?). It no longer produces _any_ video signals.
I may have to drag a 715/50 back to my house...

c.

#1 lore raw
Quoted 7 lines · Carlos O'Donell Jr.

The kernel still goes into an infinite loop of unknown origin:
---
PCI or EISA Bus Adapter 0 is not registered...
---
(see ./arch/parisc/kernel/pci.c' -> PCI_PORT_IN) Don't know where this gets called from, or how the ugly macro expands :)

that ugly macro expands into inb/inw/inl. that printk probably needs to go away -- i don't think it's helpful. maybe it should stay for adapter != 0, but so many drivers probe ISA space for their devices that we're going to keep getting them.

i bet the loop isn't infinite... probably driver probing. If you turn off CONFIG_PCI, the driver shouldn't be compiled in.

Quoted 11 lines

Sadly I think I zapped my VGA out on the 712/60 (A common occurence?). It no longer produces _any_ video signals.
I may have to drag a 715/50 back to my house...

c.

_______________________________________________
parisc-linux mailing list
[email protected]
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Signature · 1 line
Revolutions do not require corporate support.
#2 lore raw
Quoted 9 lines

that ugly macro expands into inb/inw/inl. that printk probably needs to go away -- i don't think it's helpful. maybe it should stay for adapter != 0, but so many drivers probe ISA space for their devices that we're going to keep getting them.

i bet the loop isn't infinite... probably driver probing. If you turn off CONFIG_PCI, the driver shouldn't be compiled in.

Ah, excellent. I think I understand how that works. Sadly, I'm not so sure that's the case, since it prints:
---
PCI or EISA Bus Adapter 0 is not registered. in8(0x64) returning -1; " in8(0x60) returning -1;
---
This message is not an exact copy since my notes are at home.

This printk alternates, dare I say, forever? (I'll leave it running for a few hours and see what happens).

I'll try and figure out what, if any, driver is broken. Most importantly I'll disable PCI/EISA support for now and see how far the system boots.

Thanks W!

c.

#3 lore raw
In reply to Carlos O'Donell Jr.
Quoted 8 lines
unwind.c Thu Jun 6 08:51:49 2002 renamed from unwind.c.orig Thu Jun 6 08:51:02 2002 +1 -1
-       table->end = base_addr + end[-1].region_end;
+       table->end = base_addr + end[0].region_end - sizeof(struct unwind_table_entry);
        table->table = (struct unwind_table_entry *)table_start;
        table->length = end - start;
        table->next = NULL;

Is this a gcc-ism we're seeing?
Note-to-self: Look at .S produced from compiling unwind.c

It's definitely dangerous to use array indexes outside the bounds of the array, particularly on the PA. There can be problems with space register selection, although I wouldn't expect this to be a in the linux kernel. You can avoid index insns with "-mdisable-indexing". Let me know if you see anything strange in the assembler output.

Dave

Signature · 2 lines
J. David Anglin                                  [email protected]
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)
#4 lore raw
Quoted 7 lines

unwind.c Thu Jun 6 08:51:49 2002 line 112

Quoted 6 lines
unwind.c Thu Jun 6 08:51:49 2002 renamed from unwind.c.orig Thu Jun 6 08:51:02 2002 +1 -1
-       table->end = base_addr + end[-1].region_end;
 
+       table->end = base_addr + end[0].region_end - sizeof(struct unwind_table_entry);
        table->table = (struct unwind_table_entry *)table_start;
        table->length = end - start;
        table->next = NULL;
Quoted 7 lines

It's definitely dangerous to use array indexes outside the bounds of the array, particularly on the PA. There can be problems with space register selection, although I wouldn't expect this to be a in the linux kernel. You can avoid index insns with "-mdisable-indexing". Let me know if you see anything strange in the assembler output.

Dave

That's what I imagined, but only now do I realize that the fix is completely wrong (my bad).

It should really be something like:

table->end = base_addr + (&end[0] - sizeof(struct unwind_table_entry))->region_end;

I'm pretty sure that the end[-1] is causing a processor fault. Which then causes kdb to kick-in, but the unwind table isn't built yet so it can't find a matching entry.

If this doesn't work, then how is one supposed to correctly get the last table entry? :)

(end == __stop___unwind)

  __start___unwind = .;         /* unwind info */
  .PARISC.unwind : { *(.PARISC.unwind) }
  __stop___unwind = .;

c.

#5 lore raw
Quoted 3 lines

It should really be something like:

table->end = base_addr + (&end[0] - sizeof(struct unwind_table_entry))->region_end;

You should be able to just do something like

struct unwind_table_entry *p = end;
...
table->end = base_addr + (--p)->region_end;

Quoted 5 lines

I'm pretty sure that the end[-1] is causing a processor fault. Which then causes kdb to kick-in, but the unwind table isn't built yet so it can't find a matching entry.

Quoted 8 lines

If this doesn't work, then how is one supposed to correctly get the last table entry? :)

(end == __stop___unwind)

  __start___unwind = .;         /* unwind info */
  .PARISC.unwind : { *(.PARISC.unwind) }
  __stop___unwind = .;

Dave

Signature · 2 lines
J. David Anglin                                  [email protected]
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)
#6 lore raw
In reply to Carlos O'Donell Jr.
Quoted 7 lines
Quoted 7 lines

It's definitely dangerous to use array indexes outside the bounds of the array, particularly on the PA. There can be problems with space register selection, although I wouldn't expect this to be a in the linux kernel. You can avoid index insns with "-mdisable-indexing". Let me know if you see anything strange in the assembler output.

Dave

The following is the output using gcc 3.0.4.

unwind.c Fri Jun 7 09:23:17 2002 renamed from unwind-orig.c Fri Jun 7 11:01:22 2002 +1 -1
@@ -106,7 +106,7 @@
        table->base_addr = base_addr;
        table->gp = gp;
        table->start = base_addr + start[0].region_start;
-       table->end = base_addr + end[-1].region_end;
+       table->end = base_addr + (--end)->region_end;
        table->table = (struct unwind_table_entry *)table_start;
        table->length = end - start;
        table->next = NULL;
unwind.s Fri Jun 7 09:24:55 2002 renamed from unwind-orig.s Fri Jun 7 11:01:33 2002 +9 -8
@@ -7,22 +7,23 @@
        .CALLINFO FRAME=0,NO_CALLS
        .ENTRY
        stw %r24,12(%r26)
-       ldw -52(%r30),%r21
+       ldw -52(%r30),%r22
        stw %r25,4(%r26)
        stw %r23,8(%r26)
-       ldw 0(%r21),%r19
-       ldw -56(%r30),%r22
+       ldw 0(%r22),%r19
+       ldw -56(%r30),%r20
        addl %r24,%r19,%r19
        stw %r19,16(%r26)
-       sub %r22,%r21,%r20
-       ldw -12(%r22),%r19
-       extrs %r20,27,28,%r20
+       ldo -16(%r20),%r20
+       ldw 4(%r20),%r19
+       sub %r20,%r22,%r21
+       extrs %r21,27,28,%r21
        addl %r24,%r19,%r24
-       stw %r20,28(%r26)
        stw %r24,20(%r26)
+       stw %r21,28(%r26)
        stw %r0,0(%r26)
        bv %r0(%r2)
-       stw %r21,24(%r26)
+       stw %r22,24(%r26)
        .EXIT
        .PROCEND
 .Lfe1:

Aside from some register juggling, the only difference is that we now use 'ldo,ldw,sub,extrs' instead of 'sub,ldw,extrs'. The question is: Can this do bad things?

I'll have time on Monday to go through it thoroughly.

c.

#7 lore raw
Quoted 8 lines
unwind.s Fri Jun 7 09:24:55 2002 renamed from unwind-orig.s Fri Jun 7 11:01:33 2002 +2 -2
-       ldw -12(%r22),%r19
-       extrs %r20,27,28,%r20
+       ldo -16(%r20),%r20
+       ldw 4(%r20),%r19

Aside from some register juggling, the only difference is that we now use 'ldo,ldw,sub,extrs' instead of 'sub,ldw,extrs'. The question is: Can this do bad things?

The only possible difference is the space register selection could be different for the ldw insns in the old code versus the new because the pointer values in the base differ by 16.

Dave

Signature · 2 lines
J. David Anglin                                  [email protected]
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)
#8 lore raw
Quoted 3 lines · John David Anglin

The only possible difference is the space register selection could be different for the ldw insns in the old code versus the new because the pointer values in the base differ by 16.

but not in the kernel because all space registers are set to 0.

Signature · 1 line
Revolutions do not require corporate support.
#9 lore raw
Quoted 6 lines · John David Anglin
Quoted 3 lines

The only possible difference is the space register selection could be different for the ldw insns in the old code versus the new because the pointer values in the base differ by 16.

but not in the kernel because all space registers are set to 0.

Are they always 0 or are there situations where drivers change a space register value? If they never change, then the memory limit is still 3GB.

Dave

Signature · 2 lines
J. David Anglin                                  [email protected]
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)
#10 lore raw
Quoted 3 lines · John David Anglin

Are they always 0 or are there situations where drivers change a space register value? If they never change, then the memory limit is still 3GB.

Nobody changes sr4-7 (the default space registers). Some drivers play with sr1. We use sr3 to access the user's process. There's no problem with accessing 4GB in either the kernel or the user process (though we limit to 3.75GB currently, i believe, just to prevent unfortunate accidents).

Signature · 1 line
Revolutions do not require corporate support.