NetBSD Problem Report #39257

From www@NetBSD.org  Thu Jul 31 14:45:08 2008
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 8717763B9AA
	for <gnats-bugs@gnats.netbsd.org>; Thu, 31 Jul 2008 14:45:08 +0000 (UTC)
Message-Id: <20080731144507.C70CE63B99A@narn.NetBSD.org>
Date: Thu, 31 Jul 2008 14:45:07 +0000 (UTC)
From: konradk@pacomp.pl
Reply-To: konradk@pacomp.pl
To: gnats-bugs@NetBSD.org
Subject: Mistake in sys/arch/powerpc/powerpc/trap_subr.S file
X-Send-Pr-Version: www-1.0

>Number:         39257
>Category:       port-powerpc
>Synopsis:       Mistake in sys/arch/powerpc/powerpc/trap_subr.S file
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-powerpc-maintainer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jul 31 14:50:00 +0000 2008
>Closed-Date:    Sun Dec 05 13:36:01 +0000 2010
>Last-Modified:  Sun Dec 05 13:36:01 +0000 2010
>Originator:     Konrad Karpowicz
>Release:        none
>Organization:
Pacomp
>Environment:
none
>Description:
Hello
Our company uses FreeBSD on PowerPC e300 core and I wanted to compare our TLB Data Store Miss Exception handler with the same part of code in NetBSD. In my opinion there is a serious bug in sys/powerpc/powerpc/trap_subr.S file - lines 617-624. Value stored in R1 is not used. And changed value in R2 causes error when reloading TLB entry. It would be great if I could contact someone who is able to confirm (or not) my opinion.
Kind regards,
Konrad

>How-To-Repeat:
I do not know - my FreeBSD kernel crashes.
>Fix:
I think it is not difficult but I am not sure at the moment.

>Release-Note:

>Audit-Trail:
From: Tim Rightnour <root@garbled.net>
To: konradk@pacomp.pl
Cc: gnats-bugs@netbsd.org
Subject: RE: port-powerpc/39257: Mistake in sys/arch/powerpc/powerpc/trap_subr.S file
Date: Thu, 31 Jul 2008 11:55:01 -0700 (MST)

 On 31-Jul-2008 konradk@pacomp.pl wrote:
 > Our company uses FreeBSD on PowerPC e300 core and I wanted to compare our TLB
 > Data Store Miss Exception handler with the same part of code in NetBSD. In my
 > opinion there is a serious bug in sys/powerpc/powerpc/trap_subr.S file -
 > lines
 > 617-624. Value stored in R1 is not used. And changed value in R2 causes error
 > when reloading TLB entry. It would be great if I could contact someone who is
 > able to confirm (or not) my opinion.
 > Kind regards,
 > Konrad

 I looked at the file at the location you mentioned, but I don't think the
 current code in CVS matches whatever you are looking at.  The line numbers you
 gave don't match up.  Can you please either tell me what the CVS revision
 number at the top of the file you are looking at is, or look at the most
 current one, which is:

 /*      $NetBSD: trap_subr.S,v 1.64 2008/05/24 21:39:01 phx Exp $       */

 Then let me know which line numbers in that file.

 ---
 Tim Rightnour <root@garbled.net>
 NetBSD: Free multi-architecture OS http://www.netbsd.org/
 Genecys: Open Source 3D MMORPG: http://www.genecys.org/

From: Konrad Karpowicz <konradk@pacomp.pl>
To: gnats-bugs@NetBSD.org
Cc: port-powerpc-maintainer@netbsd.org, gnats-admin@netbsd.org, 
 netbsd-bugs@netbsd.org, konstrukcja <konstrukcja@pacomp.pl>
Subject: Re: port-powerpc/39257: Mistake in sys/arch/powerpc/powerpc/trap_subr.S
 file
