Fix ATA SMART and CHECK POWER MODE

This patch fixes two things:

1) CHECK POWER MODE

The error return value wasn't always zero, so it would show up as
offline.  Error is now explicitly set to zero.

2) SMART

The smart values that were returned were invalid and tools like skdump
would not recognize that the smart data was actually valid and would
dump weird output.  The data has been fixed up and raw value support was
added.  Tools like skdump and palimpsest work as expected.

Signed-of-by: Brian Wheeler <bdwheele@indiana.edu>
---

* Brian Wheeler <bdwheele@indiana.edu> [2011-02-07 16:48]:
> This patch fixes two things:
> 
> 1) CHECK POWER MODE
> 
> The error return value wasn't always zero, so it would show up as
> offline.  Error is now explicitly set to zero.
> 
> 2) SMART
> 
> The smart values that were returned were invalid and tools like skdump
> would not recognize that the smart data was actually valid and would
> dump weird output.  The data has been fixed up and raw value support was
> added.  Tools like skdump and palimpsest work as expected.
> 
> Signed-of-by: Brian Wheeler <bdwheele@indiana.edu>
> ---
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index dd63664..4d4ccfa 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -34,15 +34,37 @@
> 
>  #include <hw/ide/internal.h>
> 
> -static const int smart_attributes[][5] = {
> -    /* id,  flags, val, wrst, thrsh */
> -    { 0x01, 0x03, 0x64, 0x64, 0x06}, /* raw read */
> -    { 0x03, 0x03, 0x64, 0x64, 0x46}, /* spin up */
> -    { 0x04, 0x02, 0x64, 0x64, 0x14}, /* start stop count */
> -    { 0x05, 0x03, 0x64, 0x64, 0x36}, /* remapped sectors */
> -    { 0x00, 0x00, 0x00, 0x00, 0x00}
> +/* These values were taking from a running system. */
> +static const int smart_attributes[][12] = {
> +    /* id,  flags, hflags, val, wrst, raw (up to 6 bytes) */
> +    /* raw read error rate*/
> +    { 0x01, 0x03, 0x00, 0x74, 0x63, 0x31, 0x6d, 0x3f, 0x0d},
> +    /* spin up */
> +    { 0x03, 0x03, 0x00, 0x61, 0x61},
> +    /* start stop count */
> +    { 0x04, 0x02, 0x00, 0x64, 0x64, 0x64},
> +    /* remapped sectors */
> +    { 0x05, 0x03, 0x00, 0x64, 0x64},
> +    /* power on hours */
> +    { 0x09, 0x03, 0x00, 0x61, 0x61, 0x68, 0x0a},
> +    /* power cycle count */
> +    { 0x0c, 0x03, 0x00, 0x64, 0x64, 0x32},
> +    /* airflow-temperature-celsius */
> +    { 190,  0x03, 0x00, 0x64, 0x64, 0x1f, 0x00, 0x16, 0x22},
> +    { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
>  };
> 
> +static const int smart_thresholds[][2] = {
> +    { 0x01, 0x06},
> +    { 0x03, 0x00},
> +    { 0x04, 0x14},
> +    { 0x05, 0x24},
> +    { 0x09, 0x00},
> +    { 190,  0x32},
> +    { 0x00, 0x00}
> +};

Are these values from a specification or what?  WOuld be good to
document where we get the values that are being used.  If you are
selecting some defaults, what those values are and why.

> +
> +
>  /* XXX: DVDs that could fit on a CD will be reported as a CD */
>  static inline int media_present(IDEState *s)
>  {
> @@ -1843,6 +1865,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>          break;
>      case WIN_CHECKPOWERMODE1:
G>      case WIN_CHECKPOWERMODE2:
> +        s->error = 0;
>          s->nsector = 0xff; /* device active or idle */
>          s->status = READY_STAT | SEEK_STAT;
>          ide_set_irq(s->bus);
> @@ -2068,24 +2091,24 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>  	case SMART_ATTR_AUTOSAVE:
>  		switch (s->sector) {
>  		case 0x00:
> -		s->smart_autosave = 0;
> -		break;
> +		    s->smart_autosave = 0;
> +		    break;
>  		case 0xf1:
> -		s->smart_autosave = 1;
> -		break;
> +		    s->smart_autosave = 1;
> +		    break;
>  		default:
> -		goto abort_cmd;
> +		    goto abort_cmd;

Did you edit fix these up automatically? tabs/spaces?

>  		}
>  		s->status = READY_STAT | SEEK_STAT;
>  		ide_set_irq(s->bus);
>  		break;
>  	case SMART_STATUS:
>  		if (!s->smart_errors) {
> -		s->hcyl = 0xc2;
> -		s->lcyl = 0x4f;
> +		    s->hcyl = 0xc2;
> +		    s->lcyl = 0x4f;
>  		} else {
> -		s->hcyl = 0x2c;
> -		s->lcyl = 0xf4;
> +		    s->hcyl = 0x2c;
> +		    s->lcyl = 0xf4;

same

>  		}
>  		s->status = READY_STAT | SEEK_STAT;
>  		ide_set_irq(s->bus);
> @@ -2094,13 +2117,13 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>  		memset(s->io_buffer, 0, 0x200);
>  		s->io_buffer[0] = 0x01; /* smart struct version */
>  		for (n=0; n<30; n++) {
> -		if (smart_attributes[n][0] == 0)
> +		    if (smart_attributes[n][0] == 0)
>  			break;

If you are going to touch the if, then CODING_STYLE wants braced ifs.

   if () {
   }

> -		s->io_buffer[2+0+(n*12)] = smart_attributes[n][0];
> -		s->io_buffer[2+1+(n*12)] = smart_attributes[n][4];
> +		    s->io_buffer[2+0+(n*12)] = smart_thresholds[n][0];
> +		    s->io_buffer[2+1+(n*12)] = smart_thresholds[n][1];
>  		}
>  		for (n=0; n<511; n++) /* checksum */
> -		s->io_buffer[511] += s->io_buffer[n];
> +		    s->io_buffer[511] += s->io_buffer[n];
>  		s->io_buffer[511] = 0x100 - s->io_buffer[511];
>  		s->status = READY_STAT | SEEK_STAT;
>  		ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
> @@ -2110,21 +2133,22 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>  		memset(s->io_buffer, 0, 0x200);
>  		s->io_buffer[0] = 0x01; /* smart struct version */
>  		for (n=0; n<30; n++) {
> -		if (smart_attributes[n][0] == 0)
> +		    if (smart_attributes[n][0] == 0)

CODING_STYLE if()

>  			break;
> -		s->io_buffer[2+0+(n*12)] = smart_attributes[n][0];
> -		s->io_buffer[2+1+(n*12)] = smart_attributes[n][1];
> -		s->io_buffer[2+3+(n*12)] = smart_attributes[n][2];
> -		s->io_buffer[2+4+(n*12)] = smart_attributes[n][3];
> +		    int i;
> +		    for(i = 0; i < 12; i++) {
> +			s->io_buffer[2+i+(n*12)] = smart_attributes[n][i];
> +		    }
>  		}
> +

   whitespace

>  		s->io_buffer[362] = 0x02 | (s->smart_autosave?0x80:0x00);
>  		if (s->smart_selftest_count == 0) {
> -		s->io_buffer[363] = 0;
> +		    s->io_buffer[363] = 0;
>  		} else {
> -		s->io_buffer[363] =
> +		    s->io_buffer[363] =
>  			s->smart_selftest_data[3 + 
> -					   (s->smart_selftest_count - 1) *
> -					   24];
> +					       (s->smart_selftest_count - 1) *
> +					       24];
>  		}
>  		s->io_buffer[364] = 0x20; 
>  		s->io_buffer[365] = 0x01; 
> @@ -2136,9 +2160,9 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>  		s->io_buffer[372] = 0x02; /* minutes for poll short test */
>  		s->io_buffer[373] = 0x36; /* minutes for poll ext test */
>  		s->io_buffer[374] = 0x01; /* minutes for poll conveyance */
> -
> -		for (n=0; n<511; n++) 
> -		s->io_buffer[511] += s->io_buffer[n];
> +		for (n=0; n<511; n++) { /* checksum */
> +		    s->io_buffer[511] += s->io_buffer[n];
> +		}
>  		s->io_buffer[511] = 0x100 - s->io_buffer[511];
>  		s->status = READY_STAT | SEEK_STAT;
>  		ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
> @@ -2147,32 +2171,31 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>  	case SMART_READ_LOG:
>  		switch (s->sector) {
>  		case 0x01: /* summary smart error log */
> -		memset(s->io_buffer, 0, 0x200);
> -		s->io_buffer[0] = 0x01;
> -		s->io_buffer[1] = 0x00; /* no error entries */
> -		s->io_buffer[452] = s->smart_errors & 0xff;
> -		s->io_buffer[453] = (s->smart_errors & 0xff00) >> 8;
> -
> -		for (n=0; n<511; n++)
> +		    memset(s->io_buffer, 0, 0x200);
> +		    s->io_buffer[0] = 0x01;
> +		    s->io_buffer[1] = 0x00; /* no error entries */
> +		    s->io_buffer[452] = s->smart_errors & 0xff;
> +		    s->io_buffer[453] = (s->smart_errors & 0xff00) >> 8;
> +		    for (n=0; n<511; n++)

   brace

>  			s->io_buffer[511] += s->io_buffer[n];
> -		s->io_buffer[511] = 0x100 - s->io_buffer[511];
> -		break;
> +		    s->io_buffer[511] = 0x100 - s->io_buffer[511];
> +		    break;
>  		case 0x06: /* smart self test log */
> -		memset(s->io_buffer, 0, 0x200);
> -		s->io_buffer[0] = 0x01;
> -		if (s->smart_selftest_count == 0) {
> +		    memset(s->io_buffer, 0, 0x200);
> +		    s->io_buffer[0] = 0x01;
> +		    if (s->smart_selftest_count == 0) {
>  			s->io_buffer[508] = 0;
> -		} else {
> +		    } else {
>  			s->io_buffer[508] = s->smart_selftest_count;
>  			for (n=2; n<506; n++) 
> -			s->io_buffer[n] = s->smart_selftest_data[n];
> -		}
> -		for (n=0; n<511; n++)
> +			    s->io_buffer[n] = s->smart_selftest_data[n];
> +		    }
> +		    for (n=0; n<511; n++)

   brace

>  			s->io_buffer[511] += s->io_buffer[n];
> -		s->io_buffer[511] = 0x100 - s->io_buffer[511];
> -		break;
> +		    s->io_buffer[511] = 0x100 - s->io_buffer[511];
> +		    break;
>  		default:
> -		goto abort_cmd;
> +		    goto abort_cmd;
>  		}
>  		s->status = READY_STAT | SEEK_STAT;
>  		ide_transfer_start(s, s->io_buffer, 0x200, ide_transfer_stop);
> 

This patchset looks a lot larger than it should since there is a lot of
indentation movement, it would be good to see a version that just
implemented the changes needed, which AFAICT are mainly the additional
attributes and limits.