Re: [Qemu-devel] [PATCH 1/2] Type-safe ioport callbacks

[Anthony Liguori <anthony@codemonkey.ws>]

On 10/26/2010 12:18 PM, Avi Kivity wrote:
>  On 10/26/2010 05:09 PM, Blue Swirl wrote:
>> On Tue, Oct 26, 2010 at 8:05 AM, Avi Kivity<avi@redhat.com>  wrote:
>> >    On 10/25/2010 08:38 PM, Blue Swirl wrote:
>> >>
>> >> >
>> >> >    I don't really see why we need registration; 
>> cpu_register_io() takes
>> >> >    function pointers, a size, and an opaque, and gives an 
>> integer handle
>> >> >  in
>> >> >    return.  With the IOPort object approach, you set up the 
>> IOPort with
>> >> >    function pointers, size is implied, and the opaque is derived 
>> using
>> >> >    container_of(); the handle is simply the address of the object.
>> >>
>> >>  With the handle, we can separate setting up the structures at device
>> >>  level, and mapping the object using only the handle at bus or other
>> >>  higher level. Can this be done with the object approach?
>> >
>> >  I believe so.  The handle is simply an indirect pointer, no?
>>
>> Yes, but then the object should also contain size information. That
>> should not be needed for mapping at higher level.
>
> Sorry, I don't follow your meaning.
>
> When I said "size is implied" I meant that the IOPort object has a 
> separate function pointer for sizes 1, 2, and 4, so it 
> ioport_register() doesn't need a size parameter.  But I don't see how 
> that relates to your comment.

Yeah, I don't think it makes sense to combine "this is how to dispatch 
I/O" with "this is a region of I/O address space".

I think an IORegion should contain an IOPort structure though.  I think 
the name needs rethinking.

Maybe:

struct PortIOHandler;
struct MemoryIOHandler;

And it would be good to add a memory callback to this series too.

Regards,

Anthony Liguori

>> >>  The purpose of that patch series was to perform the separation 
>> for PCI
>> >>  BARs. I wasn't so happy with the series, so I never pushed.
>> >
>> >  In fact I think an IOPort is even more suitable; if we need 
>> additional
>> >  attributes we can use a derived object:
>> >
>> >  struct PCIIOPort {
>> >      IOPort ioport;
>> >      /* additional fields */
>> >  };
>>
>> One issue with my series was that it would be great if the devices
>> just had some BAR structures (used by PCI layer to map the devices)
>> inside PCI/qdev structures, but I invented that too late. Maybe this
>> can be addressed in your design?
>
> It looks to be orthogonal.  It would be great to have a BAR object; 
> that object can then use your API, my API, or the existing API.
>