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

[Simon Burge <simonb%NetBSD.org@localhost>]

[ Not sure why part of John's reply got trimmed from this but
  is OK in gnats ]

John Nemeth wrote:

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

Using "return (ret)" fixes this for me - svwsata(4) attaches and
ahcisata(4) doesn't attempt to.

        svwsata0 at pci1 dev 14 function 0
        svwsata0: ServerWorks HT-1000 SATA Controller (rev. 0x00)
        svwsata0: using irq 11 for native-PCI interrupt
        atabus0 at svwsata0 channel 0
        atabus1 at svwsata0 channel 1
        atabus2 at svwsata0 channel 2
        atabus3 at svwsata0 channel 3
        ...
        svwsata0 port 0: device present, speed: 1.5Gb/s
        wd0 at atabus0 drive 0: <FUJITSU MHT2080BH>
        wd0: 76319 MB, 155061 cyl, 16 head, 63 sec, 512 bytes/sect x 156301488 
sectors
        svwsata0 port 1: PHY offline
        svwsata0 port 2: PHY offline
        svwsata0 port 3: PHY offline

Manuel - OK for John to commit that fix?

Cheers,
Simon.