From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:33029) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QxkNc-0006xp-Pn for qemu-devel@nongnu.org; Sun, 28 Aug 2011 14:46:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QxkNb-0000M7-Ee for qemu-devel@nongnu.org; Sun, 28 Aug 2011 14:46:48 -0400 Received: from mail-fx0-f45.google.com ([209.85.161.45]:48640) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QxkNb-0000Iq-18 for qemu-devel@nongnu.org; Sun, 28 Aug 2011 14:46:47 -0400 Received: by fxbb27 with SMTP id b27so4304955fxb.4 for ; Sun, 28 Aug 2011 11:46:46 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20110828181404.GE11446@zapo> References: <1314546216-26613-1-git-send-email-avi@redhat.com> <20110828181404.GE11446@zapo> Date: Sun, 28 Aug 2011 20:46:45 +0200 Message-ID: From: "Edgar E. Iglesias" Content-Type: multipart/alternative; boundary=0015174737c44b71b004ab9533ac Subject: Re: [Qemu-devel] [PATCH] pflash_cfi01/pflash_cfi02: convert to memory API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: qemu-devel@nongnu.org, Richard Henderson --0015174737c44b71b004ab9533ac Content-Type: text/plain; charset=ISO-8859-1 On Sun, Aug 28, 2011 at 8:14 PM, Edgar E. Iglesias wrote: > On Sun, Aug 28, 2011 at 06:43:36PM +0300, Avi Kivity wrote: > > cfi02 is annoying in that is ignores some address bits; we probably > > want explicit support in the memory API for that. > > > > In order to get the correct opaque into the MemoryRegion object, the > > allocation scheme is changed so that the flash emulation code allocates > > memory, instead of the caller. This clears a FIXME in the flash code. > > > > Signed-off-by: Avi Kivity > > --- > > > > This is a rework of the pflash conversion to the memory API. It has > changed > > significantly - the change in allocation described above - so please > review > > carefully. > > > > The mips_malta change also changes behavious - previously on of the two > > aliases was mapped as rom/device while the other was mapped as plain rom. > > Now both aliases are mapped as rom/device. I'm guessing that this is the > > right behaviour, and the old behaviour was just an implementation > limitation, > > but if someone can check, that would be even better. That file has the > most > > sensitive changes so please review it extra carefully. > > Regarding the flash mapping, your description matches my understanding of > it. > There is a difference between the 0x1fc and the 0x1e0 mappings though, in > that > reading from 0x1fc00010 does not map to the flash but instead gets decoded > into a read from a revision register. To read from the flash at 0x10, you > need to go via the 0x1e0 mapping. > > That was never modelled by QEMU, instead the malta board writes into the > backing ram of the flash area, see: > /* Board ID = 0x420 (Malta Board with CoreLV) > XXX: theoretically 0x1e000010 should map to flash and 0x1fc00010 should > map to the board ID. */ > stl_p(memory_region_get_ram_ptr(bios) + 0x10, 0x00000420); > > I assume this will continue to work even after your changes but it would be > good to test it. I can check it. > The 0x10 hack still works as expected. Cheers --0015174737c44b71b004ab9533ac Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On Sun, Aug 28, 2011 at 8:14 PM, Edgar E. Iglesias <edgar.iglesias@gmail.com><= /span> wrote:
On Sun, Aug 28, 2011 at 06:43:36PM +0300, Avi Kivity wrot= e:
> cfi02 is annoying in that is ignores some address bits; we probably > want explicit support in the memory API for that.
>
> In order to get the correct opaque into the MemoryRegion object, the > allocation scheme is changed so that the flash emulation code allocate= s
> memory, instead of the caller. =A0This clears a FIXME in the flash cod= e.
>
> Signed-off-by: Avi Kivity <avi@re= dhat.com>
> ---
>
> This is a rework of the pflash conversion to the memory API. =A0It has= changed
> significantly - the change in allocation described above - so please r= eview
> carefully.
>
> The mips_malta change also changes behavious - previously on of the tw= o
> aliases was mapped as rom/device while the other was mapped as plain r= om.
> Now both aliases are mapped as rom/device. =A0I'm guessing that th= is is the
> right behaviour, and the old behaviour was just an implementation limi= tation,
> but if someone can check, that would be even better. =A0That file has = the most
> sensitive changes so please review it extra carefully.

Regarding the flash mapping, your description matches my understandin= g of it.
There is a difference between the 0x1fc and the 0x1e0 mappings though, in t= hat
reading from 0x1fc00010 does not map to the flash but instead gets decoded<= br> into a read from a revision register. To read from the flash at 0x10, you need to go via the 0x1e0 mapping.

That was never modelled by QEMU, instead the malta board writes into the backing ram of the flash area, see:
/* Board ID =3D 0x420 (Malta Board with CoreLV)
=A0 XXX: theoretically 0x1e000010 should map to flash and 0x1fc00010 shoul= d
=A0 map to the board ID. */
stl_p(memory_region_get_ram_ptr(bios) + 0x10, 0x000= 00420);

I assume this will continue to work even after your changes but it wo= uld be
good to test it. I can check it.

The 0x= 10 hack still works as expected.

Cheers
--0015174737c44b71b004ab9533ac--