From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NZUcL-0005Va-BD for qemu-devel@nongnu.org; Mon, 25 Jan 2010 14:28:57 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NZUcG-0005Tg-Nh for qemu-devel@nongnu.org; Mon, 25 Jan 2010 14:28:57 -0500 Received: from [199.232.76.173] (port=33804 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NZUcG-0005TX-Hy for qemu-devel@nongnu.org; Mon, 25 Jan 2010 14:28:52 -0500 Received: from mail-iw0-f188.google.com ([209.85.223.188]:37188) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NZUcG-0000WZ-8a for qemu-devel@nongnu.org; Mon, 25 Jan 2010 14:28:52 -0500 Received: by iwn26 with SMTP id 26so3757137iwn.14 for ; Mon, 25 Jan 2010 11:28:51 -0800 (PST) Message-ID: <4B5DF0F1.9080108@codemonkey.ws> Date: Mon, 25 Jan 2010 13:28:49 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <20100111171703.GC11936@redhat.com> <4B4CF925.80602@codemonkey.ws> <20100125192015.GD11998@redhat.com> In-Reply-To: <20100125192015.GD11998@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH-RFC 02/13] kvm: add API to set ioeventfd List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org 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 >>> --- >>> 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 > Avi, what's your take on this patch? > >