NetBSD Problem Report #52468

From www@NetBSD.org  Sun Aug  6 20:38:31 2017
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" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id EC1FD7A1FE
	for <gnats-bugs@gnats.NetBSD.org>; Sun,  6 Aug 2017 20:38:30 +0000 (UTC)
Message-Id: <20170806203830.049CC7A2A6@mollari.NetBSD.org>
Date: Sun,  6 Aug 2017 20:38:29 +0000 (UTC)
From: glitch@glitchwrks.com
Reply-To: glitch@glitchwrks.com
To: gnats-bugs@NetBSD.org
Subject: ISA DMA incorrectly maps I/O port 0x80 for page address registers
X-Send-Pr-Version: www-1.0

>Number:         52468
>Category:       kern
>Synopsis:       ISA DMA incorrectly maps I/O port 0x80 for page address registers
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    jdolecek
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Aug 06 20:40:00 +0000 2017
>Closed-Date:    Sat Jun 16 00:02:48 +0000 2018
>Last-Modified:  Sat Jun 16 00:02:48 +0000 2018
>Originator:     Jonathan Chapman
>Release:        NetBSD/i386 7.1
>Organization:
glitchwrks.com
>Environment:
NetBSD netbsd-ipc.bv.theglitchworks.net 7.1 NetBSD 7.1 (IPC) #28: Sun Aug  6 15:58:18 EDT 2017  glitch@netbsd-ipc.bv.theglitchworks.net:/usr/src/sys/arch/i386/compile/IPC i386
>Description:
While developing an ISA bus driver for NetBSD, I found that I could not bus_space_map() I/O port 0x80, which is typically where the PC BIOS writes POST debug codes. You can plug in a little board with a hex display and view these codes; some software also will write to 0x80 after the OS has taken over as a status display.

I was able to map my driver onto port 0x300 (I have a POST display card that supports this) and verify the driver was functioning correctly. Going through /usr/src/sys/dev/isa/isareg.h I discovered that the ISA DMA system was mapping its DMA page address registers starting at 0x80. /usr/src/sys/dev/isa/isadma.c showed that a 16-byte block was being mapped here. The DMA registers do not start at 0x80, they begin at 0x81 and extend to 0x8F, which this patch corrects.
>How-To-Repeat:
Attempt to bus_space_map() a one-byte I/O port at 0x80 on an ISA bus with an ISA DMA controller attached.
>Fix:
The following two patches apply cleanly to the 7.1 source and resolve the problem.

*** usr/src/sys/dev/isa/isareg.h.orig	2017-08-06 16:25:21.671407088 -0400
--- usr/src/sys/dev/isa/isareg.h	2017-08-06 16:30:10.668423204 -0400
***************
*** 55,61 ****
  #define	IO_PPI		0x061		/* Programmable Peripheral Interface */
  #define	IO_RTC		0x070		/* RTC */
  #define	IO_NMI		IO_RTC		/* NMI Control */
! #define	IO_DMAPG	0x080		/* DMA Page Registers */
  #define	IO_ICU2		0x0A0		/* 8259A Interrupt Controller #2 */
  #define	IO_DMA2		0x0C0		/* 8237A DMA Controller #2 */
  #define	IO_NPX		0x0F0		/* Numeric Coprocessor */
--- 55,61 ----
  #define	IO_PPI		0x061		/* Programmable Peripheral Interface */
  #define	IO_RTC		0x070		/* RTC */
  #define	IO_NMI		IO_RTC		/* NMI Control */
! #define	IO_DMAPG	0x081		/* DMA Page Registers */
  #define	IO_ICU2		0x0A0		/* 8259A Interrupt Controller #2 */
  #define	IO_DMA2		0x0C0		/* 8237A DMA Controller #2 */
  #define	IO_NPX		0x0F0		/* Numeric Coprocessor */


