On 08/14/2012 11:30 AM, liu ping fan wrote:
> To make memoryRegion survive without the protection of qemu big lock,
> we need to pin its based Object.
> In current code, the type of mr->opaque are quite different. Lots of
> them are Object, but quite of them are not yet.
>
> The most challenge for changing from memory_region_init_io(..., void
> *opaque, ...) to memory_region_init_io(..., Object *opaque,...) is
> such codes:
> hw/ide/cmd646.c:
> static void setup_cmd646_bar(PCIIDEState *d, int bus_num)
> {
> IDEBus *bus = &d->bus[bus_num];
> CMD646BAR *bar = &d->cmd646_bar[bus_num];
>
> bar->bus = bus;
> bar->pci_dev = d;
> memory_region_init_io(&bar->cmd, &cmd646_cmd_ops, bar, "cmd646-cmd", 4);
> memory_region_init_io(&bar->data, &cmd646_data_ops, bar, "cmd646-data",
> 8);
> }
> If we passed in mr's based Object @d to substitute @bar, then we can
> not pass the extra info @bus_num.
>
> To solve such issue, introduce extra member "Object *base" for MemoryRegion.
>
> diff --git a/memory.c b/memory.c
> index 643871b..afd5dea 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -931,6 +931,7 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
>
> void memory_region_init_io(MemoryRegion *mr,
> const MemoryRegionOps *ops,
> + Object *base,
> void *opaque,
> const char *name,
> uint64_t size)
> @@ -938,6 +939,7 @@ void memory_region_init_io(MemoryRegion *mr,
> memory_region_init(mr, name, size);
> mr->ops = ops;
> mr->opaque = opaque;
> + mr->base = base;
> mr->terminates = true;
> mr->destructor = memory_region_destructor_iomem;
> mr->ram_addr = ~(ram_addr_t)0;
> diff --git a/memory.h b/memory.h
> index bd1bbae..2746e70 100644
> --- a/memory.h
> +++ b/memory.h
> @@ -120,6 +120,7 @@ struct MemoryRegion {
> /* All fields are private - violators will be prosecuted */
> const MemoryRegionOps *ops;
> void *opaque;
> + Object *base;
> MemoryRegion *parent;
> Int128 size;
> target_phys_addr_t addr;
>
>
> Any comment?
>
I prefer that we convert the third parameter (opaque) to be an Object.
That is a huge change, but I think it will improve the code base overall.
Other options are:
1) add MemoryRegionOps::ref(MemoryRegion *) and ::unref(MemoryRegion *)
If NULL, these callbacks are ignored. If not, they are called with the
MemoryRegion as a parameter. Their responsibility is to derive the
Object from the MemoryRegion (through the opaque or using
container_of()) and ref or unref it respectively.
2) add Object *MemoryRegionOps::object(MemoryRegion *)
Similar; if NULL it is ignored, otherwise it is used to derive the
Object, which the memory core will ref and unref.
3) add bool MemoryRegionOps::opaque_is_object
Tells the memory core that it is safe to cast the opaque into an Object.
4) add memory_region_set_object(MemoryRegion *, Object *)
Like your proposal, but avoids adding an extra paramter and changing all
call sites.
--
error compiling committee.c: too many arguments to function
