NetBSD Problem Report #53520

From www@NetBSD.org  Mon Aug 13 18:45:10 2018
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 6400B7A16B
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 13 Aug 2018 18:45:10 +0000 (UTC)
Message-Id: <20180813184459.C5DC37A1FE@mollari.NetBSD.org>
Date: Mon, 13 Aug 2018 18:44:59 +0000 (UTC)
From: nullnilaki@gmail.com
Reply-To: nullnilaki@gmail.com
To: gnats-bugs@NetBSD.org
Subject: delay(9) is ineffective in early initialization phase sgimips kernel.
X-Send-Pr-Version: www-1.0

>Number:         53520
>Category:       port-sgimips
>Synopsis:       delay(9) is ineffective in early initialization phase sgimips kernel.
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    tsutsui
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Aug 13 18:50:00 +0000 2018
>Closed-Date:    Sun Sep 09 04:59:37 +0000 2018
>Last-Modified:  Sun Sep 09 04:59:37 +0000 2018
>Originator:     Naruaki Etomi
>Release:        NetBSD/sgimips 8.99.21
>Organization:
Japan
>Environment:
NetBSD  8.99.21 NetBSD 8.99.21 (GENERIC32_IP12) #1: Sun Aug 12 14:48:19 UTC 2018  naruaki@xserve:/usr/obj.sgimips/sys/arch/sgimips/compile/GENERIC32_IP2x sgimips

>Description:
delay(9) is used in int_attach().
https://nxr.netbsd.org/xref/src/sys/arch/sgimips/dev/int.c#164

However, ci_divisor_delay is not set yet.

It's quite meaningless.
>How-To-Repeat:

>Fix:
---------------------------------------------------------------------------------------------

diff -Naru src.orig/sys/arch/sgimips/sgimips/machdep.c src/sys/arch/sgimips/sgimips/machdep.c
--- src.orig/sys/arch/sgimips/sgimips/machdep.c 2017-11-07 03:01:06.000000000 +0000
+++ src/sys/arch/sgimips/sgimips/machdep.c      2018-08-13 18:00:07.617278991 +0000
@@ -333,6 +333,10 @@
         */
        curcpu()->ci_cpu_freq = strtoul(cpufreq, NULL, 10) * 1000000;

