On 05/01/2012 09:26 AM, Avi Kivity wrote:
On 05/01/2012 05:15 PM, Anthony Liguori wrote:
I think a nice fix would be to make it_shift as memory region mutator
and allow it to be set after initialization.


Indeed I wanted to make it_shift as part of the memory core.  But a
mutator?  It doesn't change in real hardware, so this looks artificial.
So far all mutators really change at runtime.


QOM has a concept of initialization and realize.  You can change
properties after initialization but not before realize.

So as long as it_shift can be set after initialization but before
realize (which I think is roughly memory_region_add_subregion) it
doesn't need to be a mutator.

Ok, good.


What is the problem with delaying region initialization until realize?

We need to initialize the MemoryRegion in order to expose it as a
property.  We want to do that during initialize.  Here's an example:

qom-create isa-i8259 foo
qom-set /peripheral/foo/io it_shift 1
qom-set /peripheral/foo/realize true

For this to work, it_shift needs to be a QOM property of the "io"
MemoryRegion.  The MemoryRegion needs to be created in instance_init.

So it looks like we need two phase initialization for memory regions as
well?

Yes, exactly. Converting MemoryRegion to QOM will give it a two phase initialization FWIW.

Not so pretty.

It's unavoidable because we're dealing with a graph. The connections between objects may form loops which effectively means that we have dependency loops. That means we need to be able to create all objects first and then establish the links between them. This is why we need two stage initialization.

b) There's some duplication in MemoryRegions with respect to QOM.
Memory regions want to have a name but with QOM they'll be addressable
via a path.  I go back and forth about how aggressively we want to
refactor MemoryRegions.

These days region names are purely for debugging.  The ABI bit was moved
to a separate function.

Fair enough.

BTW, in the branch I've posted, I've got a number of memory API
conversions or removal of legacy interfaces.

Nice.  But you use get_system_io(), which is bad.

Only in the device setup code. That will all eventually get moved into a SuperIO chip. The PC code needs massive amounts of refactoring.

Regards,

Anthony Liguori




Reply via email to