On Tue, Aug 21, 2012 at 8:41 PM, Avi Kivity <a...@redhat.com> wrote:
> 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) {
Oh, thanks. I have not realize it at the first glance. Thank you for
the great patient.
During these days, when I try to verify the convert, I run into the
issue similar as you point out for "assigned device can disappear
while one of its BARs remain". For example,
typedef struct PCIIDEState {
PCIDevice dev;
IDEBus bus[2]; ==> lies in parent obj,
..........................
}
It is an create-in-place issue,
void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
{
qbus_create_inplace(&idebus->qbus, TYPE_IDE_BUS, dev, NULL);
idebus->bus_id = bus_id;
}
When I try to fix such issue, I got another patchset "Subject: [PATCH
0/10] rework on hot unplug", in a short word, unplug will break the
refer circular, so we can have back ref from IDEBus to PCIIDEState
will CC you.
Thanks and regards,
pingfan
> 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
