[parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[Carlos O'Donell Jr.]

Randolph,

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

--- unwind.c.orig       Thu Jun  6 08:51:02 2002
+++ unwind.c    Thu Jun  6 08:51:49 2002
@@ -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.

Re: [parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[Matthew Wilcox]

On Thu, Jun 06, 2002 at 09:21:15AM -0400, Carlos O'Donell Jr. wrote:
> 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.

> 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
> parisc-linux@lists.parisc-linux.org
> http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

-- 
Revolutions do not require corporate support.

Re: [parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[Carlos O'Donell Jr.]

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

Re: [parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[John David Anglin]

> -       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
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: [parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[Carlos O'Donell Jr.]

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

Re: [parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[John David Anglin]

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

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

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: [parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[Carlos O'Donell Jr.]

> > 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-orig.c       Fri Jun  7 11:01:22 2002
+++ unwind.c    Fri Jun  7 09:23:17 2002
@@ -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-orig.s       Fri Jun  7 11:01:33 2002
+++ unwind.s    Fri Jun  7 09:24:55 2002
@@ -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.

Re: [parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[John David Anglin]

> -       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
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: [parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[Matthew Wilcox]

On Fri, Jun 07, 2002 at 12:42:37PM -0400, John David Anglin wrote:
> 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.

-- 
Revolutions do not require corporate support.

Re: [parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[John David Anglin]

> On Fri, Jun 07, 2002 at 12:42:37PM -0400, John David Anglin wrote:
> > 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
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

Re: [parisc-linux] kdb getting healthier? (PCI guru needed, that's you willy!)

[Matthew Wilcox]

On Fri, Jun 07, 2002 at 01:01:29PM -0400, John David Anglin wrote:
> 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).

-- 
Revolutions do not require corporate support.