From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:44695)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1S7Rm4-0002zv-FA
	for qemu-devel@nongnu.org; Tue, 13 Mar 2012 09:28:28 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1S7Rly-0001n6-52
	for qemu-devel@nongnu.org; Tue, 13 Mar 2012 09:28:24 -0400
Received: from mx1.redhat.com ([209.132.183.28]:40600)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1S7Rlx-0001mv-UA
	for qemu-devel@nongnu.org; Tue, 13 Mar 2012 09:28:18 -0400
Message-ID: <4F5F4B6C.9040508@redhat.com>
Date: Tue, 13 Mar 2012 14:28:12 +0100
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1331641907-8348-1-git-send-email-bos@je-eigen-domein.nl>
	<4F5F453A.7020606@redhat.com> <4F5F4A2E.2000706@redhat.com>
In-Reply-To: <4F5F4A2E.2000706@redhat.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 3/3] [v2] ide: Adds wwn=hex qdev option
 allowing the user to specify a disk's World Wide Name
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Floris Bos <bos@je-eigen-domein.nl>, qemu-devel@nongnu.org, Floris Bos <dev@noc-ps.com>

Il 13/03/2012 14:22, Kevin Wolf ha scritto:
>>> >>      put_le16(p + 82, (1 << 14) | (1 << 5) | 1);
>>> >>      /* 13=flush_cache_ext,12=flush_cache,10=lba48 */
>>> >>      put_le16(p + 83, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10));
>> > 
>> > Bit 14 should not be set here.  Not your fault, but perhaps you can fix
>> > it too?
> 
> Table in the spec says: "14 - Shall be set to one"

uff, I hate those pages.  The wrong one is word 86.  Bit 14 is reserved
there.  It's wrong in ide_cfata_identify, too.

>>> >> -    /* 14=set to 1, 1=SMART self test, 0=SMART error logging */
>>> >> -    put_le16(p + 84, (1 << 14) | 0);
>>> >> +    /* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */
>>> >> +    if (s->wwn) {
>>> >> +        put_le16(p + 84, (1 << 14) | (1 << 8) | 0);
>>> >> +    } else {
>>> >> +        put_le16(p + 84, (1 << 14) | 0);
>>> >> +    }
>>> >>      /* 14 = NOP supported, 5=WCACHE enabled, 0=SMART feature set enabled */
>>> >>      if (bdrv_enable_write_cache(s->bs))
>>> >>           put_le16(p + 85, (1 << 14) | (1 << 5) | 1);
>>> >> @@ -152,8 +156,12 @@ static void ide_identify(IDEState *s)
>>> >>           put_le16(p + 85, (1 << 14) | 1);
>>> >>      /* 13=flush_cache_ext,12=flush_cache,10=lba48 */
>>> >>      put_le16(p + 86, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10));

> What about ide_cfata_identify, while we're at it? Or isn't it supported
> there?

No idea.

Paolo