NetBSD Problem Report #39642

From root@azeotrope.org  Sun Sep 28 02:10:38 2008
Return-Path: <root@azeotrope.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 72D2A63B884
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 28 Sep 2008 02:10:38 +0000 (UTC)
Message-Id: <20080928021033.2218D14DE@sloth.azeotrope.org>
Date: Sat, 27 Sep 2008 21:10:32 -0500 (CDT)
From: khym@sloth.azeotrope.org
Reply-To: khym@sloth.azeotrope.org
To: gnats-bugs@gnats.NetBSD.org
Subject: tcom(4) driver is broken
X-Send-Pr-Version: 3.95

>Number:         39642
>Category:       kern
>Synopsis:       tcom(4) driver is broken
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Sep 28 02:15:00 +0000 2008
>Closed-Date:    Sun Sep 28 15:41:08 +0000 2008
>Last-Modified:  Sun Sep 28 19:50:02 +0000 2008
>Originator:     Dave Huang
>Release:        NetBSD 4.99.72
>Organization:

>Environment:


System: NetBSD sloth.azeotrope.org 4.99.72 NetBSD 4.99.72 (FOXY) #4: Sat Sep 27 20:02:31 CDT 2008 khym@cheetah.azeotrope.org:/usr/obj.i386/sys/arch/i386/compile/FOXY i386
Architecture: i386
Machine: i386
>Description:
	The tcom(4) driver probes and attaches com ports, but opening a tty
device causes fatal page fault in supervisor mode. It looks like the changes
in http://mail-index.netbsd.org/source-changes/2008/03/14/msg003488.html
were made for com(4) devices, but not for the devices that attached com(4)
as a slave device (i.e., multiport serial cards).

>How-To-Repeat:
	Boot -current kernel on a machine that has a multiport serial card
that uses the tcom(4) driver, then watch it crash when the boot process
runs "ttyflags -a". Or boot single-user, and do "stty -f /dev/tty02"
(assuming tty02 is one of the tcom ports).

>Fix:
	I'm not sure if this is correct, but it seems to work for me. I
think something similar will need to be done to the other multiport serial
cards in dev/isa that have the same basic structure. On further inspection,
it looks like ast(4) was already fixed (PR kern/38776); however, boca, ioat,
moxa, and rtfps look like they're still broken. My patch does things
differently than the way ast was fixed; perhaps the ast way is better;
I don't actually know anything about the kernel config mechanism.

Index: tcom.c
===================================================================
RCS file: /cvsroot/src/sys/dev/isa/tcom.c,v
retrieving revision 1.16
diff -u -r1.16 tcom.c
--- tcom.c	28 Apr 2008 20:23:52 -0000	1.16
+++ tcom.c	28 Sep 2008 01:37:59 -0000
@@ -91,28 +91,27 @@
 #define	STATUS_SIZE	8		/* 8 bytes reserved for irq status */

 struct tcom_softc {
-	struct device sc_dev;
+	device_t sc_dev;
 	void *sc_ih;

 	bus_space_tag_t sc_iot;
 	int sc_iobase;

 	int sc_alive;			/* mask of slave units attached */
-	void *sc_slaves[NSLAVES];	/* com device unit numbers */
+	device_t sc_slaves[NSLAVES];	/* com slave devices */
 	bus_space_handle_t sc_slaveioh[NSLAVES];
 	bus_space_handle_t sc_statusioh;
 };

-int tcomprobe(struct device *, struct cfdata *, void *);
-void tcomattach(struct device *, struct device *, void *);
+int tcomprobe(struct device *, cfdata_t, void *);
+void tcomattach(struct device *, device_t, void *);
 int tcomintr(void *);

