[Qemu-devel] Re: [PATCH-RFC 02/13] kvm: add API to set ioeventfd

[Michael S. Tsirkin]

On Tue, Jan 12, 2010 at 04:35:17PM -0600, Anthony Liguori wrote:
> On 01/11/2010 11:17 AM, Michael S. Tsirkin wrote:
>> Signed-off-by: Michael S. Tsirkin<address@hidden>
>> ---
>>   kvm-all.c |   24 ++++++++++++++++++++++++
>>   kvm.h     |    3 +++
>>   2 files changed, 27 insertions(+), 0 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index a312654..aa00119 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -1113,3 +1113,27 @@ void kvm_remove_all_breakpoints(CPUState *current_env)
>>   {
>>   }
>>   #endif /* !KVM_CAP_SET_GUEST_DEBUG */
>> +
>> +#ifdef KVM_IOEVENTFD
>> +int kvm_set_ioeventfd(uint16_t addr, uint16_t data, int fd, bool assigned)
>> +{
>> +    struct kvm_ioeventfd kick = {
>> +        .datamatch = data,
>> +        .addr = addr,
>> +        .len = 2,
>> +        .flags = KVM_IOEVENTFD_FLAG_DATAMATCH | KVM_IOEVENTFD_FLAG_PIO,
>> +        .fd = fd,
>> +    };
>> +    if (!assigned)
>> +        kick.flags |= KVM_IOEVENTFD_FLAG_DEASSIGN;
>> +    int r = kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD,&kick);
>> +    if (r<  0)
>> +        return r;
>> +    return 0;
>> +}
>>    
>
> I think we really ought to try to avoid having global state used here.   
> That means either we need to pass a CPUState to this function or we need  
> to have a ioeventfd be allocated as a structure that is then passed  
> around that can store a copy of the kvm_state fetched through CPUState.
>
> I think the later is best.  We really don't want ioeventfd scattered  
> throughout the code.
>
> Regards,
>
> Anthony Liguori


I don't really understand this comment.
I have no idea where to get CPUState in
a device and how to get kvm state from there.

And all other kvm-all functions use kvm_state
already, let's fix them first if we want to
get rid of global state?

Avi, what's your take on this patch?

-- 
MST