From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jaggi, Manish" Subject: Re: [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code Date: Tue, 14 Apr 2015 01:28:53 +0000 Message-ID: <1428974939119.6172@caviumnetworks.com> References: <552B74B3.6000702@caviumnetworks.com>, <552B9DAA.9030706@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <552B9DAA.9030706@citrix.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall , Xen Devel , Stefano Stabellini , Julien Grall , Ian Campbell , "Kumar, Vijaya" , "Prasun.kapoor@cavium.com" List-Id: xen-devel@lists.xenproject.org Hi Julien, ________________________________________ From: xen-devel-bounces@lists.xen.org on behalf of Julien Grall Sent: Monday, April 13, 2015 4:12 PM To: Jaggi, Manish; Xen Devel; Stefano Stabellini; Julien Grall; Ian Campbell; Kumar, Vijaya; Prasun.kapoor@cavium.com Subject: Re: [Xen-devel] [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code Hi Manish, On 13/04/15 08:48, Manish Jaggi wrote: > Add ARM PCI Support > --------------- > a) Place holder functions are added for pci_conf_read/write calls. > b) Macros dev_is_pci, pci_to_dev are implemented in > drivers/passthrough/pci/arm code > > Signed-off-by: Manish Jaggi > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/pci.c | 60 +++++++++++++++++++++++ > xen/drivers/passthrough/arm/Makefile | 1 + > xen/drivers/passthrough/arm/pci.c | 88 > ++++++++++++++++++++++++++++++++++ > xen/drivers/passthrough/arm/smmu.c | 1 - > xen/drivers/passthrough/pci.c | 9 ++-- > xen/include/asm-arm/device.h | 33 +++++++++---- > xen/include/asm-arm/domain.h | 3 ++ > xen/include/asm-arm/pci.h | 7 +-- > 9 files changed, 186 insertions(+), 17 deletions(-) > [...] > diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c > index 004aba9..68c292b 100644 > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -1252,9 +1252,12 @@ static int assign_device(struct domain *d, u16 > seg, u8 bus, u8 devfn) > /* Prevent device assign if mem paging or mem sharing have been > * enabled for this domain */ > if ( unlikely(!need_iommu(d) && > - (d->arch.hvm_domain.mem_sharing_enabled || > - d->mem_event->paging.ring_page || > - p2m_get_hostp2m(d)->global_logdirty)) ) > + (d->mem_event->paging.ring_page > +#ifdef CONFIG_X86 > + || d->arch.hvm_domain.mem_sharing_enabled || > + p2m_get_hostp2m(d)->global_logdirty > +#endif > + )) ) Please avoid #ifdef CONFIG_* and introduce an arch macro. [Manish] ok > return -EXDEV; > > if ( !spin_trylock(&pcidevs_lock) ) > diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h > index a72f7c9..009ff0a 100644 > --- a/xen/include/asm-arm/device.h > +++ b/xen/include/asm-arm/device.h > @@ -6,6 +6,17 @@ > enum device_type > { > DEV_DT, > + DEV_ENUMERATED, > +}; > + > +enum device_class > +{ > + DEVICE_SERIAL, > + DEVICE_IOMMU, > + DEVICE_GIC, > + DEVICE_PCI, > + /* Use for error */ > + DEVICE_UNKNOWN, > }; Why? It will be very unlikely that we have to create a structure device for the IOMMU, GIC and SERIAL. It would make more sense to add a DEV_PCI directly to device_type. [manish] ok. > > struct dev_archdata { > @@ -16,28 +27,30 @@ struct dev_archdata { > struct device > { > enum device_type type; > + enum device_class class; > #ifdef HAS_DEVICE_TREE > struct dt_device_node *of_node; /* Used by drivers imported from > Linux */ > #endif > struct dev_archdata archdata; > +#ifdef HAS_PCI > + void *pci_dev; This field is not necessary. I've added one for the DT for keeping compatibility with Linux. [Manish] pci_dev is not in struct device currently. Didn't get you I have added as It is required for to_pci_dev call. > +#endif [..] > +int dev_is_pci(device_t *dev); This could easily be a macro. [manish] ok > > struct device_desc { > /* Device name */ > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > index 9e0419e..41ae847 100644 > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -42,6 +42,8 @@ struct vtimer { > uint64_t cval; > }; > > +#define has_arch_pdevs(d) (!list_empty(&(d)->arch.pdev_list)) > + > struct arch_domain > { > #ifdef CONFIG_ARM_64 > @@ -56,6 +58,7 @@ struct arch_domain > xen_pfn_t *grant_table_gpfn; > > struct io_handler io_handlers; > + struct list_head pdev_list; > /* Continuable domain_relinquish_resources(). */ > enum { > RELMEM_not_started, > diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h > index de13359..b8ec882 100644 > --- a/xen/include/asm-arm/pci.h > +++ b/xen/include/asm-arm/pci.h > @@ -1,7 +1,8 @@ > -#ifndef __X86_PCI_H__ > -#define __X86_PCI_H__ > +#ifndef __ARM_PCI_H__ > +#define __ARM_PCI_H__ > > struct arch_pci_dev { > + void *dev; void * is error-prone. Why can't you use the use the real structure? [manish]Will change it. I believe dev_archdata structure has also a void * (in asm-arm/device.h). So all void * are error prone in xen ? Regards, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel