Old ICH7 SATA-2 question

Jeremy Chadwick jdc at koitsu.org
Sun Feb 24 03:09:29 UTC 2013


On Sat, Feb 23, 2013 at 04:13:07PM -0800, Jeremy Chadwick wrote:
> On Sun, Feb 24, 2013 at 02:28:08AM +0400, Michael BlackHeart wrote:
> > 2013/2/24 Jeremy Chadwick <jdc at koitsu.org>:
> >
> > {snipping irrelevant stuff and fixing formatting}
> >
> > atapci1 at pci0:0:31:2:    class=0x01018f card=0x26011043 chip=0x27c08086 rev=0x01 hdr=0x00
> >     vendor     = 'Intel Corporation'
> >     device     = 'N10/ICH7 Family SATA IDE Controller'
> >
> > {snip}
> 
> I had written up a significantly longer reply to this Email, but once I
> finished and went back reviewing the information provided, my memory
> recalled having this exact conversation some time ago.  After some
> extensive digging, I found it -- circa late 2008:
> 
> http://lists.freebsd.org/pipermail/freebsd-stable/2008-October/046306.html
> 
> The result of this conversation was that FreeBSD at the time -- this
> would have been probably FreeBSD 8.0 or 8.1 -- contained a bug:
> ata_chipset.c (part of the classic ata(4) driver) was misidentifying the
> different revisions of ICH7 and therefore limiting controller capacities
> incorrectly.
> 
> Possibly a regression has been introduced as a result of the ATA-via-CAM
> migration (now the default), which uses a completely different set of
> code.

CC'ing mav@ because I need some help/clarification on this one.

I received an off-list mail from another ICH7 user, particularly one who
is using an SSD.  Their controller is identical (same vendor/device ID),
and all their devices also claim "SATA" as well as "150MBytes/sec".

However, "diskinfo -t" on the individuals' SSD shows quite clearly rates
of up to 200MBytes/second.

So the issue appears to be cosmetic.  The question is why.

Let me clarify because some list members have already gotten confused
about some of the information output by the kernel and utilities.  I'm
going to use my own system (different controller, etc.) to educate list
members.  The fact I'm using AHCI has no bearing on this educational
section, let me make that clear!

* The following output during boot reflects the results of the ATA
IDENTIFY command, and indicates what the **disk itself** (or more
specifically, the controller on the disk itself) claims it is capable
of:

	ada0 at ahcich0 bus 0 scbus0 target 0 lun 0
	ada0: <INTEL SSDSA2CW080G3 4PC10302> ATA-8 SATA 2.x device

This indicates the ada0 disk supports up to the SATA 2.x revision (i.e.
SATA300).  This DOES NOT indicate what the motherboard (or HBA) SATA
controllers' PHY/port is operating at; it only indicates what the disk
supports "up to".

* The subsequent output during boot reflects the actual motherboard (or
HBA) SATA controllers' PHY/port speed, including what has been
negotiated:

	ada0: 300.000MB/s transfers (SATA 2.x, UDMA6, PIO 8192bytes)

There's a 1:1 mapping between SATA revision and PHY speed, effectively,
unless otherwise throttled down or forced in some manner.  I'm reminded
of the non-ATA_CAM function ata_satarev2str() where the revision map is
like this:

value 0    = ""        (default speed value is used??  unsure)
value 1    = "SATA150" (150MB/sec)
value 2    = "SATA300" (300MB/sec)
value 3    = "SATA600" (600MB/sec)
value 0xff = "SATA"    (default speed value is used)
else       = "???"     (default speed value is used??  unsure)

Now for the rest...

I've taken a look at the code and it's very difficult for me to follow;
I'm not entirely sure, but it does look like pieces of
sys/dev/ata/chipsets are still used today.  I need mav@ to verify that
fact however, because if ata_intel_probe() isn't used any more, then
that might explain what's going on here (possibly).

The "negotiated speed" printing comes from sys/cam/ata/ata_xpt.c, in
ata_announce_periph().  The code logic here is simple, while the complex
bits (e.g. what sets CTS_SATA_VALID_REVISION) are elsewhere.

First, the speed:

