From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=38886 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PwVly-0007HU-LW for qemu-devel@nongnu.org; Mon, 07 Mar 2011 03:26:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PwVlv-0002nz-Si for qemu-devel@nongnu.org; Mon, 07 Mar 2011 03:26:33 -0500 Received: from mx1.redhat.com ([209.132.183.28]:25220) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PwVlv-0002nq-Io for qemu-devel@nongnu.org; Mon, 07 Mar 2011 03:26:31 -0500 Message-ID: <4D74971D.5040905@redhat.com> Date: Mon, 07 Mar 2011 09:28:13 +0100 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v5] PING: Fix ATA SMART and CHECK POWER MODE References: <1298986223.419.0.camel@nibbler.dlib.indiana.edu> <20110306192657.GE14590@volta.aurel32.net> In-Reply-To: <20110306192657.GE14590@volta.aurel32.net> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 >> >> -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) { >> >> >> >> >> >> >> >