From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51448) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XNIzO-0006nu-JB for qemu-devel@nongnu.org; Fri, 29 Aug 2014 06:01:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XNIzH-00079A-3f for qemu-devel@nongnu.org; Fri, 29 Aug 2014 06:01:02 -0400 Received: from mail-we0-f171.google.com ([74.125.82.171]:44371) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XNIzG-00078t-P5 for qemu-devel@nongnu.org; Fri, 29 Aug 2014 06:00:55 -0400 Received: by mail-we0-f171.google.com with SMTP id u56so1977765wes.2 for ; Fri, 29 Aug 2014 03:00:53 -0700 (PDT) Message-ID: <54004F3F.8020207@linaro.org> Date: Fri, 29 Aug 2014 12:00:31 +0200 From: Eric Auger MIME-Version: 1.0 References: <1407594349-9291-1-git-send-email-eric.auger@linaro.org> <1407594349-9291-6-git-send-email-eric.auger@linaro.org> <20140812024147.GD23065@voom.redhat.com> <53E9BA2A.8040209@linaro.org> <20140813033255.GH7628@voom.redhat.com> In-Reply-To: <20140813033255.GH7628@voom.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 05/10] hw/vfio/pci: split vfio_get_device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: peter.maydell@linaro.org, kim.phillips@freescale.com, eric.auger@st.com, joel.schopp@amd.com, patches@linaro.org, will.deacon@arm.com, qemu-devel@nongnu.org, a.rigo@virtualopensystems.com, Bharat.Bhushan@freescale.com, agraf@suse.de, alex.williamson@redhat.com, stuart.yoder@freescale.com, a.motakis@virtualopensystems.com, kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org On 08/13/2014 05:32 AM, David Gibson wrote: > On Tue, Aug 12, 2014 at 08:54:34AM +0200, Eric Auger wrote: >> On 08/12/2014 04:41 AM, David Gibson wrote: >>> On Sat, Aug 09, 2014 at 03:25:44PM +0100, Eric Auger wrote: >>>> vfio_get_device now takes a VFIODevice as argument. The function is split >>>> into 4 functional parts: dev_info query, device check, region populate >>>> and interrupt populate. the last 3 are specialized by parent device and >>>> are added into DeviceOps. >>> >>> Why is splitting these up into 4 stages useful, rather than having a >>> single sub-class specific callback? >> >> Hi David, >> >> VFIOPlatformDevice already inherits from SysBusDevice and hence cannot >> inherit from another VFIODevice. Same for VFIOPCIDevice that inherits >> from PCIDevice. This is why I created this non QOM struct. But did you >> mean something else? > > Ah, yes, sorry, I missed that, though it's obvious now I think about > it. > >> Then splitting into 4: This was to share some code between platform and >> PCI (dev_info query) and vfio_get_device was quite big already. I >> thought it makes sense to split it into functional parts. > > Hm, ok. So splitting out dev_info_query certainly makes sense then. > But does splitting the two populate sections make sense? Is it > plausible that two different VFIO capable busses would share one of > these functions but not the other? Hi David, Coming back to you on that topic. There is no other justification for splitting the code into 3 functions except than having shorter functions with reduced functionality. But I acknowledge it would simplify the diff between original code and new one so I intend to keep a single specialized functions instead of 3. Best Regards Eric >