Date: Fri, 01 Aug 2008 11:43:03 +0200

 Tim Rightnour pisze:
 >  I looked at the file at the location you mentioned, but I don't think the
 >  current code in CVS matches whatever you are looking at.  The line numbers you
 >  gave don't match up.  Can you please either tell me what the CVS revision
 >  number at the top of the file you are looking at is, or look at the most
 >  current one, which is
 Dear Tim,
 Thank you very much for quick answer. You are right, I was looking at 
 trap_subr.S v1.59.
 In v 1.64 the line numbers are 540-547.
 Lines 540 and 541: in my opinion the contents of R1 are not used anymore.
 Line 544: change of the contents of R2 results in incorrect value loaded 
 into R1 in line 547.
 Kind regards,
 Konrad



 Tim Rightnour pisze:
 > The following reply was made to PR port-powerpc/39257; it has been noted by GNATS.
 >
 > From: Tim Rightnour <root@garbled.net>
 > To: konradk@pacomp.pl
 > Cc: gnats-bugs@netbsd.org
 > Subject: RE: port-powerpc/39257: Mistake in sys/arch/powerpc/powerpc/trap_subr.S file
 > Date: Thu, 31 Jul 2008 11:55:01 -0700 (MST)
 >
 >  On 31-Jul-2008 konradk@pacomp.pl wrote:
 >  > Our company uses FreeBSD on PowerPC e300 core and I wanted to compare our TLB
 >  > Data Store Miss Exception handler with the same part of code in NetBSD. In my
 >  > opinion there is a serious bug in sys/powerpc/powerpc/trap_subr.S file -
 >  > lines
 >  > 617-624. Value stored in R1 is not used. And changed value in R2 causes error
 >  > when reloading TLB entry. It would be great if I could contact someone who is
 >  > able to confirm (or not) my opinion.
 >  > Kind regards,
 >  > Konrad
 >  
 >  I looked at the file at the location you mentioned, but I don't think the
 >  current code in CVS matches whatever you are looking at.  The line numbers you
 >  gave don't match up.  Can you please either tell me what the CVS revision
 >  number at the top of the file you are looking at is, or look at the most
 >  current one, which is:
 >  
 >  /*      $NetBSD: trap_subr.S,v 1.64 2008/05/24 21:39:01 phx Exp $       */
 >  
 >  Then let me know which line numbers in that file.
 >  
 >  ---
 >  Tim Rightnour <root@garbled.net>
 >  NetBSD: Free multi-architecture OS http://www.netbsd.org/
 >  Genecys: Open Source 3D MMORPG: http://www.genecys.org/
 >  
 >
 >   

From: Havard Eidnes <he@NetBSD.org>
To: konradk@pacomp.pl
Cc: gnats-bugs@NetBSD.org, port-powerpc-maintainer@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, konstrukcja@pacomp.pl
Subject: Re: port-powerpc/39257: Mistake in
 sys/arch/powerpc/powerpc/trap_subr.S file
Date: Fri, 01 Aug 2008 14:04:46 +0200 (CEST)

 > Tim Rightnour pisze:
 > >  I looked at the file at the location you mentioned, but I don't
 > >  think the current code in CVS matches whatever you are looking
 > >  at.  The line numbers you gave don't match up.  Can you please
 > >  either tell me what the CVS revision number at the top of the
 > >  file you are looking at is, or look at the most current one,
 > >  which is
 > Dear Tim,
 > Thank you very much for quick answer. You are right, I was looking at 
 > trap_subr.S v1.59.
 > In v 1.64 the line numbers are 540-547.
 > Lines 540 and 541: in my opinion the contents of R1 are not used anymore.
 > Line 544: change of the contents of R2 results in incorrect value loaded 
 > into R1 in line 547.

 It would probably be helpful if you quoted the actual code and gave
 a bit more context, such as which function you are talking about.
 The reason I say this is that in revision 1.64 of trap_subr.S, lines
 540-547 look as follows:

 #endif
 _C_LABEL(tlbdlmsize) = .-_C_LABEL(tlbdlmiss)

 /* LINTSTUB: Var: int tlbdsmiss[1], tlbdsmsize[1]; */
         .globl  _C_LABEL(tlbdsmiss),_C_LABEL(tlbdsmsize)
 _C_LABEL(tlbdsmiss):
         mfspr   %r2,SPR_HASH1           /* get first pointer */
         li      %r1,%r8

 which is probably not what you wanted to indicate.

 Regards,

 - Havard

