* [Qemu-devel] [PATCH v5] PING: Fix ATA SMART and CHECK POWER MODE @ 2011-03-01 13:30 Brian Wheeler 2011-03-06 19:26 ` Aurelien Jarno 2011-03-09 4:26 ` Ryan Harper 0 siblings, 2 replies; 6+ messages in thread From: Brian Wheeler @ 2011-03-01 13:30 UTC (permalink / raw) To: qemu-devel 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) { ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v5] PING: Fix ATA SMART and CHECK POWER MODE 2011-03-01 13:30 [Qemu-devel] [PATCH v5] PING: Fix ATA SMART and CHECK POWER MODE Brian Wheeler @ 2011-03-06 19:26 ` Aurelien Jarno 2011-03-07 8:28 ` Kevin Wolf 2011-03-09 4:26 ` Ryan Harper 1 sibling, 1 reply; 6+ messages in thread From: Aurelien Jarno @ 2011-03-06 19:26 UTC (permalink / raw) To: Brian Wheeler; +Cc: Kevin Wolf, qemu-devel 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) { > > > > > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v5] PING: Fix ATA SMART and CHECK POWER MODE 2011-03-06 19:26 ` Aurelien Jarno @ 2011-03-07 8:28 ` Kevin Wolf 2011-03-07 9:51 ` Stefan Hajnoczi 0 siblings, 1 reply; 6+ messages in thread From: Kevin Wolf @ 2011-03-07 8:28 UTC (permalink / raw) To: Aurelien Jarno; +Cc: qemu-devel 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) { >> >> >> >> >> >> >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v5] PING: Fix ATA SMART and CHECK POWER MODE 2011-03-07 8:28 ` Kevin Wolf @ 2011-03-07 9:51 ` Stefan Hajnoczi 0 siblings, 0 replies; 6+ messages in thread From: Stefan Hajnoczi @ 2011-03-07 9:51 UTC (permalink / raw) To: Ryan Harper; +Cc: Kevin Wolf, qemu-devel, Aurelien Jarno 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) { ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v5] PING: Fix ATA SMART and CHECK POWER MODE 2011-03-01 13:30 [Qemu-devel] [PATCH v5] PING: Fix ATA SMART and CHECK POWER MODE Brian Wheeler 2011-03-06 19:26 ` Aurelien Jarno @ 2011-03-09 4:26 ` Ryan Harper 2011-03-09 9:32 ` Kevin Wolf 1 sibling, 1 reply; 6+ messages in thread From: Ryan Harper @ 2011-03-09 4:26 UTC (permalink / raw) To: Brian Wheeler; +Cc: Kevin Wolf, qemu-devel * 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) { > > > > > -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ryanh@us.ibm.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v5] PING: Fix ATA SMART and CHECK POWER MODE 2011-03-09 4:26 ` Ryan Harper @ 2011-03-09 9:32 ` Kevin Wolf 0 siblings, 0 replies; 6+ messages in thread From: Kevin Wolf @ 2011-03-09 9:32 UTC (permalink / raw) To: Ryan Harper; +Cc: qemu-devel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-09 9:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-01 13:30 [Qemu-devel] [PATCH v5] PING: Fix ATA SMART and CHECK POWER MODE Brian Wheeler 2011-03-06 19:26 ` Aurelien Jarno 2011-03-07 8:28 ` Kevin Wolf 2011-03-07 9:51 ` Stefan Hajnoczi 2011-03-09 4:26 ` Ryan Harper 2011-03-09 9:32 ` Kevin Wolf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).