diff mbox

[v5] PING: Fix ATA SMART and CHECK POWER MODE

Message ID 1298986223.419.0.camel@nibbler.dlib.indiana.edu
State New
Headers show

Commit Message

Brian Wheeler March 1, 2011, 1:30 p.m. UTC 
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.

v5 changes:  rebase
v4 changes:  incorporate changes from Ryan Harper
v3 changes:  don't reformat code I didn't change
v2 changes:  use single structure instead of one for thresholds and one
for data.

Signed-off-by: bdwheele@indiana.edu
----------------

Comments

Aurelien Jarno March 6, 2011, 7:26 p.m. UTC | #1 
You should Cc: the IDE maintainer (Kevin Wolf) so that this patch can 
get acked. Done with this mail.

On Tue, Mar 01, 2011 at 08:30:23AM -0500, Brian Wheeler wrote:
> 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.
> 
> v5 changes:  rebase
> v4 changes:  incorporate changes from Ryan Harper
> v3 changes:  don't reformat code I didn't change
> v2 changes:  use single structure instead of one for thresholds and one
> for data.
> 
> Signed-off-by: bdwheele@indiana.edu
> ----------------
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 9c91a49..1ffca56 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -34,13 +34,26 @@
>  
>  #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 based on a Seagate ST3500418AS but have been modified
> +   to make more sense in QEMU */
> +static const int smart_attributes[][12] = {
> +    /* id,  flags, hflags, val, wrst, raw (6 bytes), threshold */
> +    /* raw read error rate*/
> +    { 0x01, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x06},
> +    /* spin up */
> +    { 0x03, 0x03, 0x00, 0x64, 0x64, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
> +    /* start stop count */
> +    { 0x04, 0x02, 0x00, 0x64, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x14},
> +    /* remapped sectors */
> +    { 0x05, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x24},
> +    /* power on hours */
> +    { 0x09, 0x03, 0x00, 0x64, 0x64, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
> +    /* power cycle count */
> +    { 0x0c, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
> +    /* airflow-temperature-celsius */
> +    { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
> +    /* end of list */
> +    { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
>  };
>  
>  /* XXX: DVDs that could fit on a CD will be reported as a CD */
> @@ -1843,6 +1856,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>          break;
>      case WIN_CHECKPOWERMODE1:
>      case WIN_CHECKPOWERMODE2:
> +        s->error = 0;
>          s->nsector = 0xff; /* device active or idle */
>          s->status = READY_STAT | SEEK_STAT;
>          ide_set_irq(s->bus);
> @@ -2097,7 +2111,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>  		if (smart_attributes[n][0] == 0)
>  			break;
>  		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+1+(n*12)] = smart_attributes[n][11];
>  		}
>  		for (n=0; n<511; n++) /* checksum */
>  		s->io_buffer[511] += s->io_buffer[n];
> @@ -2110,12 +2124,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;
> -		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 < 11; i++) {
> +			s->io_buffer[2+i+(n*12)] = smart_attributes[n][i];
> +		    }
>  		}
>  		s->io_buffer[362] = 0x02 | (s->smart_autosave?0x80:0x00);
>  		if (s->smart_selftest_count == 0) {
> 
> 
> 
> 
> 
> 
>
Kevin Wolf March 7, 2011, 8:28 a.m. UTC | #2 
Am 06.03.2011 20:26, schrieb Aurelien Jarno:
> You should Cc: the IDE maintainer (Kevin Wolf) so that this patch can 
> get acked. Done with this mail.

Thanks. In fact, I saw the patch, but it still needs a review. Can
whoever commented on the previous versions give it the review and post
an Acked-by? If not, I'll try to get to it myself.

Kevin

> 
> On Tue, Mar 01, 2011 at 08:30:23AM -0500, Brian Wheeler wrote:
>> 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.
>>
>> v5 changes:  rebase
>> v4 changes:  incorporate changes from Ryan Harper
>> v3 changes:  don't reformat code I didn't change
>> v2 changes:  use single structure instead of one for thresholds and one
>> for data.
>>
>> Signed-off-by: bdwheele@indiana.edu
>> ----------------
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index 9c91a49..1ffca56 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -34,13 +34,26 @@
>>  
>>  #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 based on a Seagate ST3500418AS but have been modified
>> +   to make more sense in QEMU */
>> +static const int smart_attributes[][12] = {
>> +    /* id,  flags, hflags, val, wrst, raw (6 bytes), threshold */
>> +    /* raw read error rate*/
>> +    { 0x01, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x06},
>> +    /* spin up */
>> +    { 0x03, 0x03, 0x00, 0x64, 0x64, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
>> +    /* start stop count */
>> +    { 0x04, 0x02, 0x00, 0x64, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x14},
>> +    /* remapped sectors */
>> +    { 0x05, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x24},
>> +    /* power on hours */
>> +    { 0x09, 0x03, 0x00, 0x64, 0x64, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
>> +    /* power cycle count */
>> +    { 0x0c, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
>> +    /* airflow-temperature-celsius */
>> +    { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
>> +    /* end of list */
>> +    { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
>>  };
>>  
>>  /* XXX: DVDs that could fit on a CD will be reported as a CD */
>> @@ -1843,6 +1856,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>>          break;
>>      case WIN_CHECKPOWERMODE1:
>>      case WIN_CHECKPOWERMODE2:
>> +        s->error = 0;
>>          s->nsector = 0xff; /* device active or idle */
>>          s->status = READY_STAT | SEEK_STAT;
>>          ide_set_irq(s->bus);
>> @@ -2097,7 +2111,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>>  		if (smart_attributes[n][0] == 0)
>>  			break;
>>  		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+1+(n*12)] = smart_attributes[n][11];
>>  		}
>>  		for (n=0; n<511; n++) /* checksum */
>>  		s->io_buffer[511] += s->io_buffer[n];
>> @@ -2110,12 +2124,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;
>> -		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 < 11; i++) {
>> +			s->io_buffer[2+i+(n*12)] = smart_attributes[n][i];
>> +		    }
>>  		}
>>  		s->io_buffer[362] = 0x02 | (s->smart_autosave?0x80:0x00);
>>  		if (s->smart_selftest_count == 0) {
>>
>>
>>
>>
>>
>>
>>
>
Stefan Hajnoczi March 7, 2011, 9:51 a.m. UTC | #3 
On Mon, Mar 7, 2011 at 8:28 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 06.03.2011 20:26, schrieb Aurelien Jarno:
>> You should Cc: the IDE maintainer (Kevin Wolf) so that this patch can
>> get acked. Done with this mail.
>
> Thanks. In fact, I saw the patch, but it still needs a review. Can
> whoever commented on the previous versions give it the review and post
> an Acked-by? If not, I'll try to get to it myself.

I think it was Ryan.  Original email below:

>> On Tue, Mar 01, 2011 at 08:30:23AM -0500, Brian Wheeler wrote:
>>> 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.
>>>
>>> v5 changes:  rebase
>>> v4 changes:  incorporate changes from Ryan Harper
>>> v3 changes:  don't reformat code I didn't change
>>> v2 changes:  use single structure instead of one for thresholds and one
>>> for data.
>>>
>>> Signed-off-by: bdwheele@indiana.edu
>>> ----------------
>>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>>> index 9c91a49..1ffca56 100644
>>> --- a/hw/ide/core.c
>>> +++ b/hw/ide/core.c
>>> @@ -34,13 +34,26 @@
>>>
>>>  #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 based on a Seagate ST3500418AS but have been modified
>>> +   to make more sense in QEMU */
>>> +static const int smart_attributes[][12] = {
>>> +    /* id,  flags, hflags, val, wrst, raw (6 bytes), threshold */
>>> +    /* raw read error rate*/
>>> +    { 0x01, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x06},
>>> +    /* spin up */
>>> +    { 0x03, 0x03, 0x00, 0x64, 0x64, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
>>> +    /* start stop count */
>>> +    { 0x04, 0x02, 0x00, 0x64, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x14},
>>> +    /* remapped sectors */
>>> +    { 0x05, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x24},
>>> +    /* power on hours */
>>> +    { 0x09, 0x03, 0x00, 0x64, 0x64, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
>>> +    /* power cycle count */
>>> +    { 0x0c, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
>>> +    /* airflow-temperature-celsius */
>>> +    { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
>>> +    /* end of list */
>>> +    { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
>>>  };
>>>
>>>  /* XXX: DVDs that could fit on a CD will be reported as a CD */
>>> @@ -1843,6 +1856,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>>>          break;
>>>      case WIN_CHECKPOWERMODE1:
>>>      case WIN_CHECKPOWERMODE2:
>>> +        s->error = 0;
>>>          s->nsector = 0xff; /* device active or idle */
>>>          s->status = READY_STAT | SEEK_STAT;
>>>          ide_set_irq(s->bus);
>>> @@ -2097,7 +2111,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>>>              if (smart_attributes[n][0] == 0)
>>>                      break;
>>>              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+1+(n*12)] = smart_attributes[n][11];
>>>              }
>>>              for (n=0; n<511; n++) /* checksum */
>>>              s->io_buffer[511] += s->io_buffer[n];
>>> @@ -2110,12 +2124,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;
>>> -            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 < 11; i++) {
>>> +                    s->io_buffer[2+i+(n*12)] = smart_attributes[n][i];
>>> +                }
>>>              }
>>>              s->io_buffer[362] = 0x02 | (s->smart_autosave?0x80:0x00);
>>>              if (s->smart_selftest_count == 0) {
Ryan Harper March 9, 2011, 4:26 a.m. UTC | #4 
* Brian Wheeler <bdwheele@indiana.edu> [2011-03-01 07:35]:
> 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.
> 
> v5 changes:  rebase
> v4 changes:  incorporate changes from Ryan Harper
> v3 changes:  don't reformat code I didn't change
> v2 changes:  use single structure instead of one for thresholds and one
> for data.
> 

Sorry, haven't had a chance to catch up on qemu-devel, meant to respond
sooner.  Changes look good.

Acked-by: Ryan Harper <ryanh@us.ibm.com>

> Signed-off-by: bdwheele@indiana.edu
> ----------------
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 9c91a49..1ffca56 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -34,13 +34,26 @@
> 
>  #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 based on a Seagate ST3500418AS but have been modified
> +   to make more sense in QEMU */
> +static const int smart_attributes[][12] = {
> +    /* id,  flags, hflags, val, wrst, raw (6 bytes), threshold */
> +    /* raw read error rate*/
> +    { 0x01, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x06},
> +    /* spin up */
> +    { 0x03, 0x03, 0x00, 0x64, 0x64, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
> +    /* start stop count */
> +    { 0x04, 0x02, 0x00, 0x64, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x14},
> +    /* remapped sectors */
> +    { 0x05, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x24},
> +    /* power on hours */
> +    { 0x09, 0x03, 0x00, 0x64, 0x64, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
> +    /* power cycle count */
> +    { 0x0c, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
> +    /* airflow-temperature-celsius */
> +    { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
> +    /* end of list */
> +    { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
>  };
> 
>  /* XXX: DVDs that could fit on a CD will be reported as a CD */
> @@ -1843,6 +1856,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>          break;
>      case WIN_CHECKPOWERMODE1:
>      case WIN_CHECKPOWERMODE2:
> +        s->error = 0;
>          s->nsector = 0xff; /* device active or idle */
>          s->status = READY_STAT | SEEK_STAT;
>          ide_set_irq(s->bus);
> @@ -2097,7 +2111,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>  		if (smart_attributes[n][0] == 0)
>  			break;
>  		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+1+(n*12)] = smart_attributes[n][11];
>  		}
>  		for (n=0; n<511; n++) /* checksum */
>  		s->io_buffer[511] += s->io_buffer[n];
> @@ -2110,12 +2124,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;
> -		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 < 11; i++) {
> +			s->io_buffer[2+i+(n*12)] = smart_attributes[n][i];
> +		    }
>  		}
>  		s->io_buffer[362] = 0x02 | (s->smart_autosave?0x80:0x00);
>  		if (s->smart_selftest_count == 0) {
> 
> 
> 
> 
>
Kevin Wolf March 9, 2011, 9:32 a.m. UTC | #5 
Am 09.03.2011 05:26, schrieb Ryan Harper:
> * Brian Wheeler <bdwheele@indiana.edu> [2011-03-01 07:35]:
>> 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.
>>
>> v5 changes:  rebase
>> v4 changes:  incorporate changes from Ryan Harper
>> v3 changes:  don't reformat code I didn't change
>> v2 changes:  use single structure instead of one for thresholds and one
>> for data.
>>
> 
> Sorry, haven't had a chance to catch up on qemu-devel, meant to respond
> sooner.  Changes look good.
> 
> Acked-by: Ryan Harper <ryanh@us.ibm.com>

Thanks, applied to the block branch.

Kevin
diff mbox

Patch

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 9c91a49..1ffca56 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -34,13 +34,26 @@ 
 
 #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 based on a Seagate ST3500418AS but have been modified
+   to make more sense in QEMU */
+static const int smart_attributes[][12] = {
+    /* id,  flags, hflags, val, wrst, raw (6 bytes), threshold */
+    /* raw read error rate*/
+    { 0x01, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x06},
+    /* spin up */
+    { 0x03, 0x03, 0x00, 0x64, 0x64, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+    /* start stop count */
+    { 0x04, 0x02, 0x00, 0x64, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x14},
+    /* remapped sectors */
+    { 0x05, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x24},
+    /* power on hours */
+    { 0x09, 0x03, 0x00, 0x64, 0x64, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+    /* power cycle count */
+    { 0x0c, 0x03, 0x00, 0x64, 0x64, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
+    /* airflow-temperature-celsius */
+    { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
+    /* end of list */
+    { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
 };
 
 /* XXX: DVDs that could fit on a CD will be reported as a CD */
@@ -1843,6 +1856,7 @@  void ide_exec_cmd(IDEBus *bus, uint32_t val)
         break;
     case WIN_CHECKPOWERMODE1:
     case WIN_CHECKPOWERMODE2:
+        s->error = 0;
         s->nsector = 0xff; /* device active or idle */
         s->status = READY_STAT | SEEK_STAT;
         ide_set_irq(s->bus);
@@ -2097,7 +2111,7 @@  void ide_exec_cmd(IDEBus *bus, uint32_t val)
 		if (smart_attributes[n][0] == 0)
 			break;
 		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+1+(n*12)] = smart_attributes[n][11];
 		}
 		for (n=0; n<511; n++) /* checksum */
 		s->io_buffer[511] += s->io_buffer[n];
@@ -2110,12 +2124,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;
-		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 < 11; i++) {
+			s->io_buffer[2+i+(n*12)] = smart_attributes[n][i];
+		    }
 		}
 		s->io_buffer[362] = 0x02 | (s->smart_autosave?0x80:0x00);
 		if (s->smart_selftest_count == 0) {