Re: [Qemu-devel] [PATCH 11/23] memory: add ioeventfd support

[Avi Kivity <avi@redhat.com>]

On 07/25/2011 10:08 PM, Anthony Liguori wrote:
>>
>> +static void as_memory_ioeventfd_add(AddressSpace *as, 
>> MemoryRegionIoeventfd *fd)
>> +{
>> +    int r;
>> +
>> +    if (!fd->match_data || fd->addr.size != 4) {
>> +        abort();
>> +    }
>> +
>> +    r = kvm_set_ioeventfd_mmio_long(fd->fd, fd->addr.start, 
>> fd->data, true);
>> +    if (r<  0) {
>> +        abort();
>> +    }
>
>
> asserts would be friendlier.

I thought asserts were disabled by default.  But I see it isn't so; will 
update.

>
> I really dislike baking ioeventfd into this API.  There is only one 
> user of ioeventfd in the tree.

Two: virtio-pci and ivshmem.

>
> I worry that by having things like ioeventfd the API, we're making it 
> too difficult to side-step the API which prevents future optimizations.
>
> I'd prefer virtio-pci to have ugliness in it where it circumvented the 
> layering vs. having such a device specific thing in generic code.

It's impossible (or at least, impossible without further information 
from the API) to do this and retain correctness.  Currently virtio-pci 
is broken wrt bridges and overlapping BARs; it's probably also broken on 
targets that bridge the I/O address space to MMIO.

With the memory API, this is fixed in a natural way by making the I/O 
address space a subregion of the bridge which does the conversion; the 
code will automatically add the needed offset and use MMIO ioeventfd 
instead of portio.

On a more general note, I don't want this to be a lean and mean API that 
throws any complexity to the users; instead I want to make writing 
devices as simple as possible and own all the smarts.

(an example - undecoded address bits can be specified to the API which 
then takes care of replication or shifting).

-- 
error compiling committee.c: too many arguments to function