-CFATTACH_DECL(tcom, sizeof(struct tcom_softc),
+CFATTACH_DECL_NEW(tcom, sizeof(struct tcom_softc),
     tcomprobe, tcomattach, NULL, NULL);

 int
-tcomprobe(struct device *parent, struct cfdata *self,
-    void *aux)
+tcomprobe(struct device *parent, cfdata_t self, void *aux)
 {
 	struct isa_attach_args *ia = aux;
 	bus_space_tag_t iot = ia->ia_iot;
@@ -181,9 +180,9 @@
 }

 void
-tcomattach(struct device *parent, struct device *self, void *aux)
+tcomattach(struct device *parent, device_t self, void *aux)
 {
-	struct tcom_softc *sc = (void *)self;
+	struct tcom_softc *sc = device_private(self);
 	struct isa_attach_args *ia = aux;
 	struct commulti_attach_args ca;
 	bus_space_tag_t iot = ia->ia_iot;
@@ -191,6 +190,7 @@

 	printf("\n");

+	sc->sc_dev = self;
 	sc->sc_iot = ia->ia_iot;
 	sc->sc_iobase = ia->ia_io[0].ir_addr;

@@ -199,14 +199,14 @@
 		if (!com_is_console(iot, iobase, &sc->sc_slaveioh[i]) &&
 		    bus_space_map(iot, iobase, COM_NPORTS, 0,
 			&sc->sc_slaveioh[i])) {
-			aprint_error_dev(&sc->sc_dev, "can't map i/o space for slave %d\n", i);
+			aprint_error_dev(sc->sc_dev, "can't map i/o space for slave %d\n", i);
 			return;
 		}
 	}

 	if (bus_space_map(iot, sc->sc_iobase + STATUS_OFFSET, STATUS_SIZE, 0,
 	    &sc->sc_statusioh)) {
-		aprint_error_dev(&sc->sc_dev, "can't map status space\n");
+		aprint_error_dev(sc->sc_dev, "can't map status space\n");
 		return;
 	}

@@ -242,7 +242,7 @@
 	for (;;) {
 #define	TRY(n) \
 		if (bits & (1 << (n))) \
-			comintr(sc->sc_slaves[n]);
+			comintr(device_private(sc->sc_slaves[n]));
 		TRY(0);
 		TRY(1);
 		TRY(2);

>Release-Note:

>Audit-Trail:
From: Dave Huang <khym@azeotrope.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/39642: tcom(4) driver is broken
Date: Sat, 27 Sep 2008 21:45:11 -0500

 Here's a patch that stores the com_softc pointers in sc_slaves[], as ast(4)
 does.

 Index: tcom.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/isa/tcom.c,v
 retrieving revision 1.16
 diff -u -r1.16 tcom.c
 --- tcom.c	28 Apr 2008 20:23:52 -0000	1.16
 +++ tcom.c	28 Sep 2008 02:42:12 -0000
 @@ -91,28 +91,27 @@
  #define	STATUS_SIZE	8		/* 8 bytes reserved for irq status */

  struct tcom_softc {
 -	struct device sc_dev;
 +	device_t sc_dev;
  	void *sc_ih;

  	bus_space_tag_t sc_iot;
  	int sc_iobase;

  	int sc_alive;			/* mask of slave units attached */
 -	void *sc_slaves[NSLAVES];	/* com device unit numbers */
 +	void *sc_slaves[NSLAVES];	/* com device softc pointers */
  	bus_space_handle_t sc_slaveioh[NSLAVES];
  	bus_space_handle_t sc_statusioh;
  };

 -int tcomprobe(struct device *, struct cfdata *, void *);
 -void tcomattach(struct device *, struct device *, void *);
 +int tcomprobe(struct device *, cfdata_t, void *);
 +void tcomattach(struct device *, device_t, void *);
  int tcomintr(void *);

 -CFATTACH_DECL(tcom, sizeof(struct tcom_softc),
 +CFATTACH_DECL_NEW(tcom, sizeof(struct tcom_softc),
      tcomprobe, tcomattach, NULL, NULL);

  int
 -tcomprobe(struct device *parent, struct cfdata *self,
 -    void *aux)
 +tcomprobe(struct device *parent, cfdata_t self, void *aux)
  {
  	struct isa_attach_args *ia = aux;
  	bus_space_tag_t iot = ia->ia_iot;
 @@ -181,16 +180,18 @@
  }

  void
 -tcomattach(struct device *parent, struct device *self, void *aux)
 +tcomattach(struct device *parent, device_t self, void *aux)
  {
 -	struct tcom_softc *sc = (void *)self;
 +	struct tcom_softc *sc = device_private(self);
  	struct isa_attach_args *ia = aux;
  	struct commulti_attach_args ca;
  	bus_space_tag_t iot = ia->ia_iot;
  	int i, iobase;
 +	device_t slave;

  	printf("\n");

 +	sc->sc_dev = self;
  	sc->sc_iot = ia->ia_iot;
  	sc->sc_iobase = ia->ia_io[0].ir_addr;

 @@ -199,14 +200,14 @@
  		if (!com_is_console(iot, iobase, &sc->sc_slaveioh[i]) &&
  		    bus_space_map(iot, iobase, COM_NPORTS, 0,
  			&sc->sc_slaveioh[i])) {
 -			aprint_error_dev(&sc->sc_dev, "can't map i/o space for slave %d\n", i);
 +			aprint_error_dev(sc->sc_dev, "can't map i/o space for slave %d\n", i);
  			return;
  		}
  	}

  	if (bus_space_map(iot, sc->sc_iobase + STATUS_OFFSET, STATUS_SIZE, 0,
  	    &sc->sc_statusioh)) {
 -		aprint_error_dev(&sc->sc_dev, "can't map status space\n");
 +		aprint_error_dev(sc->sc_dev, "can't map status space\n");
  		return;
  	}

 @@ -217,9 +218,11 @@
  		ca.ca_iobase = sc->sc_iobase + i * COM_NPORTS;
  		ca.ca_noien = 0;

 -		sc->sc_slaves[i] = config_found(self, &ca, commultiprint);
 -		if (sc->sc_slaves[i] != NULL)
 +		slave = config_found(self, &ca, commultiprint);
 +		if (slave != NULL) {
  			sc->sc_alive |= 1 << i;
 +			sc->sc_slaves[i] = device_private(slave);
 +		}
  	}

  	sc->sc_ih = isa_intr_establish(ia->ia_ic, ia->ia_irq[0].ir_irq,

From: Martin Husemann <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/39642 CVS commit: src/sys/dev/isa
Date: Sun, 28 Sep 2008 15:39:40 +0000 (UTC)

 Module Name:	src
 Committed By:	martin
 Date:		Sun Sep 28 15:39:40 UTC 2008

 Modified Files:
 	src/sys/dev/isa: tcom.c

 Log Message:
 Apply patch from Dave Huang in PR kern/39642: catch up on com(4)'s
 device_t/softc split.


 To generate a diff of this commit:
 cvs rdiff -r1.16 -r1.17 src/sys/dev/isa/tcom.c

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

State-Changed-From-To: open->closed
State-Changed-By: martin@NetBSD.org
State-Changed-When: Sun, 28 Sep 2008 15:41:08 +0000
State-Changed-Why:
second patch applied - thanks!


From: David Huang <khym@azeotrope.org>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org,
 netbsd-bugs@netbsd.org,
 gnats-admin@netbsd.org,
 martin@NetBSD.org
Subject: Re: kern/39642 (tcom(4) driver is broken)
Date: Sun, 28 Sep 2008 14:45:35 -0500

 On Sep 28, 2008, at 10:41 AM, martin@netbsd.org wrote:

 > Synopsis: tcom(4) driver is broken
 >
 > State-Changed-From-To: open->closed
 > State-Changed-By: martin@NetBSD.org
 > State-Changed-When: Sun, 28 Sep 2008 15:41:08 +0000
 > State-Changed-Why:
 > second patch applied - thanks!

 Thanks :) What about the other drivers I mentioned that are similarly  
 broken? I could put together a patch if you're interested, although I  
 have no way of testing... I'm pretty sure that they need the same sort  
 of change though.

>Unformatted:

Home	PR Database Search