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

["Andreas Färber" <afaerber@suse.de>]

Am 29.11.2011 10:52, schrieb 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:
>>>> 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).

Sorry, I simply misunderstood your suggestion (memory newbie): Since we
*continue* to go through subpage_{read,write}len(), we don't need a
specific opaque value any longer so can do this from generic code.

v2 sent out.

Andreas

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