[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/2] xen/arm: Make HAS_PCI compilable on ARM by adding place-holder code



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 <manish.jaggi@xxxxxxxxxxxxxxxxxx>
> ---
>  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 <xen/pci.h>
> +
> +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.


> +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



> 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 <manish.jaggi@xxxxxxxxxx>
> + */
> +#include <xen/pci.h>
> +#include <asm/device.h>
> +#include <xen/sched.h>
> +
> +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

    
> +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?


> 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 <xen/device_tree.h>
>  -/* 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
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.