From: Konrad Karpowicz <konradk@pacomp.pl>
To: Havard Eidnes <he@NetBSD.org>
Cc: gnats-bugs@NetBSD.org, port-powerpc-maintainer@netbsd.org, 
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, konstrukcja@pacomp.pl
Subject: Re: port-powerpc/39257: Mistake in sys/arch/powerpc/powerpc/trap_subr.S
 file
Date: Fri, 01 Aug 2008 14:23:29 +0200

 Havard Eidnes pisze:
 >> Tim Rightnour pisze:
 >>     
 >>>  I looked at the file at the location you mentioned, but I don't
 >>>  think the current code in CVS matches whatever you are looking
 >>>  at.  The line numbers you gave don't match up.  Can you please
 >>>  either tell me what the CVS revision number at the top of the
 >>>  file you are looking at is, or look at the most current one,
 >>>  which is
 >>>       
 >> Dear Tim,
 >> Thank you very much for quick answer. You are right, I was looking at 
 >> trap_subr.S v1.59.
 >> In v 1.64 the line numbers are 540-547.
 >> Lines 540 and 541: in my opinion the contents of R1 are not used anymore.
 >> Line 544: change of the contents of R2 results in incorrect value loaded 
 >> into R1 in line 547.
 >>     
 >
 > It would probably be helpful if you quoted the actual code and gave
 > a bit more context, such as which function you are talking about.
 > The reason I say this is that in revision 1.64 of trap_subr.S, lines
 > 540-547 look as follows:
 >
 > #endif
 > _C_LABEL(tlbdlmsize) = .-_C_LABEL(tlbdlmiss)
 >
 > /* LINTSTUB: Var: int tlbdsmiss[1], tlbdsmsize[1]; */
 >         .globl  _C_LABEL(tlbdsmiss),_C_LABEL(tlbdsmsize)
 > _C_LABEL(tlbdsmiss):
 >         mfspr   %r2,SPR_HASH1           /* get first pointer */
 >         li      %r1,%r8
 >
 > which is probably not what you wanted to indicate.
 >
 > Regards,
 >
 > - Havard
 Dear Havard,

 Sorry again. German NetBSD FTP server has out of date NetBSD-current.
 The lines are 590-597:

     mfspr    %r3,SPR_DMISS        /* get the miss address */
     mfsrin    %r1,%r3            /* get the segment register */
     mfsrr1    %r3
     rlwinm    %r3,%r3,18,31,31    /* get PR-bit */
     rlwnm.    %r2,%r2,%r3,1,1        /* get the key */
     bne-    9b            /* protection violation */
 8:    /* found, set reference/change bits */
     ldreg    %r1,4(%r2)        /* reload tlb entry */

 At the moment I know which line has a bug. It is line
 rlwnm.    %r2,%r2,%r3,1,1        /* get the key */
 and it should be:
 rlwnm.    %r1,%r1,%r3,1,1        /* get the key */

 Now it seems to work OK. How is it possible that you haven't noticed it 
 before?

 Kind regards,
 Konrad Karpowicz
 PACOMP

From: Havard Eidnes <he@NetBSD.org>
To: konradk@pacomp.pl
Cc: gnats-bugs@NetBSD.org, port-powerpc-maintainer@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, konstrukcja@pacomp.pl
Subject: Re: port-powerpc/39257: Mistake in
 sys/arch/powerpc/powerpc/trap_subr.S file
Date: Fri, 01 Aug 2008 14:31:11 +0200 (CEST)

 > Now it seems to work OK.

 Good!

 > How is it possible that you haven't noticed it before?

 I'll leave that to someone with powerpc clue to answer (Tim?); I
 just saw there was still an obvious mismatch; thanks for digging
 out the precise details.

 Regards,

 - Havard

State-Changed-From-To: open->closed
State-Changed-By: macallan@NetBSD.org
State-Changed-When: Sun, 05 Dec 2010 13:36:01 +0000
State-Changed-Why:
this was fixed in rev. 1.65


>Unformatted:

Home	PR Database Search