Re: [Qemu-devel] memory: could we add extra input param for memory_region_init_io()?

[Avi Kivity]

On 08/21/2012 02:18 PM, liu ping fan wrote:
>>
>>> But as
>>> it will also take the code path which has object_ref(Object*), so it
>>> has to convert, otherwise the code will corrupt.
>>> That is what I want to express.
>>
>> Option 2, for example, had MemoryRegionOps::object(MemoryRegion *) which
>> can be used to convert the opaque to an Object, or return NULL.  With
>> that option, there would be no corruption:
>>
> If we did not pass extra flag to indicate whether opaque is Object or
> not, we can not implement MemoryRegionOps::object().  Because we lack
> of something like "try,catch", and object_dynamic_cast() can not work.
> So besides memory_region_init_io(), we need extra interface to pass that flag?

The interface is MemoryRegionOps::object().

If the user does not implement it, then the opaque cannot be converted
into an object.  If the user did implement it, then the function
specifies how to convert it.

For example:


static Object *e1000_mmio_object(void *opaque)
{
    E1000State *s = opaque;

    return &s->dev.qdev.parent_obj;
}

static const MemoryRegionOps e1000_mmio_ops = {
    .read = e1000_mmio_read,
    .write = e1000_mmio_write,
    .endianness = DEVICE_LITTLE_ENDIAN,
    .object = e1000_mmio_object,
    .impl = {
        .min_access_size = 4,
        .max_access_size = 4,
    },
};

>>   qemu_mutex_lock(&mem_map_lock);
>>   MemoryRegionSection mrs = walk(&phys_map);
>>   Object *object = mrs.mr->ops->object(mrs.mr);
>>   if (object) {
>>       object_ref(object);
>>   }
>>   qemu_mutex_unlock(&mem_map_unlock);
>>

This should read:


    Object *object = NULL;
    if (mrs.mr->ops->object) {
        object = mrs.mr->ops->object(mrs.mr);
    }


>> So there is no memory corruption.  However, as I point out below, we
>> also lack the reference count.  Maybe we could do something like:
>>
> I think the hotplug issue is only for limited type of bus. And
> fortunately, for pci, they have already adopt Object.
> So for other SoC, maybe we can ignore this issue at current step

We still have to take the big qemu lock somehow.

>> If we can avoid a cycle.  Won't the device need to hold refs to the BAR?
> 
> I will check the code, but I has thought BAR is implemented by
> assigned device?  

Yes.


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