From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:58077)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1SEygs-0007RJ-M0
	for qemu-devel@nongnu.org; Tue, 03 Apr 2012 04:02:17 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1SEygm-00038a-AK
	for qemu-devel@nongnu.org; Tue, 03 Apr 2012 04:02:10 -0400
Received: from mail-wi0-f181.google.com ([209.85.212.181]:54616)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1SEygm-00035d-2c
	for qemu-devel@nongnu.org; Tue, 03 Apr 2012 04:02:04 -0400
Received: by wibhr17 with SMTP id hr17so3049778wib.10
	for <qemu-devel@nongnu.org>; Tue, 03 Apr 2012 01:02:02 -0700 (PDT)
Date: Tue, 3 Apr 2012 08:03:32 +0100
From: Stefan Hajnoczi <stefanha@gmail.com>
Message-ID: <20120403070332.GA27304@stefanha-thinkpad.localdomain>
References: <cover.1333088411.git.peter.crosthwaite@petalogix.com>
	<26d832cdffc1b31cfe1aa6fa9ca50f66d6fb9f71.1333088411.git.peter.crosthwaite@petalogix.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <26d832cdffc1b31cfe1aa6fa9ca50f66d6fb9f71.1333088411.git.peter.crosthwaite@petalogix.com>
Subject: Re: [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion
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: "Peter A. G. Crosthwaite" <peter.crosthwaite@petalogix.com>
Cc: edgar.iglesias@gmail.com, qemu-devel@nongnu.org, john.williams@petalogix.com, paul@codesourcery.com

On Fri, Mar 30, 2012 at 04:37:11PM +1000, Peter A. G. Crosthwaite wrote:
> +static void flash_sync_page(struct flash *s, int page)
> +{
> +    if (s->bdrv) {
> +        int bdrv_sector;
> +        int offset;
> +
> +        bdrv_sector = (page * s->pagesize) / 512;
> +        offset = bdrv_sector * 512;
> +        bdrv_write(s->bdrv, bdrv_sector,
> +                   s->storage + offset, (s->pagesize + 511) / 512);

Devices should not use synchronous block I/O interfaces.  sd, flash, and
a couple others still do for historical reasons but new devices should
not.

The vcpu, QEMU monitor, and VNC are all blocked while I/O takes place.
This can be avoided by using bdrv_aio_writev() instead.

Can you change this code to use bdrv_aio_writev() or is this flash
device specified to complete operations within certain time constraints?
(The problem is that the image file could be on a slow harddisk or other
media that don't meet those timing requirements.)

> +static int m25p80_init(SPISlave *ss)
> +{
> +    DriveInfo *dinfo;
> +    struct flash *s = FROM_SPI_SLAVE(struct flash, ss);
> +    /* FIXME: This should be handled centrally!  */
> +    static int mtdblock_idx;
> +    dinfo = drive_get(IF_MTD, 0, mtdblock_idx++);
> +
> +    DB_PRINT("inited m25p80 device model - dinfo = %p\n", dinfo);
> +    /* TODO: parameterize */
> +    s->size = 8 * 1024 * 1024;
> +    s->pagesize = 256;
> +    s->sectorsize = 4 * 1024;
> +    s->dirty_page = -1;
> +    s->storage = g_malloc0(s->size);

Please use qemu_blockalign(s->bdrv, s->size) to allocate I/O buffers.
It honors memory alignment requirements (necessary for O_DIRECT files).

Stefan