From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1JZbws-0003Ne-7i
	for qemu-devel@nongnu.org; Wed, 12 Mar 2008 21:09:34 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1JZbwr-0003MI-Km
	for qemu-devel@nongnu.org; Wed, 12 Mar 2008 21:09:33 -0400
Received: from [199.232.76.173] (helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1JZbwr-0003M6-Dm
	for qemu-devel@nongnu.org; Wed, 12 Mar 2008 21:09:33 -0400
Received: from hall.aurel32.net ([88.191.38.19])
	by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32)
	(Exim 4.60) (envelope-from <aurelien@aurel32.net>)
	id 1JZbwr-0005ic-25
	for qemu-devel@nongnu.org; Wed, 12 Mar 2008 21:09:33 -0400
Date: Thu, 13 Mar 2008 02:09:39 +0100
From: Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [Qemu-devel] [PATCH] mipsnet incorrect device ID fix
Message-ID: <20080313010939.GA5914@volta.aurel32.net>
References: <47BE1B61.8080604@bravegnu.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Disposition: inline
In-Reply-To: <47BE1B61.8080604@bravegnu.org>
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Vijay Kumar B." <vijaykumar@bravegnu.org>
Cc: qemu-devel@nongnu.org

On Fri, Feb 22, 2008 at 06:16:25AM +0530, Vijay Kumar wrote:
> The mipsnet device returns wrong values for device ID, since it returns  
> the contents of the pointer rather that the contents of the device ID  
> string. Also the contents of the string is returned such that the order  
> is host endianess dependent. The patch fixes both these issues.
>
> Signed-off-by: Vijay Kumar B. <vijaykumar@bravegnu.org>
>
> --- qemu-orig/hw/mipsnet.c	2007-12-27 11:18:52.000000000 +0530
> +++ qemu-mod/hw/mipsnet.c	2008-02-20 20:23:44.000000000 +0530
> @@ -101,6 +101,19 @@
>      mipsnet_update_irq(s);
>  }
>
> +static uint32_t bytes_to_int32(const unsigned char *arr)
> +{
> +    int i;
> +    uint32_t ret = 0;
> +    int nbytes = sizeof(int32_t);
> +
> +    for (i = 0; i < nbytes; i++) {
> +	ret = ret << 8 | arr[nbytes - 1 - i];
> +    }
> +
> +    return ret;
> +}

Why not use le32_to_cpu() which does the same, but in an optimized way?

>  static uint32_t mipsnet_ioport_read(void *opaque, uint32_t addr)
>  {
>      MIPSnetState *s = opaque;
> @@ -110,10 +123,10 @@
>      addr &= 0x3f;
>      switch (addr) {
>      case MIPSNET_DEV_ID:
> -	ret = *((uint32_t *)&devid);
> +	ret = bytes_to_int32(devid);

That's the '&' which is wrong here. The string can be accessed with
*((uint32_t *)devid). So you can simply use:

        ret = le32_to_cpu(*((uint32_t *)devid))

>          break;
>      case MIPSNET_DEV_ID + 4:
> -	ret = *((uint32_t *)(&devid + 4));
> +	ret = bytes_to_int32(devid + 4);

        ret = le32_to_cpu(*((uint32_t *)devid + 4));

>          break;
>      case MIPSNET_BUSY:
>  	ret = s->busy;
>
>
>

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net