From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:40496) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QxuV1-000656-Tv for qemu-devel@nongnu.org; Mon, 29 Aug 2011 01:35:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QxuV0-0002jO-GK for qemu-devel@nongnu.org; Mon, 29 Aug 2011 01:35:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39273) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QxuV0-0002iD-4v for qemu-devel@nongnu.org; Mon, 29 Aug 2011 01:35:06 -0400 Message-ID: <4E5B2505.1020808@redhat.com> Date: Mon, 29 Aug 2011 08:35:01 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1314546216-26613-1-git-send-email-avi@redhat.com> <20110828181404.GE11446@zapo> In-Reply-To: <20110828181404.GE11446@zapo> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable 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: "Edgar E. Iglesias" Cc: qemu-devel@nongnu.org, Richard Henderson On 08/28/2011 09: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 alloca= tes > > memory, instead of the caller. This clears a FIXME in the flash cod= e. > > > > 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 i= s the > > right behaviour, and the old behaviour was just an implementation li= mitation, > > 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 deco= ded > into a read from a revision register. To read from the flash at 0x10, y= ou > need to go via the 0x1e0 mapping. > > That was never modelled by QEMU, instead the malta board writes into th= e > backing ram of the flash area, see: > /* Board ID =3D 0x420 (Malta Board with CoreLV) > XXX: theoretically 0x1e000010 should map to flash and 0x1fc00010 sh= ould > map to the board ID. */ > stl_p(memory_region_get_ram_ptr(bios) + 0x10, 0x00000420); This can be modelled correctly, using a container and a new region for=20 0x10-0x14. I'll live that as an exercise to the interested writer. > I assume this will continue to work even after your changes but it woul= d be > good to test it. I can check it. > > A problem, with my compiler I see this: > qemu/hw/mips_malta.c: In function =E2=80=98mips_malta_init=E2=80=99: > qemu/hw/mips_malta.c:860:37: error: =E2=80=98bios=E2=80=99 may be used = uninitialized in this function [-Werror=3Duninitialized] > Thanks, I have a fix for this and will post an updated patch once the=20 other issues are resolved. --=20 I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.