From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:50178)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <sw@weilnetz.de>) id 1SXDnl-0004TX-2J
	for qemu-devel@nongnu.org; Wed, 23 May 2012 11:48:46 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <sw@weilnetz.de>) id 1SXDnd-0000Zo-Li
	for qemu-devel@nongnu.org; Wed, 23 May 2012 11:48:40 -0400
Received: from v220110690675601.yourvserver.net ([78.47.199.172]:41095)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <sw@weilnetz.de>) id 1SXDnd-0000Ys-FF
	for qemu-devel@nongnu.org; Wed, 23 May 2012 11:48:33 -0400
Message-ID: <4FBD06CD.2070304@weilnetz.de>
Date: Wed, 23 May 2012 17:48:29 +0200
From: Stefan Weil <sw@weilnetz.de>
MIME-Version: 1.0
References: <1337729202-26850-1-git-send-email-daniel@drv.nu>
In-Reply-To: <1337729202-26850-1-git-send-email-daniel@drv.nu>
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] ahci: SATA FIS is 20 bytes, not 0x20
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: Daniel Verkamp <daniel@drv.nu>
Cc: Kevin Wolf <kwolf@redhat.com>, Anthony Liguori <aliguori@us.ibm.com>, qemu-devel@nongnu.org, =?ISO-8859-15?Q?Andreas_F=E4rber?= <afaerber@suse.de>

Am 23.05.2012 01:26, schrieb Daniel Verkamp:
> As in the SATA and AHCI specifications, a FIS is 5 Dwords of 4 bytes
> each, which comes to 20 bytes (decimal), not 0x20.
>
> Signed-off-by: Daniel Verkamp<daniel@drv.nu>
> ---
>   hw/ide/ahci.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index a883a92..2d7d03d 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -462,7 +462,7 @@ static void ahci_check_cmd_bh(void *opaque)
>
>   static void ahci_init_d2h(AHCIDevice *ad)
>   {
> -    uint8_t init_fis[0x20];
> +    uint8_t init_fis[20];
>       IDEState *ide_state =&ad->port.ifs[0];

The current code only uses 14 elements, so 20 elements
still waste some local memory (and 0x20 elements waste
even more).

>
>
>       memset(init_fis, 0, sizeof(init_fis));
> @@ -619,7 +619,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis)
>       d2h_fis[11] = cmd_fis[11];
>       d2h_fis[12] = cmd_fis[12];
>       d2h_fis[13] = cmd_fis[13];
> -    for (i = 14; i<  0x20; i++) {
> +    for (i = 14; i<  20; i++) {
>           d2h_fis[i] = 0;
>       }

I am not sure whether this change is correct.
This code does _not_ access the array which was allocated above:

     d2h_fis = &ad->res_fis[RES_FIS_RFIS];

Regards,
Stefan W.