From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42930) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xu3I0-0006KL-Lq for qemu-devel@nongnu.org; Thu, 27 Nov 2014 12:55:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xu3Hu-0005fp-Kk for qemu-devel@nongnu.org; Thu, 27 Nov 2014 12:55:36 -0500 Received: from mail-wg0-f47.google.com ([74.125.82.47]:50921) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xu3Hu-0005fX-Ch for qemu-devel@nongnu.org; Thu, 27 Nov 2014 12:55:30 -0500 Received: by mail-wg0-f47.google.com with SMTP id n12so7067342wgh.34 for ; Thu, 27 Nov 2014 09:55:29 -0800 (PST) Message-ID: <54776543.9050001@linaro.org> Date: Thu, 27 Nov 2014 18:54:11 +0100 From: Eric Auger MIME-Version: 1.0 References: <1414764350-5140-1-git-send-email-eric.auger@linaro.org> <1414764350-5140-10-git-send-email-eric.auger@linaro.org> <5459FC19.1050200@suse.de> <5475A150.8030305@linaro.org> <5475AA53.1030509@suse.de> <5475AFED.6090800@linaro.org> <5475B781.5030009@suse.de> <5475E7C8.6040707@linaro.org> <54772F9E.3030701@linaro.org> <54773695.8090800@suse.de> <54773FEE.80002@linaro.org> <54774318.4060607@suse.de> <54774979.9090701@suse.de> <54775B9D.1000203@linaro.org> <54775E64.7070509@suse.de> <5477609A.4000305@linaro.org> <5477648B.2060103@suse.de> In-Reply-To: <5477648B.2060103@suse.de> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v7 09/16] hw/vfio/platform: add vfio-platform support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf , eric.auger@st.com, christoffer.dall@linaro.org, qemu-devel@nongnu.org, pbonzini@redhat.com, kim.phillips@freescale.com, a.rigo@virtualopensystems.com, manish.jaggi@caviumnetworks.com, joel.schopp@amd.com Cc: patches@linaro.org, Kim Phillips , will.deacon@arm.com, stuart.yoder@freescale.com, alex.williamson@redhat.com, kvmarm@lists.cs.columbia.edu On 11/27/2014 06:51 PM, Alexander Graf wrote: > > > On 27.11.14 18:34, Eric Auger wrote: >> On 11/27/2014 06:24 PM, Alexander Graf wrote: >>> >>> >>> On 27.11.14 18:13, Eric Auger wrote: >>>> On 11/27/2014 04:55 PM, Alexander Graf wrote: >>>>> >>>>> >>>>> On 27.11.14 16:28, Alexander Graf wrote: >>>>>> >>>>>> >>>>>> On 27.11.14 16:14, Eric Auger wrote: >>>>>>> On 11/27/2014 03:35 PM, Alexander Graf wrote: >>>>>>>> >>>>>>>> > > [...] > >>>>>> >>>>>> That should be easy - make it a link property. In fact, this would be >>>>>> one of those cases where not generalizing the code would've been a good >>>>>> idea. >>>> In that case the machine (init done) callback would be used to pass the >>>> vgic handle to each vfio device. Registered by the machine file, isn't >>>> it. Aren't we exactly at the same state you wanted to improve initially >>>> where the notifier is registered by the machine file, not belonging to >>>> the VFIO device, just replacing first_irq param by vgic_handle which >>>> eventually ends up as a link. >>>> >>>> This notifier still cannot be registered by the VFIO device finalize fn >>>> since the VFIO device has no handle to the interrupt controller. kind of >>>> chicken & egg problem. >>>>>> >>>>>> If device creation would live in the machine file, the machine could >>>>>> automatically set the link. Maybe you can still get there somehow? You >>>>>> could add a machine callback in the device allocation function. >>>>> >>>>> If this gets too messy, I think doing a machine attribute would work as >>>>> well here. Check out the way we pass the e500-ccsr object on e500: >>>>> >>>>> >>>>> http://git.qemu.org/?p=qemu.git;a=blob;f=hw/pci-host/ppce500.c;h=1b4c0f00236e8005c261da527d416fe6a053b353;hb=HEAD#l337 >>>>> >>>>> >>>>> http://git.qemu.org/?p=qemu.git;a=blob;f=hw/ppc/e500.c;h=2832fc0da444d89737768f7c4dcb0638e2625750;hb=HEAD#l873 >>>> >>>> looks OK indeed >>>>> >>>>> I think doing an actual link would be cleaner, but at least the above >>>>> gets you to an acceptable state that can still be improved with links >>>>> later - the basic idea is the same :). >>>> >>>> >>>> and why not "simply" a qemu_register_reset passing the vgic handle as >>>> opaque. >>> >>> Who would register this reset callback? It'd have to be someone who >>> knows both the VFIO device as well as the vGIC device. >> the machine file would. reset callback implemented in vfio-platform.c, >> looping on all instances. ~ as today for the notifier but without the >> dangling pointer. not sure you will like it though ;-) > > Ah, so you would do the actual VFIO call inside the machine file? yes in the machine file. Or > would you call a VFIO function when you see that a device is VFIO and > trigger the connection at that point? That would work too I suppose. > >>> >>> The reset idea could work as replacement for the notifier though. So you >>> could have the VFIO device register a reset callback in which it asks >>> the vgic for the number and registers the IRQ with KVM. >> arghh, still the problem of passing the vgic handle. I used the reset cb >> registration by the machine file to do that. Of course if we use your >> machine property trick we can do the registration by the VFIO driver >> itself. > > Yup, either way works IMHO :). OK I suggest I do my next patch as is and you will tell me... it will be easy to revert to machine prop anyway. Thanks for your time! Eric > > > Alex >