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