From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH for 4.6 07/13] xen: Introduce a generic way to describe device Date: Wed, 17 Dec 2014 10:30:22 +0000 Message-ID: <54915B3E.4010702@linaro.org> References: <1418760534-18163-1-git-send-email-julien.grall@linaro.org> <1418760534-18163-8-git-send-email-julien.grall@linaro.org> <549165F102000078000502B8@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Y1BsD-0000iq-OO for xen-devel@lists.xenproject.org; Wed, 17 Dec 2014 10:30:29 +0000 Received: by mail-wg0-f41.google.com with SMTP id y19so19816218wgg.28 for ; Wed, 17 Dec 2014 02:30:25 -0800 (PST) In-Reply-To: <549165F102000078000502B8@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Keir Fraser , ian.campbell@citrix.com, manish.jaggi@caviumnetworks.com, tim@xen.org, stefano.stabellini@citrix.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org Hi Jan, On 17/12/2014 10:16, Jan Beulich wrote: >>>> On 16.12.14 at 21:08, wrote: >> --- a/xen/common/Makefile >> +++ b/xen/common/Makefile >> @@ -2,6 +2,7 @@ obj-y += bitmap.o >> obj-y += core_parking.o >> obj-y += cpu.o >> obj-y += cpupool.o >> +obj-y += device.o > > Shouldn't this instead be two lines, one using HAS_PCI and the second > HAS_DEVICE_TREE? When ARM will gain PCI will, it will fail to compile because device.o is included twice. > >> @@ -75,8 +76,19 @@ struct pci_dev { >> #define PT_FAULT_THRESHOLD 10 >> } fault; >> u64 vf_rlen[6]; >> + >> + struct device dev; >> }; > > I'm not convinced yet that growing this structure (of which we have > quite many instances on some systems) is really worth it, in particular > on x86 where we (so far) only have one device type anyway. Actually this will growing by only sizeof (enum type) on x86. Having a generic way to describe device will really help ARM code (see IOMMU). If we don't have a such thing, we may need to duplicate quite a lots of code. Which will make hard to maintain. >> +#define pci_to_dev(pcidev) (&(pcidev)->dev) >> + >> +static inline struct pci_dev *dev_to_pci(struct device *dev) >> +{ >> + ASSERT(dev->type == DEV_PCI); >> + >> + return container_of(dev, struct pci_dev, dev); >> +} > > While the former is const-correct, I dislike the inability of passing > pointers to const into helper functions like the latter. I can't think > of a good solution other than introducing a second const variant > of it, but I suppose we should try to find alternatives before > adding such a construct that moves us in a direction opposite to > getting our code more const-correct. Oh right. I didn't though about that case. I will turn this inline function into a macro. Regards, -- Julien Grall