Re: [Qemu-devel] [PATCH] exec.c: Fix subpage memory access to RAM MemoryRegion

[Avi Kivity]

On 11/29/2011 12:39 AM, Andreas Färber wrote:
> Am 28.11.2011 18:17, schrieb Avi Kivity:
> > On 11/28/2011 05:06 PM, Andreas Färber wrote:
> >> Commit 95c318f5e1f88d7e5bcc6deac17330fd4806a2d3 (Fix segfault in mmio 
> >> subpage
> >> handling code.) prevented a segfault by making all subpage registrations
> >> 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_RAM to
> >> subpage_{read,write}len() and translate any access there.
> >>
> >> Signed-off-by: Andreas Färber <address@hidden>
> >> Cc: Avi Kivity <address@hidden>
> >> Cc: Gleb Natapov <address@hidden>
> >> Cc: Blue Swirl <address@hidden>
> >> ---
> >>  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 (subpage_t 
> >> *mmio,
> >>  
> >>      addr += mmio->region_offset[idx];
> >>      idx = mmio->sub_io_index[idx];
> >> +    if (unlikely(idx == IO_MEM_RAM)) {
> > 
> > IMO, io_mem_init() should have something like
> > 
> >   cpu_register_io_memory_fixed(IO_MEM_SUBPAGE_RAM, subpage_ram_read,
> > subpage_ram_write, ...);
> > 
> > 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 need
> > 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. 
In subpage_register(), if we get an IO_MEM_RAM, convert it to
IO_MEM_SUBPAGE_RAM (and copy the 'memory' to region_offset).

-- 
error compiling committee.c: too many arguments to function