NetBSD Problem Report #47389
From o.vd.linden@quicknet.nl Tue Jan 1 09:41:17 2013
Return-Path: <o.vd.linden@quicknet.nl>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
by www.NetBSD.org (Postfix) with ESMTP id C4D2A63E648
for <gnats-bugs@gnats.NetBSD.org>; Tue, 1 Jan 2013 09:41:16 +0000 (UTC)
Message-Id: <20130101094109.GA1972@sheep>
Date: Tue, 1 Jan 2013 10:41:09 +0100
From: Onno van der Linden <o.vd.linden@quicknet.nl>
To: gnats-bugs@gnats.NetBSD.org
Subject: lost interrupt messages with VT6421A and sata atapi device
>Number: 47389
>Category: kern
>Synopsis: lost interrupt messages with viaide(4) and sata atapi device
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: kern-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Tue Jan 01 09:45:01 +0000 2013
>Last-Modified: Mon Jan 07 16:55:04 +0000 2013
>Originator: Onno van der Linden
>Release: NetBSD 6.99.16
>Organization:
>Environment:
System: NetBSD sheep 6.99.16 NetBSD 6.99.16 (SHEEP) #38: Tue Jan 1 09:50:29 MET 2013 root@sheep:/usr/src/sys/arch/i386/compile/SHEEP i386
Architecture: i386
Machine: i386
>Description:
With cd0 an atapi drive attached to the sata port of a via VT6421A
scsictl cd0 identify
results in
viaide0:1:0: lost interrupt
type: atapi tc_bcount: 0 tc_skip: 74
>How-To-Repeat:
scsictl cd0 identify
>Fix:
Somehow the VT6421A doesn't like doing a PIO transfer with
both 32-bit and 16-bit I/O intructions as done in wdc_datain_pio
en wdc_dataout_pio. Fix it by checking the alignment
of the len parameter and do 32/16/8 - bit pio instructions all the way.
*** /sys/dev/pci/viaide.c.orig Mon Dec 31 20:38:58 2012
--- /sys/dev/pci/viaide.c Tue Jan 1 09:47:42 2013
***************
*** 58,63 ****
--- 58,68 ----
const struct pci_attach_args *);
static void via_setup_channel(struct ata_channel *);
+ static void via_datain_pio(struct ata_channel *chp,
+ int flags, void *bf, size_t len);
+ static void via_dataout_pio(struct ata_channel *chp,
+ int flags, void *bf, size_t len);
+
static int viaide_match(device_t, cfdata_t, void *);
static void viaide_attach(device_t, device_t, void *);
static const struct pciide_product_desc *
***************
*** 1118,1123 ****
--- 1123,1130 ----
sc->sc_wdcdev.sc_atac.atac_dma_cap = 2;
sc->sc_wdcdev.sc_atac.atac_udma_cap = 6;
}
+ sc->sc_wdcdev.datain_pio = via_datain_pio;
+ sc->sc_wdcdev.dataout_pio = via_dataout_pio;
sc->sc_wdcdev.sc_atac.atac_set_modes = sata_setup_channel;
sc->sc_wdcdev.sc_atac.atac_channels = sc->wdc_chanarray;
***************
*** 1211,1213 ****
--- 1218,1398 ----
wdcattach(wdc_cp);
}
}
+
+ static void
+ via_datain_pio(struct ata_channel *chp, int flags, void *bf, size_t len)
+ {
+ struct wdc_regs *wdr = CHAN_TO_WDC_REGS(chp);
+
+ #ifndef __NO_STRICT_ALIGNMENT
+ if ((uintptr_t)bf & 1)
+ goto unaligned;
+ if ((flags & ATA_DRIVE_CAP32) && ((uintptr_t)bf & 3))
+ goto unaligned;
+ #endif
+
+ if (flags & ATA_DRIVE_NOSTREAM) {
+ if ((flags & ATA_DRIVE_CAP32) && (len & 3) == 0) {
+ bus_space_read_multi_4(wdr->data32iot,
+ wdr->data32ioh, 0, bf, len >> 2);
+ }
+ else if ((len & 1) == 0) {
+ bus_space_read_multi_2(wdr->cmd_iot,
+ wdr->cmd_iohs[wd_data], 0, bf, len >> 1);
+ }
+ else {
+ bus_space_read_multi_1(wdr->cmd_iot,
+ wdr->cmd_iohs[wd_data], 0, bf, len);
+ }
+ } else {
+ if ((flags & ATA_DRIVE_CAP32) && (len & 3) == 0) {
+ bus_space_read_multi_stream_4(wdr->data32iot,
+ wdr->data32ioh, 0, bf, len >> 2);
+ }
+ else if ((len & 1) == 0) {
+ bus_space_read_multi_stream_2(wdr->cmd_iot,
+ wdr->cmd_iohs[wd_data], 0, bf, len >> 1);
+ }
+ else {
+ bus_space_read_multi_stream_1(wdr->cmd_iot,
+ wdr->cmd_iohs[wd_data], 0, bf, len);
+ }
+ }
+ return;
+
+ #ifndef __NO_STRICT_ALIGNMENT
+ unaligned:
+ if (flags & ATA_DRIVE_NOSTREAM) {
+ if ((flags & ATA_DRIVE_CAP32) && (len & 3) == 0) {
+ while (len > 3) {
+ uint32_t val;
+
+ val = bus_space_read_4(wdr->data32iot,
+ wdr->data32ioh, 0);
+ memcpy(bf, &val, 4);
+ bf = (char *)bf + 4;
+ len -= 4;
+ }
+ }
+ else {
+ while (len > 1) {
+ uint16_t val;
+
+ val = bus_space_read_2(wdr->cmd_iot,
+ wdr->cmd_iohs[wd_data], 0);
+ memcpy(bf, &val, 2);
+ bf = (char *)bf + 2;
+ len -= 2;
+ }
+ }
+ } else {
+ if ((flags & ATA_DRIVE_CAP32) && (len & 3) == 0) {
+ while (len > 3) {
+ uint32_t val;
+
+ val = bus_space_read_stream_4(wdr->data32iot,
+ wdr->data32ioh, 0);
+ memcpy(bf, &val, 4);
+ bf = (char *)bf + 4;
+ len -= 4;
+ }
+ }
+ else {
+ while (len > 1) {
+ uint16_t val;
+
+ val = bus_space_read_stream_2(wdr->cmd_iot,
+ wdr->cmd_iohs[wd_data], 0);
+ memcpy(bf, &val, 2);
+ bf = (char *)bf + 2;
+ len -= 2;
+ }
+ }
+ }
+ #endif
+ }
+
+ static void
+ via_dataout_pio(struct ata_channel *chp, int flags, void *bf, size_t len)
+ {
+ struct wdc_regs *wdr = CHAN_TO_WDC_REGS(chp);
+
+ #ifndef __NO_STRICT_ALIGNMENT
+ if ((uintptr_t)bf & 1)
+ goto unaligned;
+ if ((flags & ATA_DRIVE_CAP32) && ((uintptr_t)bf & 3))
+ goto unaligned;
+ #endif
+
+ if (flags & ATA_DRIVE_NOSTREAM) {
+ if ((flags & ATA_DRIVE_CAP32) && (len & 3) == 0) {
+ bus_space_write_multi_4(wdr->data32iot,
+ wdr->data32ioh, 0, bf, len >> 2);
+ }
+ else {
+ bus_space_write_multi_2(wdr->cmd_iot,
+ wdr->cmd_iohs[wd_data], 0, bf, len >> 1);
+ }
+ } else {
+ if ((flags & ATA_DRIVE_CAP32) && (len & 3) == 0) {
+ bus_space_write_multi_stream_4(wdr->data32iot,
+ wdr->data32ioh, 0, bf, len >> 2);
+ }
+ else {
+ bus_space_write_multi_stream_2(wdr->cmd_iot,
+ wdr->cmd_iohs[wd_data], 0, bf, len >> 1);
+ }
+ }
+ return;
+
+ #ifndef __NO_STRICT_ALIGNMENT
+ unaligned:
+ if (flags & ATA_DRIVE_NOSTREAM) {
+ if ((flags & ATA_DRIVE_CAP32) && (len & 3) == 0) {
+ while (len > 3) {
+ uint32_t val;
+
+ memcpy(&val, bf, 4);
+ bus_space_write_4(wdr->data32iot,
+ wdr->data32ioh, 0, val);
+ bf = (char *)bf + 4;
+ len -= 4;
+ }
+ }
+ else {
+ while (len > 1) {
+ uint16_t val;
+
+ memcpy(&val, bf, 2);
+ bus_space_write_2(wdr->cmd_iot,
+ wdr->cmd_iohs[wd_data], 0, val);
+ bf = (char *)bf + 2;
+ len -= 2;
+ }
+ }
+ } else {
+ if ((flags & ATA_DRIVE_CAP32) && (len & 3) == 0) {
+ while (len > 3) {
+ uint32_t val;
+
+ memcpy(&val, bf, 4);
+ bus_space_write_stream_4(wdr->data32iot,
+ wdr->data32ioh, 0, val);
+ bf = (char *)bf + 4;
+ len -= 4;
+ }
+ }
+ else {
+ while (len > 1) {
+ uint16_t val;
+
+ memcpy(&val, bf, 2);
+ bus_space_write_stream_2(wdr->cmd_iot,
+ wdr->cmd_iohs[wd_data], 0, val);
+ bf = (char *)bf + 2;
+ len -= 2;
+ }
+ }
+ }
+ #endif
+ }
>Audit-Trail:
From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/47389: lost interrupt messages with VT6421A and sata atapi device
Date: Tue, 1 Jan 2013 15:58:35 +0000
On Tue, Jan 01, 2013 at 09:45:01AM +0000, Onno van der Linden wrote:
> >Number: 47389
> >Category: kern
> >Synopsis: lost interrupt messages with viaide(4) and sata atapi device
...
> Somehow the VT6421A doesn't like doing a PIO transfer with
> both 32-bit and 16-bit I/O intructions as done in wdc_datain_pio
> en wdc_dataout_pio. Fix it by checking the alignment
> of the len parameter and do 32/16/8 - bit pio instructions all the way.
Might be worth using a new ATA_DRIVE_xxx flag and slightly
modifying the tests in wdc_datain/out_pio() rather than
replicating the code.
David
--
David Laight: david@l8s.co.uk
From: Onno van der Linden <o.vd.linden@quicknet.nl>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/47389: lost interrupt messages with VT6421A and sata atapi device
Date: Tue, 1 Jan 2013 17:04:21 +0100
On Tue, Jan 01, 2013 at 03:45:03PM +0000, David Laight wrote:
> Might be worth using a new ATA_DRIVE_xxx flag and slightly
> modifying the tests in wdc_datain/out_pio() rather than
> replicating the code.
Or not use an extra flag at all and change wdc_datain/out_pio()
to look like the via_data{in,out}_pio versions. Most of the time
len is a multiple of 4 anyway and if it isn't, it should be so
small (I've seen 34 bytes with a mount_cd9660 and 74 bytes
with the scsictl identify) to not make much of a performance
difference because 32-bit transfers aren't used in those case.
Onno
From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@NetBSD.org, gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org,
Onno van der Linden <o.vd.linden@quicknet.nl>
Subject: Re: kern/47389: lost interrupt messages with VT6421A and sata atapi
device
Date: Mon, 7 Jan 2013 17:53:17 +0100
On Tue, Jan 01, 2013 at 04:05:03PM +0000, Onno van der Linden wrote:
> [...]
> Or not use an extra flag at all and change wdc_datain/out_pio()
> to look like the via_data{in,out}_pio versions. Most of the time
> len is a multiple of 4 anyway and if it isn't, it should be so
> small (I've seen 34 bytes with a mount_cd9660 and 74 bytes
> with the scsictl identify) to not make much of a performance
> difference because 32-bit transfers aren't used in those case.
yes, that's probably the best way.
--
Manuel Bouyer <bouyer@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.