*** usr/src/sys/dev/isa/isadma.c.orig	2017-08-06 16:25:29.462596254 -0400
--- usr/src/sys/dev/isa/isadma.c	2017-08-06 16:30:14.680520599 -0400
*************** _isa_dmainit(struct isa_dma_state *ids,
*** 169,175 ****
  		if (bus_space_map(ids->ids_bst, IO_DMA2, DMA2_IOSIZE, 0,
  		    &ids->ids_dma2h))
  			panic("_isa_dmainit: unable to map DMA controller #2");
! 		if (bus_space_map(ids->ids_bst, IO_DMAPG, 0xf, 0,
  		    &ids->ids_dmapgh))
  			panic("_isa_dmainit: unable to map DMA page registers");

--- 169,175 ----
  		if (bus_space_map(ids->ids_bst, IO_DMA2, DMA2_IOSIZE, 0,
  		    &ids->ids_dma2h))
  			panic("_isa_dmainit: unable to map DMA controller #2");
! 		if (bus_space_map(ids->ids_bst, IO_DMAPG, 0xe, 0,
  		    &ids->ids_dmapgh))
  			panic("_isa_dmainit: unable to map DMA page registers");


>Release-Note:

>Audit-Trail:
From: glitch <glitch@glitchwrks.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/52468: ISA DMA incorrectly maps I/O port 0x80 for page
 address registers
Date: Sun, 6 Aug 2017 22:36:39 -0400

 Updated patch, unified format against /usr per #netbsd talk preferences. This patch corrects the offsets for bus_space_read() and bus_space_write() in isadma.c -- tested that ISA DMA is actually working with a SoundBlaster 16 which uses ISA DMA for sound sample playback.

 Thanks to Riastradh from #netbsd on freenode for pointing out my omission, and suggesting some of the additional fixes in this patch.

 diff -uNr usr_orig/src/sys/dev/isa/isadma.c usr/src/sys/dev/isa/isadma.c
 --- usr_orig/src/sys/dev/isa/isadma.c	2010-11-13 08:52:03.000000000 -0500
 +++ usr/src/sys/dev/isa/isadma.c	2017-08-06 19:53:51.163023103 -0400
 @@ -53,15 +53,17 @@
  struct isa_mem *isa_mem_head;

  /*
 - * High byte of DMA address is stored in this DMAPG register for
 - * the Nth DMA channel.
 + * DMA Channel to Address Page Register mapping
 + *
 + * Low byte of the DMA Address Page Register is stored in this DMAPG register
 + * for the Nth DMA channel.
   */
 -static int dmapageport[2][4] = {
 -	{0x7, 0x3, 0x1, 0x2},
 -	{0xf, 0xb, 0x9, 0xa}
 +static const int dmapageport[2][4] = {
 +	{0x87, 0x83, 0x81, 0x82},
 +	{0x8f, 0x8b, 0x89, 0x8a}
  };

 -static u_int8_t dmamode[] = {
 +static const u_int8_t dmamode[] = {
  	/* write to device/read from device */
  	DMA37MD_READ | DMA37MD_SINGLE,
  	DMA37MD_WRITE | DMA37MD_SINGLE,
 @@ -169,7 +171,7 @@
  		if (bus_space_map(ids->ids_bst, IO_DMA2, DMA2_IOSIZE, 0,
  		    &ids->ids_dma2h))
  			panic("_isa_dmainit: unable to map DMA controller #2");
 -		if (bus_space_map(ids->ids_bst, IO_DMAPG, 0xf, 0,
 +		if (bus_space_map(ids->ids_bst, IO_DMAPG, DMAPG_IOSIZE, 0,
  		    &ids->ids_dmapgh))
  			panic("_isa_dmainit: unable to map DMA page registers");

 @@ -211,7 +213,7 @@
  	/*
  	 * Unmap the registers used by the ISA DMA controller.
  	 */
 -	bus_space_unmap(ids->ids_bst, ids->ids_dmapgh, 0xf);
 +	bus_space_unmap(ids->ids_bst, ids->ids_dmapgh, DMAPG_IOSIZE);
  	bus_space_unmap(ids->ids_bst, ids->ids_dma2h, DMA2_IOSIZE);
  	bus_space_unmap(ids->ids_bst, ids->ids_dma1h, DMA1_IOSIZE);

 @@ -430,7 +432,7 @@
  		/* send start address */
  		waport = DMA1_CHN(ochan);
  		bus_space_write_1(ids->ids_bst, ids->ids_dmapgh,
 -		    dmapageport[0][ochan], (dmaaddr >> 16) & 0xff);
 +		    dmapageport[0][ochan] - IO_DMAPG, (dmaaddr >> 16) & 0xff);
  		bus_space_write_1(ids->ids_bst, ids->ids_dma1h, waport,
  		    dmaaddr & 0xff);
  		bus_space_write_1(ids->ids_bst, ids->ids_dma1h, waport,
 @@ -449,7 +451,7 @@
  		/* send start address */
  		waport = DMA2_CHN(ochan);
  		bus_space_write_1(ids->ids_bst, ids->ids_dmapgh,
 -		    dmapageport[1][ochan], (dmaaddr >> 16) & 0xff);
 +		    dmapageport[1][ochan] - IO_DMAPG, (dmaaddr >> 16) & 0xff);
  		dmaaddr >>= 1;
  		bus_space_write_1(ids->ids_bst, ids->ids_dma2h, waport,
  		    dmaaddr & 0xff);
 diff -uNr usr_orig/src/sys/dev/isa/isadmareg.h usr/src/sys/dev/isa/isadmareg.h
 --- usr_orig/src/sys/dev/isa/isadmareg.h	2008-04-28 16:23:52.000000000 -0400
 +++ usr/src/sys/dev/isa/isadmareg.h	2017-08-06 17:43:47.508172970 -0400
 @@ -47,6 +47,10 @@
  #define	ISA_DMA_MAXSIZE_DEFAULT(chan)					\
  	(((chan) & 4) ? ISA_DMA_MAXSIZE_16BIT : ISA_DMA_MAXSIZE_8BIT)

 +/* DMA Page Address Registers size */
 +
 +#define	DMAPG_IOSIZE	(1*15)
 +
  /*
   * Register definitions for DMA controller 1 (channels 0..3):
   */
 diff -uNr usr_orig/src/sys/dev/isa/isareg.h usr/src/sys/dev/isa/isareg.h
 --- usr_orig/src/sys/dev/isa/isareg.h	2005-12-11 07:22:02.000000000 -0500
 +++ usr/src/sys/dev/isa/isareg.h	2017-08-06 16:30:10.000000000 -0400
 @@ -55,7 +55,7 @@
  #define	IO_PPI		0x061		/* Programmable Peripheral Interface */
  #define	IO_RTC		0x070		/* RTC */
  #define	IO_NMI		IO_RTC		/* NMI Control */
 -#define	IO_DMAPG	0x080		/* DMA Page Registers */
 +#define	IO_DMAPG	0x081		/* DMA Page Registers */
  #define	IO_ICU2		0x0A0		/* 8259A Interrupt Controller #2 */
  #define	IO_DMA2		0x0C0		/* 8237A DMA Controller #2 */
  #define	IO_NPX		0x0F0		/* Numeric Coprocessor */

From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org, glitch@glitchwrks.com
Subject: re: kern/52468: ISA DMA incorrectly maps I/O port 0x80 for page address registers
Date: Tue, 08 Aug 2017 05:09:23 +1000

 thanks for looking into this.

 >  -static int dmapageport[2][4] = {
 >  -	{0x7, 0x3, 0x1, 0x2},
 >  -	{0xf, 0xb, 0x9, 0xa}
 >  +static const int dmapageport[2][4] = {
 >  +	{0x87, 0x83, 0x81, 0x82},
 >  +	{0x8f, 0x8b, 0x89, 0x8a}
 >   };
 [ ... ]
 >   		bus_space_write_1(ids->ids_bst, ids->ids_dmapgh,
 >  -		    dmapageport[0][ochan], (dmaaddr >> 16) & 0xff);
 >  +		    dmapageport[0][ochan] - IO_DMAPG, (dmaaddr >> 16) & 0xff);

 i would rather see this where dmapageport[] values are simply
 reduced by 1 and then bus_space accesses remain as-is.

 thanks!


 .mrg.

From: glitch <glitch@glitchwrks.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/52468: ISA DMA incorrectly maps I/O port 0x80 for page
 address registers
Date: Mon, 7 Aug 2017 18:22:50 -0400

 New patch implementing the offset change as just a decrement of the values in the dmapageport 2D array. Adjusted comments to reflect this.

 We were talking about what was best last night on IRC, I don't think anyone had a strong opinion one way or the other.

 diff -uNr usr_orig/src/sys/dev/isa/isadma.c usr/src/sys/dev/isa/isadma.c
 --- usr_orig/src/sys/dev/isa/isadma.c	2010-11-13 08:52:03.000000000 -0500
 +++ usr/src/sys/dev/isa/isadma.c	2017-08-07 18:19:33.083500767 -0400
 @@ -53,15 +53,19 @@
  struct isa_mem *isa_mem_head;

  /*
 - * High byte of DMA address is stored in this DMAPG register for
 - * the Nth DMA channel.
 + * DMA Channel to Address Page Register offset mapping
 + *
 + * Offset from IO_DMAPG is stored in this 2D array -- first dimension is
 + * the DMA controller, second dimension is the DMA channel.
 + *
 + * e.g. dmapageport[0][1] gives us the offset for DMA ch 1 on DMA1
   */
 -static int dmapageport[2][4] = {
 -	{0x7, 0x3, 0x1, 0x2},
 -	{0xf, 0xb, 0x9, 0xa}
 +static const int dmapageport[2][4] = {
 +	{0x6, 0x2, 0x0, 0x1},
 +	{0xe, 0xa, 0x8, 0x9}
  };

 -static u_int8_t dmamode[] = {
 +static const u_int8_t dmamode[] = {
  	/* write to device/read from device */
  	DMA37MD_READ | DMA37MD_SINGLE,
  	DMA37MD_WRITE | DMA37MD_SINGLE,
 @@ -169,7 +173,7 @@
  		if (bus_space_map(ids->ids_bst, IO_DMA2, DMA2_IOSIZE, 0,
  		    &ids->ids_dma2h))
  			panic("_isa_dmainit: unable to map DMA controller #2");
 -		if (bus_space_map(ids->ids_bst, IO_DMAPG, 0xf, 0,
 +		if (bus_space_map(ids->ids_bst, IO_DMAPG, DMAPG_IOSIZE, 0,
  		    &ids->ids_dmapgh))
  			panic("_isa_dmainit: unable to map DMA page registers");

 @@ -211,7 +215,7 @@
  	/*
  	 * Unmap the registers used by the ISA DMA controller.
  	 */
 -	bus_space_unmap(ids->ids_bst, ids->ids_dmapgh, 0xf);
 +	bus_space_unmap(ids->ids_bst, ids->ids_dmapgh, DMAPG_IOSIZE);
  	bus_space_unmap(ids->ids_bst, ids->ids_dma2h, DMA2_IOSIZE);
  	bus_space_unmap(ids->ids_bst, ids->ids_dma1h, DMA1_IOSIZE);

 diff -uNr usr_orig/src/sys/dev/isa/isadmareg.h usr/src/sys/dev/isa/isadmareg.h
 --- usr_orig/src/sys/dev/isa/isadmareg.h	2008-04-28 16:23:52.000000000 -0400
 +++ usr/src/sys/dev/isa/isadmareg.h	2017-08-06 17:43:47.508172970 -0400
 @@ -47,6 +47,10 @@
  #define	ISA_DMA_MAXSIZE_DEFAULT(chan)					\
  	(((chan) & 4) ? ISA_DMA_MAXSIZE_16BIT : ISA_DMA_MAXSIZE_8BIT)

 +/* DMA Page Address Registers size */
 +
 +#define	DMAPG_IOSIZE	(1*15)
 +
  /*
   * Register definitions for DMA controller 1 (channels 0..3):
   */
 diff -uNr usr_orig/src/sys/dev/isa/isareg.h usr/src/sys/dev/isa/isareg.h
 --- usr_orig/src/sys/dev/isa/isareg.h	2005-12-11 07:22:02.000000000 -0500
 +++ usr/src/sys/dev/isa/isareg.h	2017-08-06 16:30:10.000000000 -0400
 @@ -55,7 +55,7 @@
  #define	IO_PPI		0x061		/* Programmable Peripheral Interface */
  #define	IO_RTC		0x070		/* RTC */
  #define	IO_NMI		IO_RTC		/* NMI Control */
 -#define	IO_DMAPG	0x080		/* DMA Page Registers */
 +#define	IO_DMAPG	0x081		/* DMA Page Registers */
  #define	IO_ICU2		0x0A0		/* 8259A Interrupt Controller #2 */
  #define	IO_DMA2		0x0C0		/* 8237A DMA Controller #2 */
  #define	IO_NPX		0x0F0		/* Numeric Coprocessor */

From: glitch <glitch@glitchwrks.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/52468: ISA DMA incorrectly maps I/O port 0x80 for page
 address registers
Date: Wed, 3 Jan 2018 10:20:25 -0500

 Cleaning up the inbox and found this thread. Anything I need to do on this PR?

From: "Jaromir Dolecek" <jdolecek@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52468 CVS commit: src/sys/dev/isa
Date: Tue, 29 May 2018 06:14:34 +0000

 Module Name:	src
 Committed By:	jdolecek
 Date:		Tue May 29 06:14:34 UTC 2018

 Modified Files:
 	src/sys/dev/isa: isadma.c isadmareg.h isareg.h

 Log Message:
 fix off-by-one in the mapping of the ISA DMA page registers, they actually
 start at 0x81; the code used bus_space_map() starting from 0x80 but
 used +1 offset for actual I/O, now it maps starting 0x81 and does I/O
 without offset

 the reads and writes work exactly the same as before, but this frees
 0x80 for being mapped independantly

 patch provided in PR kern/52468 by Jonathan Chapman; checked against the spec
 and also FreeBSD sys/x86/isa/isa_dma.c


 To generate a diff of this commit:
 cvs rdiff -u -r1.66 -r1.67 src/sys/dev/isa/isadma.c
 cvs rdiff -u -r1.8 -r1.9 src/sys/dev/isa/isadmareg.h
 cvs rdiff -u -r1.9 -r1.10 src/sys/dev/isa/isareg.h

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

Responsible-Changed-From-To: kern-bug-people->jdolecek
Responsible-Changed-By: jdolecek@NetBSD.org
Responsible-Changed-When: Tue, 29 May 2018 06:22:26 +0000
Responsible-Changed-Why:
I'm taking care of this.


State-Changed-From-To: open->pending-pullups
State-Changed-By: jdolecek@NetBSD.org
State-Changed-When: Tue, 29 May 2018 06:22:26 +0000
State-Changed-Why:
I've committed the latest variant of the patch, I've finally got to review
the change. I've not tested it with any real ISA device, but the code change
is clear enough.
It's too late for 8.0 cycle for such change I think, but I'll ask for pullup,
so it can get to eventual 8.0.1 or 8.1.0.


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52468 CVS commit: [netbsd-8] src/sys/dev/isa
Date: Thu, 7 Jun 2018 19:39:54 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Thu Jun  7 19:39:54 UTC 2018

 Modified Files:
 	src/sys/dev/isa [netbsd-8]: isadma.c isadmareg.h isareg.h

 Log Message:
 Pull up following revision(s) (requested by jdolecek in ticket #841):

 	sys/dev/isa/isadmareg.h: revision 1.9
 	sys/dev/isa/isareg.h: revision 1.10
 	sys/dev/isa/isadma.c: revision 1.67

 fix off-by-one in the mapping of the ISA DMA page registers, they actually
 start at 0x81; the code used bus_space_map() starting from 0x80 but
 used +1 offset for actual I/O, now it maps starting 0x81 and does I/O
 without offset

 the reads and writes work exactly the same as before, but this frees
 0x80 for being mapped independantly
 patch provided in PR kern/52468 by Jonathan Chapman; checked against the spec
 and also FreeBSD sys/x86/isa/isa_dma.c


 To generate a diff of this commit:
 cvs rdiff -u -r1.66 -r1.66.52.1 src/sys/dev/isa/isadma.c
 cvs rdiff -u -r1.8 -r1.8.80.1 src/sys/dev/isa/isadmareg.h
 cvs rdiff -u -r1.9 -r1.9.156.1 src/sys/dev/isa/isareg.h

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

State-Changed-From-To: pending-pullups->closed
State-Changed-By: jdolecek@NetBSD.org
State-Changed-When: Sat, 16 Jun 2018 00:02:48 +0000
State-Changed-Why:
Pulled up to netbsd-8. Thanks for report.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.43 2018/01/16 07:36:43 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.