[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH-RFC 02/13] kvm: add API to set ioeventfd
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH-RFC 02/13] kvm: add API to set ioeventfd |
Date: |
Mon, 25 Jan 2010 21:28:27 +0200 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
On Mon, Jan 25, 2010 at 01:28:49PM -0600, Anthony Liguori wrote:
> On 01/25/2010 01:20 PM, Michael S. Tsirkin wrote:
>> 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.
>>
>
> This function doesn't seem to get called in the current patches so it's
> hard to evaluate what the right thing to do is.
>> And all other kvm-all functions use kvm_state
>> already, let's fix them first if we want to
>> get rid of global state?
>>
>
> Any time an operation is tied to a CPU in some way, we should not rely
> on global state. Based on the name and the fact that it references PIO,
> it strongly suggests to me that it's somehow related to a CPU but since
> it's not used in the current code it's hard to validate that.
>
> Regards,
>
> Anthony Liguori
It is called in the binding. Pls take a look.
The function does not reference a specific CPU: it binds
an eventfd descriptor to a specific address/data pair
for all CPUs.
>
>> Avi, what's your take on this patch?
>>
>>