2022         /* Report connection speed */
2023         speed = cpi.base_transfer_speed;
2024         if (cts.ccb_h.status == CAM_REQ_CMP && cts.transport == XPORT_ATA) {
2025                 struct  ccb_trans_settings_pata *pata =
2026                     &cts.xport_specific.ata;
2027
2028                 if (pata->valid & CTS_ATA_VALID_MODE)
2029                         speed = ata_mode2speed(pata->mode);
2030         }
2031         if (cts.ccb_h.status == CAM_REQ_CMP && cts.transport == XPORT_SATA) {
2032                 struct  ccb_trans_settings_sata *sata =
2033                     &cts.xport_specific.sata;
2034
2035                 if (sata->valid & CTS_SATA_VALID_REVISION)
2036                         speed = ata_revision2speed(sata->revision);
2037         }
2038         mb = speed / 1000;
2039         if (mb > 0)
2040                 printf("%s%d: %d.%03dMB/s transfers",
2041                        periph->periph_name, periph->unit_number,
2042                        mb, speed % 1000);

The if() statement that is being used in Michael's case is the one for
XPORT_SATA, not XPORT_PATA; that will be proven further below.  I then
had two questions:

1. Where does base_transfer_speed get set?

For SATA devices, it gets set in sys/dev/ata/ata-all.c (I think).  The
default value chosen is 150000:

1884                 if (ch->flags & ATA_SATA)
1885                         cpi->base_transfer_speed = 150000;
1886                 else
1887                         cpi->base_transfer_speed = 3300;

2. Where does CTS_SATA_VALID_REVISION get set, which can in effect
override base_transfer_speed?

The jury is still out on this one as you'll see.

Now on to the "protocol revision" printing code, i.e. "SATA 2.x" --
remember we're talking about the negotiated speed/protocol, not what's
returned from ATA IDENTIFY (e.g. "camcontrol identify") for the disk.

2060         if (cts.ccb_h.status == CAM_REQ_CMP && cts.transport == XPORT_SATA) {
2061                 struct ccb_trans_settings_sata *sata =
2062                     &cts.xport_specific.sata;
2063
2064                 printf(" (");
2065                 if (sata->valid & CTS_SATA_VALID_REVISION)
2066                         printf("SATA %d.x, ", sata->revision);
2067                 else
2068                         printf("SATA, ");
2069                 if (sata->valid & CTS_SATA_VALID_MODE)
2070                         printf("%s, ", ata_mode2string(sata->mode));
2071                 if ((sata->valid & CTS_ATA_VALID_ATAPI) && sata->atapi != 0)
2072                         printf("ATAPI %dbytes, ", sata->atapi);
2073                 if (sata->valid & CTS_SATA_VALID_BYTECOUNT)
2074                         printf("PIO %dbytes", sata->bytecount);
2075                 printf(")");
2076         }
2077         printf("\n");

Here we can see that XPORT_SATA must be set, because Michael's kernel
output clearly shows the above printf()s.

But once again we're back to CTS_SATA_VALID_REVISION.  Without
CTS_SATA_VALID_REVISION being set, ata_xpt.c chooses to simply say
"SATA".  That's all -- just "SATA".  And that is what Michael and others
with this chip see.

The question is, simply, why does this model of ICH7 result in the
bit CTS_SATA_VALID_REVISION, in the "valid" member of the appropriate
ccb_trans_settings_sata struct, not being set correctly.

I dug further than this, but I ended up chasing my tail while trying to
look at sys/dev/ata/ata-all.c in ataaction() to figure out what went on.
I pretty much lost faith (well, not faith, just lost the driving force)
when I tried to find ATA_GETREV -- it's generated during buildworld as
an inline function and comes from a DEVMETHOD() macro expansion, I
believe; look for ata_getrev instead.

As I've said many times in the past: the rabbit hole is deep and it is
very easy to get lost when you lack familiarity with this code.

-- 
| Jeremy Chadwick                                   jdc at koitsu.org |
| UNIX Systems Administrator                http://jdc.koitsu.org/ |
| Mountain View, CA, US                                            |
| Making life hard for others since 1977.             PGP 4BD6C0CB |


More information about the freebsd-stable mailing list