qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] memory: could we add extra input param for memory_regio


From: liu ping fan
Subject: Re: [Qemu-devel] memory: could we add extra input param for memory_region_init_io()?
Date: Tue, 21 Aug 2012 12:48:45 +0800

On Sun, Aug 19, 2012 at 7:36 PM, Avi Kivity <address@hidden> wrote:
> On 08/19/2012 02:23 PM, Peter Maydell wrote:
>> On 19 August 2012 11:12, Avi Kivity <address@hidden> wrote:
>>> On 08/17/2012 10:41 AM, liu ping fan wrote:
>>>> And something like omap_mpu_timer_init() in file hw/omap1.c , the
>>>> opaque(omap_mpu_timer_s) is got from g_malloc0, which makes things
>>>> even harder to handle.  And the DO_CAST can not work for such issue.
>>>
>>> IMO omap_mpu_timer_s should be a DeviceState.  Peter?
>>
>> Ideally, yes, but qemu is full of devices that haven't yet made the leap
>> to QOM.
>>
>> omap1 is particularly tricky because I don't actually have any test images
>> for it, so refactoring it is a leap in the dark. [I've tried asking on this 
>> list
>> if anybody had test images a couple of times without success.]
>
> Okay.  Most of the options in this thread don't involve wholesale
> conversion of the opaque parameter in memory_region_init_io() to type
> Object *, so it's not necessary to convert everything.
>
But I think if the mr belongs to mem address space, it can not avoid
object_ref(Object *opaque) in mmio-dispatch code. ( If we use extra
flag to indicate whether the opaque is Object or not, then we lose the
meaning to transfer opaque to Object type, and maybe
memory_region_set_object() is a better choice).  And the only exempt
is that mr belong to io address space.

After carefully reading the code, I think
1. we assume Object &opaque have 1:1 relationship, if not, the Object
can be  refactored,
   which means children of Object (composite kid device or bus) will
be introduced. This
   may cause the modeling problem(but currently, can not find the example)

2. Some MemoryRegionOps are shared by different type of Device.
    For example, cirrus_vga_mem_ops will handle CirrusVGAState, and
the related Object
    can be ISACirrusVGAState or PCICirrusVGAState. So we need to remodel the
    CirrusVGAState as composite device of its parent,  and introducing
    qdev_create_in_place() just like qbus_create_in_place().

3. For the case, where opaque has no Object to relate with, simply
embeded Object at
    the head of them. (And there are lots of such issue in current code)

4. For pio, currently, we can ignore them.





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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]