+       /* Calibrate later. */
+       curcpu()->ci_cycles_per_hz = curcpu()->ci_cpu_freq / (2 * hz);
+       curcpu()->ci_divisor_delay = curcpu()->ci_cpu_freq / (2 * 1000000);
+
        /*
         * Check machine (IPn) type.
         *
---------------------------------------------------------------------------------------------

This patch set Untrusted(ARCBIOS) clock. It's better than not doing anything.
Tested on Indy and Indigo R4000.

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: port-sgimips-maintainer->tsutsui
Responsible-Changed-By: tsutsui@NetBSD.org
Responsible-Changed-When: Sun, 02 Sep 2018 13:11:01 +0000
Responsible-Changed-Why:
I'll handle this.


State-Changed-From-To: open->analyzed
State-Changed-By: tsutsui@NetBSD.org
State-Changed-When: Sun, 02 Sep 2018 13:11:01 +0000
State-Changed-Why:
The basic analysis (temporary initialization of ci_divisor_delay for
early use of delay(9)) is reasonable. However it looks something wrong
around delay() implementation of sgimips. I will also investigate it.


From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: gnats-bugs@NetBSD.org
Cc: tsutsui@ceres.dti.ne.jp
Subject: Re: port-sgimips/53520 (delay(9) is ineffective in early initialization
	 phase sgimips kernel.)
Date: Sat, 8 Sep 2018 01:28:13 +0900

 > The basic analysis (temporary initialization of ci_divisor_delay for
 > early use of delay(9)) is reasonable. However it looks something wrong
 > around delay() implementation of sgimips. I will also investigate it.

 Quoted from port-sgimips@:
  http://mail-index.netbsd.org/port-sgimips/2018/09/02/msg000775.html

 > Notes for PR/53520:
 >  NetBSD/sgimips uses the ci_divisor_delay value for the 4.4BSD derived
 >  dumb delay loop. However ci_devisor_delay is designed for mips3_delay()
 >  in sys/arch/mips/mips/mips3_clock.c that uses MIPS3's internal CPU counter,
 >  i.e. "a number of CPU clock count per microsecond."
 >  (note ci_cycles_per_hz is also for common mips3_clockintr() in
 >   sys/arch/mips/mips/mips3_clockintr.c)
 > 
 >  For sgimips delay implementation (traditional 4.4BSD derived delay loop),
 >  the "cpuspeed" value (that represents "instructions per microsecond")
 >  should be used instead, as pmax and newsmips do.
 > 
 >  Anyway the cpuspeed value is not so precise as noted in
 >  sys/arch/mips/mips/mips_mcclock.c
 >   https://nxr.netbsd.org/xref/src/sys/arch/mips/mips/mips_mcclock.c?r=1.19#198
 >  so current ci_divisor_delay value is still acceptable, I think.

 The 'cpuspeed' values in mips_mcclock.c are similar with
 "(frequency [MHz]) / 2" for ci_divisor_delay so it seems
 acceptable for now to (ab)use it for traditional sgimips delay.

 I'll commit the fix in the PR soon. (with some proper comment)

 ---
 Izumi Tsutsui

From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: gnats-bugs@NetBSD.org
Cc: tsutsui@ceres.dti.ne.jp
Subject: Re: port-sgimips/53520 (delay(9) is ineffective in early initializationphase
	 sgimips kernel.)
Date: Sat, 8 Sep 2018 02:37:12 +0900

 > The basic analysis (temporary initialization of ci_divisor_delay for
 > early use of delay(9)) is reasonable.

 Note actual early calls of delay(9) are in sgimips/dev/int.c int_attach():

 ---
 	switch (mach_type) {
 	case MACH_SGI_IP6 | MACH_SGI_IP10:
 		/* Clean out interrupt masks */
 		bus_space_write_1(iot, ioh, INT1_LOCAL_MASK, 0);

 		/* Turn off timers and clear interrupts */
 		bus_space_write_1(iot, ioh, INT1_TIMER_CONTROL,
 		    (TIMER_SEL0 | TIMER_16BIT | TIMER_SWSTROBE));
 		bus_space_write_1(iot, ioh, INT1_TIMER_CONTROL,
 		    (TIMER_SEL1 | TIMER_16BIT | TIMER_SWSTROBE));
 		bus_space_write_1(iot, ioh, INT1_TIMER_CONTROL,
 		    (TIMER_SEL2 | TIMER_16BIT | TIMER_SWSTROBE));
 		wbflush();
 =>		delay(4);
 		bus_space_read_1(iot, ioh, INT1_TIMER_0_ACK);
 		bus_space_read_1(iot, ioh, INT1_TIMER_1_ACK);

 		platform.intr_establish = int1_intr_establish;
 		platform.intr1 = int1_local_intr;
 		platform.intr2 = int_8254_intr0;
 		platform.intr4 = int_8254_intr1;
 		int_8254_cal();
 		break;

 	case MACH_SGI_IP12:
 	case MACH_SGI_IP20:
 	case MACH_SGI_IP22:
 		/* Clean out interrupt masks */
 		bus_space_write_1(iot, ioh, INT2_LOCAL0_MASK, 0);
 		bus_space_write_1(iot, ioh, INT2_LOCAL1_MASK, 0);
 		bus_space_write_1(iot, ioh, INT2_MAP_MASK0, 0);
 		bus_space_write_1(iot, ioh, INT2_MAP_MASK1, 0);

 		/* Reset timer interrupts */
 		bus_space_write_1(iot, ioh, INT2_TIMER_CONTROL,
 		    (TIMER_SEL0 | TIMER_16BIT | TIMER_SWSTROBE));
 		bus_space_write_1(iot, ioh, INT2_TIMER_CONTROL,
 		    (TIMER_SEL1 | TIMER_16BIT | TIMER_SWSTROBE));
 		bus_space_write_1(iot, ioh, INT2_TIMER_CONTROL,
 		    (TIMER_SEL2 | TIMER_16BIT | TIMER_SWSTROBE));
 		wbflush();
 =>		delay(4);
 		bus_space_write_1(iot, ioh, INT2_TIMER_CLEAR, 0x03);

 		if (mach_type == MACH_SGI_IP12) {
 			platform.intr_establish = int2_intr_establish;
 			platform.intr1 = int2_local0_intr;
 			platform.intr2 = int2_local1_intr;
 			platform.intr3 = int_8254_intr0;
 			platform.intr4 = int_8254_intr1;
 			int_8254_cal();
 		} else {
 			platform.intr_establish = int2_intr_establish;
 			platform.intr0 = int2_local0_intr;
 			platform.intr1 = int2_local1_intr;
 #ifdef MIPS3
 			curcpu()->ci_cpu_freq = int2_cpu_freq(self);
 #endif
 		}
 		break;
 ---
 but it looks IP2x machines (MIPS3 Indy and Indigo2) works without
 this initialization (i.e. it doesn't require delay(9), or short delay
 by calling a function is enough?).

 Indigo R3k machines (IP6/10/12) are actually affected by
 too short delay(9) caused by this lack of initialization,
 according to report of Etomi-san (PR originator).

 ---
 Izumi Tsutsui

From: "Izumi Tsutsui" <tsutsui@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53520 CVS commit: src/sys/arch/sgimips/sgimips
Date: Sat, 8 Sep 2018 02:33:34 +0000

 Module Name:	src
 Committed By:	tsutsui
 Date:		Sat Sep  8 02:33:34 UTC 2018

 Modified Files:
 	src/sys/arch/sgimips/sgimips: machdep.c

 Log Message:
 Initialize ci_divisor_delay by temporary cpufreq for early delay(9) calls.

 This is actually required for Indigo R3k.
 Reported by Naruaki Etomi in PR port-sgimips/53520.


 To generate a diff of this commit:
 cvs rdiff -u -r1.146 -r1.147 src/sys/arch/sgimips/sgimips/machdep.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: analyzed->closed
State-Changed-By: tsutsui@NetBSD.org
State-Changed-When: Sun, 09 Sep 2018 04:59:37 +0000
State-Changed-Why:
Fix committed. If it's worth to pullup this to netbsd-8
(i.e. Indigo R3K is functional on the release branch) please let me know.


>Unformatted:

Home	PR Database Search