From owner-freebsd-stable@FreeBSD.ORG Tue Oct 21 00:38:53 2003 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id ADE0216A4B3 for ; Tue, 21 Oct 2003 00:38:53 -0700 (PDT) Received: from smtp.covadmail.net (mx05.covadmail.net [63.65.120.65]) by mx1.FreeBSD.org (Postfix) with SMTP id 46A0943FB1 for ; Tue, 21 Oct 2003 00:38:49 -0700 (PDT) (envelope-from strick@covad.net) Received: (covad.net 21219 invoked from network); 21 Oct 2003 07:38:44 -0000 Received: from unknown (HELO ice.nodomain) (68.165.100.44) by sun-qmail16 with SMTP; 21 Oct 2003 07:38:43 -0000 Received: from ice.nodomain (localhost [127.0.0.1]) by ice.nodomain (8.12.8p1/8.12.8) with ESMTP id h9L7cktk000812; Tue, 21 Oct 2003 00:38:47 -0700 (PDT) (envelope-from dan@ice.nodomain) Received: (from dan@localhost) by ice.nodomain (8.12.8p1/8.12.8/Submit) id h9L7ckYQ000811; Tue, 21 Oct 2003 00:38:46 -0700 (PDT) Date: Tue, 21 Oct 2003 00:38:46 -0700 (PDT) From: Dan Strick Message-Id: <200310210738.h9L7ckYQ000811@ice.nodomain> To: freebsd-stable@freebsd.org cc: dan@ice.nodomain cc: jhb@freefall.freebsd.org Subject: FreeBSD 4.9-RC3 and the ICH5 ATA controllers X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Oct 2003 07:38:53 -0000 On Sat, 18 Oct 2003, Murray Stokely wrote in freebsd-stable: > ... > release candidate (RC3) is now available for i386 > ... > The firewire module loading issue has been resolved, but the > SATA issue has not. John Baldwin has a patch posted at > http://people.freebsd.org/~jhb/patches/sata.patch > ... I fetched the SATA patch and examined it. It seems to be identical to the changes I suggested in my Oct 13 post to freebsd-stable (modulo the order of cases in switch statements) with one minor exception. I was wondering if the patch is based on my suggested changes. (I have received no feedback on my email to freebsd-stable.) If so, did someone test them or did you just "take my word" for the correctness of the changes? Is the patch likely to make it into release 4.9? I have discovered that on my system with the SATA controller configured for "legacy" mode and *without* the patch, RC3 fails to configure my CD drive for DMA even though I set hw.ata.atapi_dma=1 in /boot/loader.conf.local. The problem is that when the ICH5 SATA controller is configured in legacy mode, the ICH5 IDE controller disappears from PCI configuration space. Then the IDE devices appear to be connected to the SATA controller, which is an unknown controller if the patch is not installed. Thus some sort of patch is needed even if the SATA controller is configured in legacy mode. (I have not yet figured out how to discover the correct device configurations when the SATA controller is in legacy mode. The BIOS changes the subsystem device ID in the SATA function PCI configuration space, but I can't find any documentation on this and I don't understand how one should determine which ATA channel is really SATA and which is really PATA. Perhaps the information required for correct device configuration can be obtained with the ATA IDENTIFY commands. The problem may be beyond the scope of a quick bug fix for release 4.9. The patch as you have it now is probably the best fix for the moment. It may incorrectly report IDE devices as being attached to a SATA controller, but that is merely a cosmetic error. At least the devices are configured more or less correctly.) I was also wondering about the minor difference between the patch and my suggested changes. In function ata_pci_match() in ata-pci.c, the patch does: ATA_OUTB(ch->r_bmio, ATA_BMSTAT_PORT, dmastat | ATA_BMSTAT_INTERRUPT); where my code does: ATA_OUTB(ch->r_bmio, ATA_BMSTAT_PORT, dmastat & 0x7c); Setting the ATA_BMSTAT_INTERRUPT bit in dmastat is non functional since we know from context that the bit is already set. The ICH5 SATA Bus Master Status Register, offset ATA_BMSTAT_PORT in the bus master i/o control block, is described in section 11.2.2 of the Intel ICH5 datasheet. Bits 1, 2, 7 are R/WC. In order to clear just the ATA_BMSTAT_INTERRUPT (bit 2) and leave the other bits alone we should clear the other R/WC bits in the status register value before writing it back into the register (i.e. write back the value (dmastat & 0x7d)). Bit 0 happens to be RO for the ICH5 ATA controllers, so (dmastat & 0x7c) does the same thing, but on reconsidering the code I favor writing the value (dmastat & 0x7d). Using cpp symbols, that would be: (dmastat & ~(ATA_BMSTAT_DMA_SIMPLEX|ATA_BMSTAT_ERROR)) Just to add to the confusion, different PCI ATA controllers treat bit 7 differently. The ICH5 SATA controller uses it to say something obscure about the current/previous DMA operation. The bit is reserved for the ICH5 IDE/PATA controller. The so far unpublished INCITS T13 ATA/ATAPI Host Adapters Standard says the bit is normally RO and means two ATA channels cannot be simultaneously active (hence the cpp macro name ATA_BMSTAT_DMA_SIMPLEX) but some ATA vendors use it differently. I read somewhere that PCI-SIG also has a document that expresses an opinion on this bit, but I don't have access to PCI-SIG standards so I can't check that out. In any case, the above code would only be executed for the ICH5 ATA controllers, so only the ICH5 usage really matters here. The upshot is that the current version of the patch might clear the ATA_BMSTAT_DMA_SIMPLEX and ATA_BMSTAT_ERROR error bits inappropriately. The ATA_BMSTAT_DMA_SIMPLEX bit (which actually means something different for the ICH5) does not seem to be used by the ATA driver in a way that would make a difference. The driver might be using the ATA_BMSTAT_ERROR bit but I don't know if the patch interferes with this usage. It may be a waste to spend too much effort beating this to death since there may be a whole new ATA driver in FreeBSD CURRENT. Have I thoroughly confused you? Sorry about that... Dan Strick strick@covad.net