[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCHv2 02/12] kvm: add API to set ioeventfd
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCHv2 02/12] kvm: add API to set ioeventfd |
Date: |
Tue, 2 Mar 2010 19:41:03 +0200 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
On Thu, Feb 25, 2010 at 01:19:30PM -0600, Anthony Liguori wrote:
> On 02/25/2010 12:28 PM, Michael S. Tsirkin wrote:
>> Comment on kvm usage: rather than require users to do if (kvm_enabled())
>> and/or ifdefs, this patch adds an API that, internally, is defined to
>> stub function on non-kvm build, and checks kvm_enabled for non-kvm
>> run.
>>
>> While rest of qemu code still uses if (kvm_enabled()), I think this
>> approach is cleaner, and we should convert rest of code to it
>> long term.
>>
>
> I'm not opposed to that.
>
>> Signed-off-by: Michael S. Tsirkin<address@hidden>
>> ---
>>
>> Avi, Marcelo, pls review/ack.
>>
>> kvm-all.c | 22 ++++++++++++++++++++++
>> kvm.h | 16 ++++++++++++++++
>> 2 files changed, 38 insertions(+), 0 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 1a02076..9742791 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -1138,3 +1138,25 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t
>> *sigset)
>>
>> return r;
>> }
>> +
>> +#ifdef KVM_IOEVENTFD
>> +int kvm_set_ioeventfd(uint16_t addr, uint16_t data, int fd, bool assigned)
>>
>
> I think this API could use some love. You're using a very limited set
> of things that ioeventfd can do and you're multiplexing creation and
> destruction in a single call.
>
> I think:
>
> kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t data);
> kvm_unset_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t data);
>
> Would be better. Alternatively, an API that matched ioeventfd exactly:
>
> kvm_set_ioeventfd(int fd, uint64_t addr, int size, uint64_t data, int
> flags);
> kvm_unset_ioeventfd(...);
>
> Could work too.
>
> Regards,
>
> Anthony Liguori
>
So I renamed to kvm_set_ioeventfd_pio_word, but I still left assign
boolean in place because both implementation and usage take up
less code this way.
It's just an internal function, so no biggie to change it later ...
--
MST
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Qemu-devel] Re: [PATCHv2 02/12] kvm: add API to set ioeventfd,
Michael S. Tsirkin <=