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

[Andreas Färber]

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.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg