Re: kern/38056: ahcisata fails to attach with a ServerWorks HT-1000 SATA

[jnemeth%victoria.tc.ca@localhost (John Nemeth)]

The following reply was made to PR kern/38056; it has been noted by GNATS.

From: jnemeth%victoria.tc.ca@localhost (John Nemeth)
To: Simon Burge <simonb%NetBSD.org@localhost>, Manuel Bouyer 
<bouyer%antioche.eu.org@localhost>
Cc: gnats-bugs%NetBSD.org@localhost, kern-bug-people%NetBSD.org@localhost, 
netbsd-bugs%NetBSD.org@localhost
Subject: Re: kern/38056: ahcisata fails to attach with a ServerWorks HT-1000 
SATA
Date: Mon, 18 Feb 2008 19:33:13 -0800

 On Jul 12,  7:09am, Simon Burge wrote:
 } Manuel Bouyer wrote:
 }
 } > I suspect it's a side effect of support for AHCI RAID controllers.
 } > What does pcidump say for this device ?
 } >
 } > ahcisata should probably not match any controller announcing a RAID 
subclass
 .
 } > We should use a table of compatible devices for RAID class.
 
      I think I may have found the bug.
 sys/dev/pci/ahcisata_pci:ahci_pci_match() looks like this:
 
         int ret = 0;
 
         if (PCI_CLASS(pa->pa_class) == PCI_CLASS_MASS_STORAGE &&
             ((PCI_SUBCLASS(pa->pa_class) == PCI_SUBCLASS_MASS_STORAGE_SATA &&
              PCI_INTERFACE(pa->pa_class) == PCI_INTERFACE_SATA_AHCI) ||
              PCI_SUBCLASS(pa->pa_class) == PCI_SUBCLASS_MASS_STORAGE_RAID)) {
                 /* check if the chip is in ahci mode */
                 if (pci_mapreg_map(pa, AHCI_PCI_ABAR,
                     PCI_MAPREG_TYPE_MEM | PCI_MAPREG_MEM_TYPE_32BIT, 0,
                     &regt, &regh, NULL, &size) != 0)
                         return (0);
                 if (bus_space_read_4(regt, regh, AHCI_GHC) & AHCI_GHC_AE)
                         ret = 3;
                 bus_space_unmap(regt, regh, size);
                 return (3);
         }
 
         return (ret);
 
      Notice that after bus_space_read_4(), it does bus_space_unmap()
 followed by return (3) regardless of what the AHCI_GHC register
 contained.  I think that "return (3)" should be "return (ret)", that
 way it will only return 3 if AHCI_GHC & AHCI_GHC_AE != 0.  This bug has
 been there since rev. 1.1, but would have been uncovered by rev. 1.3 on
 Feb. 11th, which added the check against
 PCI_SUBCLASS_MASS_STORAGE_RAID.
 
 }-- End of excerpt from Simon Burge