From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52766) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RVKMN-0001uw-SW for qemu-devel@nongnu.org; Tue, 29 Nov 2011 04:52:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RVKMJ-0003JZ-AJ for qemu-devel@nongnu.org; Tue, 29 Nov 2011 04:52:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:24050) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RVKMI-0003JI-VH for qemu-devel@nongnu.org; Tue, 29 Nov 2011 04:52:15 -0500 Message-ID: <4ED4AB4A.2010209@redhat.com> Date: Tue, 29 Nov 2011 11:52:10 +0200 From: Avi Kivity MIME-Version: 1.0 References: <1322492805-5530-1-git-send-email-afaerber@suse.de> <4ED3C22E.1070607@redhat.com> <4ED40DB6.3020701@suse.de> In-Reply-To: <4ED40DB6.3020701@suse.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] exec.c: Fix subpage memory access to RAM MemoryRegion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-1?Q?Andreas_F=E4rber?= Cc: Blue Swirl , qemu-devel@nongnu.org, Gleb Natapov On 11/29/2011 12:39 AM, Andreas F=E4rber wrote: > Am 28.11.2011 18:17, schrieb Avi Kivity: > > On 11/28/2011 05:06 PM, Andreas F=E4rber wrote: > >> Commit 95c318f5e1f88d7e5bcc6deac17330fd4806a2d3 (Fix segfault in mmi= o subpage > >> handling code.) prevented a segfault by making all subpage registrat= ions > >> over an existing memory page perform an unassigned access. Symptoms = were > >> writes not taking effect and reads returning zero. > >> > >> Very small page sizes are not currently supported either, so subpage= memory > >> areas cannot fully be avoided. > >> > >> Therefore revert the previous fix and defer recognition of IO_MEM_RA= M to > >> subpage_{read,write}len() and translate any access there. > >> > >> Signed-off-by: Andreas F=E4rber > >> Cc: Avi Kivity > >> Cc: Gleb Natapov > >> Cc: Blue Swirl > >> --- > >> exec.c | 33 +++++++++++++++++++++++++++++++-- > >> 1 files changed, 31 insertions(+), 2 deletions(-) > >> > >> diff --git a/exec.c b/exec.c > >> index 6b92198..fba5ba1 100644 > >> --- a/exec.c > >> +++ b/exec.c > >> @@ -3508,6 +3508,21 @@ static inline uint32_t subpage_readlen (subpa= ge_t *mmio, > >> =20 > >> addr +=3D mmio->region_offset[idx]; > >> idx =3D mmio->sub_io_index[idx]; > >> + if (unlikely(idx =3D=3D IO_MEM_RAM)) { > >=20 > > IMO, io_mem_init() should have something like > >=20 > > cpu_register_io_memory_fixed(IO_MEM_SUBPAGE_RAM, subpage_ram_read, > > subpage_ram_write, ...); > >=20 > > so you don't need those ugly switches; you just convert IO_MEM_RAM to > > IO_MEM_SUBPAGE_RAM. Maybe even register IO_MEM_RAM itself. Note nee= d > > to handle dirty logging carefully. > > That didn't work because cpu_register_io_memory_fixed() is called from > subpage_init(), which is called once for the whole page only, and the > actual subpages are set up with multiple calls to subpage_register() > instead. I don't mean replacing the subpage handle with a call to c_r_io_m_f(); just make the handle that is placed supage_t::sub_io_index have real io callbacks. In io_mem_init(), call cpu_register_io_memory_fixed() with a new mem_read[] callback array an the existing notdirty_mem_write[] array.=20 In subpage_register(), if we get an IO_MEM_RAM, convert it to IO_MEM_SUBPAGE_RAM (and copy the 'memory' to region_offset). --=20 error compiling committee.c: too many arguments to function