Re: [Qemu-devel] [RFC] Plan for moving forward with QOM

[Anthony Liguori <anthony@codemonkey.ws>]

On 12/14/2011 08:01 AM, Avi Kivity wrote:
> On 12/14/2011 03:53 PM, Anthony Liguori wrote:
>>> The fact that serial.c (which really should be called 16550.c)
>>> contains ISA
>>> specific IO callback routines feels like ISASerial isn't doing its job
>>> proberly to start with :-)
>>
>>
>> It's not really ISA specific callbacks but I need to think through
>> interaction with the memory API.
>>
>> In my qom-next branch, serial.c exposes a
>> serial_ioport_read/serial_ioport_write function.  isa-serial.c and
>> mm-serial.c create MemoryRegions based on thin wrappers around that
>> callback.
>>
>> Ideally, serial.c would export a MemoryRegion directly for the
>> registers it supported and isa-serial.c and mm-serial.c could bridge
>> that to the appropriate address space.
>>
>> I think the problem here ends up being itl_shift.  I think Avi's
>> trying to avoid having a non-flat dispatch space so you can't really
>> create a MemoryRegion that dispatches to another MemoryRegion.
>
> In fact I want the ability to create new address spaces.  For example
> index/data style interfaces, as found in RTC, IOAPIC, and PCI 0xcf8
> style config space, can be hooked to drive an RTC MemoryRegion, an
> IOAPIC MemoryRegion, and the PCI config space address space.

I think that might be overgeneralizing a bit.  There tends to be a lot of 
weirdness in how register demultiplexing works.

A UART uses a bit to determine whether address 0/1 reads the divisor or the 
thr/ier register.  It also dispatches reads/writes to different registers.

Even PCI config space is a bit weird.  The address register always needs to have 
the high bit set, otherwise ~0U is returned.  You could do something weird like 
either offset the address space, or have some special logic to handle it, but I 
think this tends to vary so much that you'll end up with a lot of special cases 
instead of having the logic live in the device where it more correctly belongs.

>
>>    We discussed it before and I believe he was planning on adding
>> itl_shift as a MemoryRegion mutator.
>
> I don't think it makes sense as a mutator, can it_shift change
> dynamically?  But as part of setup, yes.
>
>> Avi, did I understand that all correctly?
>
> If you mean that the interface between serial.c and isa-serial.c (or
> however they're renamed) should be MemoryRegions exposed by one and
> mapped by the other, then yes, I think that's the right interface.

So I guess it's just a question of how to make it work.  I actually think the 
most natural way is to have a MemoryRegion not attached to an AddressSpace and 
have the MemoryRegion bounce requests to another MemoryRegion.

I think that violates the basic design of the memory API though.

Regards,

Anthony Liguori