From mboxrd@z Thu Jan 1 00:00:00 1970 From: Manish Jaggi Subject: Re: [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code Date: Mon, 13 Apr 2015 16:01:41 +0530 Message-ID: <552B9B0D.5000608@caviumnetworks.com> References: <552B74B3.6000702@caviumnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: "Prasun.kapoor@cavium.com" , "Kumar, Vijaya" , Julien Grall , Ian Campbell , Xen Devel List-Id: xen-devel@lists.xenproject.org On Monday 13 April 2015 03:44 PM, Stefano Stabellini wrote: > On Mon, 13 Apr 2015, 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/arch/arm/Makefile b/xen/arch/arm/Makefile >> index 935999e..aaf5d88 100644 >> --- a/xen/arch/arm/Makefile >> +++ b/xen/arch/arm/Makefile >> @@ -39,6 +39,7 @@ obj-y += device.o >> obj-y += decode.o >> obj-y += processor.o >> obj-y += smc.o >> +obj-$(HAS_PCI) += pci.o >> #obj-bin-y += ....o >> diff --git a/xen/arch/arm/pci.c b/xen/arch/arm/pci.c >> new file mode 100644 >> index 0000000..9438f41 >> --- /dev/null >> +++ b/xen/arch/arm/pci.c >> @@ -0,0 +1,60 @@ >> +#include >> + >> +void _pci_conf_write( >> + uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func, >> + uint32_t reg, uint32_t data, int bytes) >> +{ >> +} >> + >> +uint32_t _pci_conf_read( >> + uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func, >> + uint32_t reg, int bytes) >> +{ >> + return 0; >> +} > I guess that they are going to be implemented with platform specific > code? Although I thought that the mmcfg stuff is somewhat standard across > architectures. > yes. _pci_conf_read/write will call pcihb_conf_read/write (pcihb = pci_host_bridge). ref: http://lists.xen.org/archives/html/xen-devel/2015-02/msg02717.html >> +uint8_t pci_conf_read8( >> + uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func, >> + uint32_t reg) >> +{ >> + return (uint8_t)_pci_conf_read(seg, bus, dev, func, reg, 1); >> +} >> + >> +uint16_t pci_conf_read16( >> + uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func, >> + uint32_t reg) >> +{ >> + return (uint8_t)_pci_conf_read(seg, bus, dev, func, reg, 2); >> +} >> + >> +uint32_t pci_conf_read32( >> + uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func, >> + uint32_t reg) >> +{ >> + return (uint8_t)_pci_conf_read(seg, bus, dev, func, reg, 4); >> +} >> + >> +void pci_conf_write8( >> + uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func, >> + uint32_t reg, uint8_t data) >> +{ >> + _pci_conf_write(seg, bus, dev, func, reg, data, 1); >> +} >> + >> +void pci_conf_write16( >> + uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func, >> + uint32_t reg, uint16_t data) >> +{ >> + _pci_conf_write(seg, bus, dev, func, reg, data, 2); >> +} >> + >> +void pci_conf_write32( >> + uint32_t seg, uint32_t bus, uint32_t dev, uint32_t func, >> + uint32_t reg, uint32_t data) >> +{ >> + _pci_conf_write(seg, bus, dev, func, reg, data, 4); >> +} >> + >> +void arch_pci_ro_device(int seg, int bdf) >> +{ >> +} > This is also missing an implementation. Maybe you should add /* TODO */ > here too ok. > > >> diff --git a/xen/drivers/passthrough/arm/Makefile >> b/xen/drivers/passthrough/arm/Makefile >> index f4cd26e..1a41549 100644 >> --- a/xen/drivers/passthrough/arm/Makefile >> +++ b/xen/drivers/passthrough/arm/Makefile >> @@ -1,2 +1,3 @@ >> obj-y += iommu.o >> obj-y += smmu.o >> +obj-$(HAS_PCI) += pci.o >> diff --git a/xen/drivers/passthrough/arm/pci.c >> b/xen/drivers/passthrough/arm/pci.c >> new file mode 100644 >> index 0000000..07a5a78 >> --- /dev/null >> +++ b/xen/drivers/passthrough/arm/pci.c >> @@ -0,0 +1,88 @@ >> +/* >> + * PCI helper functions for arm for passthrough support. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, >> USA. >> + * >> + * Copyright (C) 2015 Cavium Networks >> + * >> + * Author: Manish Jaggi >> + */ >> +#include >> +#include >> +#include >> + >> +int _dump_pci_devices(struct pci_seg *pseg, void *arg) >> +{ >> + struct pci_dev *pdev; >> + >> + printk("==== segment %04x ====\n", pseg->nr); >> + >> + list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list ) >> + { >> + printk("%04x:%02x:%02x.%u - dom %-3d - node %-3d - MSIs < ", >> + pseg->nr, pdev->bus, >> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), >> + pdev->domain ? pdev->domain->domain_id : -1, >> + (pdev->node != NUMA_NO_NODE) ? pdev->node : -1); >> + printk(">\n"); >> + } >> + >> + return 0; >> +} >> + >> +struct device* pci_to_dev(struct pci_dev *pci) >> +{ >> + if(!pci->arch.dev) >> + { >> + device_t *dev; >> + dev = xzalloc (struct device); >> + pci->arch.dev = dev; >> + dev->class = DEVICE_PCI; >> + dev->type = DEV_ENUMERATED; >> + } >> + return pci->arch.dev; >> +} > There must be a better place to initialize pci->arch.dev than here ok will check. > >> +struct pci_dev* to_pci_dev(struct device *dev) >> +{ >> + if(dev->class == DEVICE_PCI) >> + return dev->pci_dev; >> + return NULL; >> +} >> + >> +int dev_is_pci(struct device *dev) >> +{ >> + if(dev->class == DEVICE_PCI) >> + return 1; >> + return 0; >> +} >> + >> + >> +int pci_clean_dpci_irqs(struct domain *d) >> +{ >> + /* This is a placeholder function */ >> + return 0; >> +} >> + >> +struct pci_dev* pci_alloc_msix(struct pci_dev *pdev, struct pci_seg *pseg, >> + u8 bus, u8 devfn) >> +{ >> + /* This is a placeholder function. It is implemented only on x86 */ >> + return 0; >> +} >> + >> +void pci_cleanup_msi(struct pci_dev *pdev) >> +{ >> + /* This is a placeholder function. It is implemented only on x86 */ >> +} > Although they are just placeholders for now, you are planning to > implement them on ARM, right? > > No. As MSIs are handled by guest directly in ARM. There are added to make compilation clean. >> diff --git a/xen/drivers/passthrough/arm/smmu.c >> b/xen/drivers/passthrough/arm/smmu.c >> index 8a9b58b..1406261 100644 >> --- a/xen/drivers/passthrough/arm/smmu.c >> +++ b/xen/drivers/passthrough/arm/smmu.c >> @@ -183,7 +183,6 @@ static void __iomem *devm_ioremap_resource(struct device >> *dev, >> * Xen: PCI functions >> * TODO: It should be implemented when PCI will be supported >> */ >> -#define to_pci_dev(dev) (NULL) >> static inline int pci_for_each_dma_alias(struct pci_dev *pdev, >> int (*fn) (struct pci_dev *pdev, >> u16 alias, void *data), >> 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 >> + )) ) >> 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, >> }; >> 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; >> +#endif >> }; >> typedef struct device device_t; >> #include >> -/* TODO: Correctly implement dev_is_pci when PCI is supported on ARM */ >> -#define dev_is_pci(dev) ((void)(dev), 0) >> #define dev_is_dt(dev) ((dev->type == DEV_DT) >> -enum device_class >> -{ >> - DEVICE_SERIAL, >> - DEVICE_IOMMU, >> - DEVICE_GIC, >> - /* Use for error */ >> - DEVICE_UNKNOWN, >> -}; >> +struct pci_dev; >> +device_t* pci_to_dev(struct pci_dev *pci); >> +#ifndef HAS_PCI >> +#define to_pci_dev(dev) (NULL) >> +#else >> +struct pci_dev* to_pci_dev(device_t *dev); >> +#endif >> +int dev_is_pci(device_t *dev); >> 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; >> }; >> -#endif /* __X86_PCI_H__ */ >> +#endif /* __ARM_PCI_H__ */ >> -- >> 1.7.9.5 >>