Re: [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

["Goel, Sameer" <sgoel@codeaurora.org>]

On 10/12/2017 8:06 AM, Julien Grall wrote:
> Hi Sameer,
> 
> On 21/09/17 01:37, Sameer Goel wrote:
>> Add support for parsing IORT table to initialize SMMU devices.
>> * The code for creating an SMMU device has been modified, so that the SMMU
>> device can be initialized.
>> * The NAMED NODE code has been commented out as this will need DOM0 kernel
>> support.
>> * ITS code has been included but it has not been tested.
>>
>> Signed-off-by: Sameer Goel <sgoel@codeaurora.org>
>> ---
>>   xen/arch/arm/setup.c               |   3 +
>>   xen/drivers/acpi/Makefile          |   1 +
>>   xen/drivers/acpi/arm/Makefile      |   1 +
>>   xen/drivers/acpi/arm/iort.c        | 173 +++++++++++++++++++++----------------
>>   xen/drivers/passthrough/arm/smmu.c |   1 +
>>   xen/include/acpi/acpi_iort.h       |  17 ++--
>>   xen/include/asm-arm/device.h       |   2 +
>>   xen/include/xen/acpi.h             |  21 +++++
>>   xen/include/xen/pci.h              |   8 ++
>>   9 files changed, 146 insertions(+), 81 deletions(-)
>>   create mode 100644 xen/drivers/acpi/arm/Makefile
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 92f173b..4ba09b2 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -49,6 +49,7 @@
>>   #include <asm/setup.h>
>>   #include <xsm/xsm.h>
>>   #include <asm/acpi.h>
>> +#include <acpi/acpi_iort.h>
>>     struct bootinfo __initdata bootinfo;
>>   @@ -796,6 +797,8 @@ void __init start_xen(unsigned long boot_phys_offset,
>>         tasklet_subsys_init();
>>   +    /* Parse the ACPI iort data */
>> +    acpi_iort_init();
>>         xsm_dt_init();
>>   diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile
>> index 444b11d..e7ffd82 100644
>> --- a/xen/drivers/acpi/Makefile
>> +++ b/xen/drivers/acpi/Makefile
>> @@ -1,5 +1,6 @@
>>   subdir-y += tables
>>   subdir-y += utilities
>> +subdir-$(CONFIG_ARM) += arm
>>   subdir-$(CONFIG_X86) += apei
>>     obj-bin-y += tables.init.o
>> diff --git a/xen/drivers/acpi/arm/Makefile b/xen/drivers/acpi/arm/Makefile
>> new file mode 100644
>> index 0000000..7c039bb
>> --- /dev/null
>> +++ b/xen/drivers/acpi/arm/Makefile
>> @@ -0,0 +1 @@
>> +obj-y += iort.o
>> diff --git a/xen/drivers/acpi/arm/iort.c b/xen/drivers/acpi/arm/iort.c
>> index 2e368a6..7f54062 100644
>> --- a/xen/drivers/acpi/arm/iort.c
>> +++ b/xen/drivers/acpi/arm/iort.c
>> @@ -14,17 +14,47 @@
>>    * This file implements early detection/parsing of I/O mapping
>>    * reported to OS through firmware via I/O Remapping Table (IORT)
>>    * IORT document number: ARM DEN 0049A
>> + *
>> + * Based on Linux drivers/acpi/arm64/iort.c
>> + * => commit ca78d3173cff3503bcd15723b049757f75762d15
>> + *
>> + * Xen modification:
>> + * Sameer Goel <sgoel@codeaurora.org>
>> + * Copyright (C) 2017, The Linux Foundation, All rights reserved.
>> + *
>>    */
>>   -#define pr_fmt(fmt)    "ACPI: IORT: " fmt
>> -
>> -#include <linux/acpi_iort.h>
>> -#include <linux/iommu.h>
>> -#include <linux/kernel.h>
>> -#include <linux/list.h>
>> -#include <linux/pci.h>
>> -#include <linux/platform_device.h>
>> -#include <linux/slab.h>
>> +#include <xen/acpi.h>
>> +#include <acpi/acpi_iort.h>
> 
> Why do you need to include there? Can't this be done after all the <xen/> ?
I was just clubbing the acpi includes. I can move this after all xen/
> 
>> +#include <xen/fwnode.h>
>> +#include <xen/iommu.h>
>> +#include <xen/lib.h>
>> +#include <xen/list.h>
>> +#include <xen/pci.h>
>> +
>> +#include <asm/device.h>
>> +
>> +/* Xen: Define compatibility functions */
>> +#define FW_BUG        "[Firmware Bug]: "
>> +#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
>> +#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
>> +
>> +/* Alias to Xen allocation helpers */
>> +#define kfree xfree
>> +#define kmalloc(size, flags)            _xmalloc(size, sizeof(void *))
>> +#define kzalloc(size, flags)            _xzalloc(size, sizeof(void *))
> 
> Likely you would need the same macros in the SMMUv3 driver. Could we think of a common headers implementing the Linux compat layer?
I agree that this will be a very useful. i'll try to propose something.
> 
>> +
>> +/* Redefine WARN macros */
>> +#undef WARN
>> +#undef WARN_ON
>> +#define WARN(condition, format...) ({                    \
>> +    int __ret_warn_on = !!(condition);                \
>> +    if (unlikely(__ret_warn_on))                    \
>> +        printk(format);                        \
>> +    unlikely(__ret_warn_on);                    \
>> +})
> 
> Again, you should at least try to modify the common code version before deciding to redefine it here.
The xen macro seems such that it explicitly tries to block a return by wrapping this macro in a loop. I had changed the common function in the last iteration and there seemed to be a pushback. 
> 
>> +#define WARN_TAINT(cond, taint, format...) WARN(cond, format)
>> +#define WARN_ON(cond)                      (!!cond)
>>     #define IORT_TYPE_MASK(type)    (1 << (type))
>>   #define IORT_MSI_TYPE        (1 << ACPI_IORT_NODE_ITS_GROUP)
>> @@ -256,6 +286,13 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
>>       acpi_status status;
>>         if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {
>> +        status = AE_NOT_IMPLEMENTED;
>> +/*
>> + * We need the namespace object name from dsdt to match the iort node, this
> 
> Please add a "Xen: TODO:" in front.
Ok.
> 
>> + * will need additions to the kernel xen bus notifiers.
>> + * So, disabling the named node code till a proposal is approved.
>> + */
>> +#if 0
>>           struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
>>           struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
>>           struct acpi_iort_named_component *ncomp;
>> @@ -275,11 +312,12 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
>>           status = !strcmp(ncomp->device_name, buf.pointer) ?
>>                               AE_OK : AE_NOT_FOUND;
>>           acpi_os_free(buf.pointer);
>> +#endif
>>       } else if (node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
>>           struct acpi_iort_root_complex *pci_rc;
>> -        struct pci_bus *bus;
>> +        struct pci_dev *pci_dev;
> 
> Do you really need to modify the code? Wouldn't it be possible to do
> 
> #define pci_bus pci_dev
The pci_dev is the container for the generic device. I wanted to call this out explicitly here. We can do the above if you insist :).
> 
> With an explanation why you do that on top.
> 
>>   -        bus = to_pci_bus(dev);
>> +        pci_dev = to_pci_dev(dev);
> 
> Same here?
> 
>>           pci_rc = (struct acpi_iort_root_complex *)node->node_data;
>>             /*
>> @@ -287,12 +325,11 @@ static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
>>            * with root complexes. Each segment number can represent only
>>            * one root complex.
>>            */
>> -        status = pci_rc->pci_segment_number == pci_domain_nr(bus) ?
>> +        status = pci_rc->pci_segment_number == pci_domain_nr(pci_dev) ?
>>                               AE_OK : AE_NOT_FOUND;
>>       } else {
>>           status = AE_NOT_FOUND;
>>       }
>> -out:
>>       return status;
>>   }
>>   @@ -320,6 +357,11 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
>>       return 0;
>>   }
>>   +/*
>> + * Named components are not supported yet so we do not need the
>> + * iort_node_get_id function
> 
> Missing full stop + TODO.
Ok.
> 
>> + */
>> +#if 0
>>   static
>>   struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>>                       u32 *id_out, u8 type_mask,
>> @@ -358,6 +400,7 @@ struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
>>         return NULL;
>>   }
>> +#endif
>>     static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
>>                           u32 rid_in, u32 *rid_out,
>> @@ -410,6 +453,10 @@ fail_map:
>>       return NULL;
>>   }
>>   +/* Xen: Comment out the NamedComponent and ITS mapping code till the support
> 
> + TODO here please.
> 
Ok.
>> + * is available.
>> + */
>> +#if 0
>>   static struct acpi_iort_node *iort_find_dev_node(struct device *dev)
>>   {
>>       struct pci_bus *pbus;
>> @@ -481,7 +528,7 @@ static int iort_dev_find_its_id(struct device *dev, u32 req_id,
>>       return 0;
>>   }
>>   -/**
>> +/*
> 
> Why this change?
Oversight.
> 
>>    * iort_get_device_domain() - Find MSI domain related to a device
>>    * @dev: The device.
>>    * @req_id: Requester ID for the device.
>> @@ -510,7 +557,7 @@ static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
>>       *rid = alias;
>>       return 0;
>>   }
>> -
>> +#endif
> 
> Please avoid dropping newline.
> 
>>   static int arm_smmu_iort_xlate(struct device *dev, u32 streamid,
>>                      struct fwnode_handle *fwnode,
>>                      const struct iommu_ops *ops)
>> @@ -546,6 +593,7 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
>>       return ret ? NULL : ops;
>>   }
>>   +#if 0 /* Xen: We do not need this function for Xen */
>>   /**
>>    * iort_set_dma_mask - Set-up dma mask for a device.
>>    *
>> @@ -567,7 +615,7 @@ void iort_set_dma_mask(struct device *dev)
>>       if (!dev->dma_mask)
>>           dev->dma_mask = &dev->coherent_dma_mask;
>>   }
>> -
>> +#endif
> 
> Same here.
> 
>>   /**
>>    * iort_iommu_configure - Set-up IOMMU configuration for a device.
>>    *
>> @@ -583,14 +631,13 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
>>       u32 streamid = 0;
>>         if (dev_is_pci(dev)) {
>> -        struct pci_bus *bus = to_pci_dev(dev)->bus;
>> +        struct pci_dev *pci_device = to_pci_dev(dev);
> 
> See above.
> 
>>           u32 rid;
>>   -        pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
>> -                       &rid);
>> +        rid = PCI_BDF2(pci_device->bus, pci_device->devfn);
> 
> I believe we had a discussion on v1 explaining why this is wrong. So I don't understand why
The intent here was to have some working support code to get the SMMUv3 driver ready for a first review. I still need to research on how to fix this correctly based on the v1 comments. I'll fix it in the next RFC.
> 
>>             node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
>> -                      iort_match_node_callback, &bus->dev);
>> +                      iort_match_node_callback, dev);
>>           if (!node)
>>               return NULL;
>>   @@ -600,6 +647,13 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
>>           ops = iort_iommu_xlate(dev, parent, streamid);
>>         } else {
>> +        return NULL;
>> +/*
>> + * We need the namespace object name from dsdt to match the iort node, this
> 
> Xen:
> 
>> + * will need additions to the kernel xen bus notifiers.
>> + * So, disabling the named node code till a proposal is approved.
>> + */
>> +#if 0
>>           int i = 0;
>>             node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
>> @@ -616,11 +670,17 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
>>               parent = iort_node_get_id(node, &streamid,
>>                             IORT_IOMMU_TYPE, i++);
>>           }
>> +#endif
>>       }
>>         return ops;
>>   }
>>   +/*
>> + * Xen: Not using the parsing ops for now. Need to check and see if it will
>> + * be useful to use these in some form, or let the driver parse IORT node.
>> + */
>> +#if 0
>>   static void __init acpi_iort_register_irq(int hwirq, const char *name,
>>                         int trigger,
>>                         struct resource *res)
>> @@ -807,7 +867,7 @@ const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
>>           return NULL;
>>       }
>>   }
>> -
>> +#endif
> 
> Please avoid dropping newline.
> 
>>   /**
>>    * iort_add_smmu_platform_device() - Allocate a platform device for SMMU
>>    * @node: Pointer to SMMU ACPI IORT node
>> @@ -817,78 +877,42 @@ const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
>>   static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
> 
> Looking at the changes in this function. You basically rewrite everything. I would prefer if you comment the current one and implement from scratch the Xen version.
Agree.
> 
>>   {
>>       struct fwnode_handle *fwnode;
>> -    struct platform_device *pdev;
>> -    struct resource *r;
>> -    enum dev_dma_attr attr;
>> -    int ret, count;
>> -    const struct iort_iommu_config *ops = iort_get_iommu_cfg(node);
>> -
>> -    if (!ops)
>> -        return -ENODEV;
>> -
>> -    pdev = platform_device_alloc(ops->name, PLATFORM_DEVID_AUTO);
>> -    if (!pdev)
>> -        return -ENOMEM;
>> -
>> -    count = ops->iommu_count_resources(node);
>> -
>> -    r = kcalloc(count, sizeof(*r), GFP_KERNEL);
>> -    if (!r) {
>> -        ret = -ENOMEM;
>> -        goto dev_put;
>> -    }
>> -
>> -    ops->iommu_init_resources(r, node);
>> +    struct device *dev;
>> +    int ret;
>>   -    ret = platform_device_add_resources(pdev, r, count);
>>       /*
>> -     * Resources are duplicated in platform_device_add_resources,
>> -     * free their allocated memory
>> +     * Not enabling the parsing ops for now. The corresponding driver
>> +     * can parse this information as needed, so deleting relevant code as
>> +     * compared to base revision.
>>        */
>> -    kfree(r);
>>   -    if (ret)
>> -        goto dev_put;
>> +    dev = kzalloc(sizeof(struct device), GFP_KERNEL);
>> +    if (!dev)
>> +        return -ENOMEM;
>>         /*
>>        * Add a copy of IORT node pointer to platform_data to
>>        * be used to retrieve IORT data information.
>>        */
>> -    ret = platform_device_add_data(pdev, &node, sizeof(node));
>> -    if (ret)
>> -        goto dev_put;
>> -
>> -    /*
>> -     * We expect the dma masks to be equivalent for
>> -     * all SMMUs set-ups
>> -     */
>> -    pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
>> +    dev->type = DEV_ACPI;
>> +    dev->acpi_node = node;
>>         fwnode = iort_get_fwnode(node);
>>         if (!fwnode) {
>>           ret = -ENODEV;
>> -        goto dev_put;
>> +        goto error;
>>       }
>>   -    pdev->dev.fwnode = fwnode;
>> -
>> -    attr = ops->iommu_is_coherent(node) ?
>> -                 DEV_DMA_COHERENT : DEV_DMA_NON_COHERENT;
>> -
>> -    /* Configure DMA for the page table walker */
>> -    acpi_dma_configure(&pdev->dev, attr);
>> +    dev->fwnode = fwnode;
>>   -    ret = platform_device_add(pdev);
>> -    if (ret)
>> -        goto dma_deconfigure;
>> +    /* Call the acpi init functions for IOMMU devices */
>> +    ret = acpi_device_init(DEVICE_IOMMU, (void *)dev, node->type);
>>         return 0;
>>   -dma_deconfigure:
>> -    acpi_dma_deconfigure(&pdev->dev);
>> -dev_put:
>> -    platform_device_put(pdev);
>> +error:
>> +    kfree(dev);
>>         return ret;
>>   }
>> @@ -957,5 +981,6 @@ void __init acpi_iort_init(void)
>>         iort_init_platform_devices();
>>   -    acpi_probe_device_table(iort);
>> +    /* Xen; Do not need a device table probe */
>> +    /* acpi_probe_device_table(iort);*/
> 
> Please use either
> 
> #if 0
> 
> #endif
> 
> or introduce a dummy acpi_probe_device_table(...) at the start.
Will introduce a dummy acpi_probe.
> 
>>   }
>> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
>> index 362d578..ad956d5 100644
>> --- a/xen/drivers/passthrough/arm/smmu.c
>> +++ b/xen/drivers/passthrough/arm/smmu.c
>> @@ -181,6 +181,7 @@ static void __iomem *devm_ioremap_resource(struct device *dev,
>>    * Xen: PCI functions
>>    * TODO: It should be implemented when PCI will be supported
>>    */
>> +#undef to_pci_dev
> 
> Why this change?
I had redefine the to_pci_dev to get the actual pci_dev struct. smmu driver does not use pci yet.
> 
>>   #define to_pci_dev(dev)    (NULL)
>>   static inline int pci_for_each_dma_alias(struct pci_dev *pdev,
>>                        int (*fn) (struct pci_dev *pdev,
>> diff --git a/xen/include/acpi/acpi_iort.h b/xen/include/acpi/acpi_iort.h
>> index 77e0809..d4315a4 100644
>> --- a/xen/include/acpi/acpi_iort.h
>> +++ b/xen/include/acpi/acpi_iort.h
> 
> You probably want to re-sync this headers as it changed quite a bit and would avoid some specific #if 0 for Xen.
> 
>> @@ -19,27 +19,32 @@
>>   #ifndef __ACPI_IORT_H__
>>   #define __ACPI_IORT_H__
>>   -#include <linux/acpi.h>
>> -#include <linux/fwnode.h>
>> -#include <linux/irqdomain.h>
>> +#include <xen/acpi.h>
>> +#include <asm/device.h>
>>   +/* Xen: Not using IORT IRQ bindings */
>> +#if 0
>>   #define IORT_IRQ_MASK(irq)        (irq & 0xffffffffULL)
>>   #define IORT_IRQ_TRIGGER_MASK(irq)    ((irq >> 32) & 0xffffffffULL)
>>     int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node);
>>   void iort_deregister_domain_token(int trans_id);
>>   struct fwnode_handle *iort_find_domain_token(int trans_id);
>> -#ifdef CONFIG_ACPI_IORT
>> +#endif
>> +#ifdef CONFIG_ARM_64
> 
> As said in the first version, I see no point of replacing CONFIG_ACPI_IORT with CONFIG_ARM_64. You should instead take advantage of the Kconfig to add a new config ACPI_IORT and select on Arm64 with ACPI.

I agree. I saw your comments a bit late for this RFC. I will add this to the next set.
> 
>>   void acpi_iort_init(void);
>>   bool iort_node_match(u8 type);
>> +#if 0
>>   u32 iort_msi_map_rid(struct device *dev, u32 req_id);
>>   struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id);
>>   /* IOMMU interface */
>>   void iort_set_dma_mask(struct device *dev);
>> +#endif
>>   const struct iommu_ops *iort_iommu_configure(struct device *dev);
>>   #else
>>   static inline void acpi_iort_init(void) { }
>>   static inline bool iort_node_match(u8 type) { return false; }
>> +#if 0
>>   static inline u32 iort_msi_map_rid(struct device *dev, u32 req_id)
>>   { return req_id; }
>>   static inline struct irq_domain *iort_get_device_domain(struct device *dev,
>> @@ -47,12 +52,10 @@ static inline struct irq_domain *iort_get_device_domain(struct device *dev,
>>   { return NULL; }
>>   /* IOMMU interface */
>>   static inline void iort_set_dma_mask(struct device *dev) { }
>> +#endif
>>   static inline
>>   const struct iommu_ops *iort_iommu_configure(struct device *dev)
>>   { return NULL; }
>>   #endif
>>   -#define IORT_ACPI_DECLARE(name, table_id, fn)        \
>> -    ACPI_DECLARE_PROBE_ENTRY(iort, name, table_id, 0, NULL, 0, fn)
>> -
>>   #endif /* __ACPI_IORT_H__ */
>> diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
>> index 5027c87..4eef9ce 100644
>> --- a/xen/include/asm-arm/device.h
>> +++ b/xen/include/asm-arm/device.h
>> @@ -7,6 +7,7 @@
>>   enum device_type
>>   {
>>       DEV_DT,
>> +    DEV_ACPI,
>>   };
>>     struct dev_archdata {
>> @@ -20,6 +21,7 @@ struct device
>>   #ifdef CONFIG_HAS_DEVICE_TREE
>>       struct dt_device_node *of_node; /* Used by drivers imported from Linux */
>>   #endif
>> +    void *acpi_node; /*Current use case is acpi_iort_node */
> 
> Can you explain why you need that? After the creation of fwnode, I was expecting of_node to disappear. So I don't really fancy see acpi_node here more it does not exist in Linux.
of_node stays for backwards compatibility with smmu v2 driver. I will try to find a way to remove acpi_node.
> 
>>       struct fwnode_handle *fwnode; /*fw device node identifier */
>>       struct iommu_fwspec *iommu_fwspec;
>>       struct dev_archdata archdata;
>> diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h
>> index 9409350..2f6aae1 100644
>> --- a/xen/include/xen/acpi.h
>> +++ b/xen/include/xen/acpi.h
>> @@ -32,6 +32,7 @@
>>     #include <acpi/acpi.h>
>>   #include <asm/acpi.h>
>> +#include <xen/fwnode.h>
> 
> I think this and ...
> 
>>     #define ACPI_MADT_GET_(fld, x) (((x) & ACPI_MADT_##fld##_MASK) / \
>>       (ACPI_MADT_##fld##_MASK & -ACPI_MADT_##fld##_MASK))
>> @@ -49,6 +50,26 @@
>>                   (!(entry)) || (unsigned long)(entry) + sizeof(*(entry)) > (end) ||  \
>>                   (entry)->header.length < sizeof(*(entry)))
>>   +static inline struct fwnode_handle *acpi_alloc_fwnode_static(void)
>> +{
>> +    struct fwnode_handle *fwnode;
>> +
>> +    fwnode = xzalloc(struct fwnode_handle);
>> +    if (!fwnode)
>> +        return NULL;
>> +
>> +    fwnode->type = FWNODE_ACPI_STATIC;
>> +
>> +    return fwnode;
>> +}
>> +
>> +static inline void acpi_free_fwnode_static(struct fwnode_handle *fwnode)
>> +{
>> +    if (!fwnode || fwnode->type != FWNODE_ACPI_STATIC)
>> +        return;
>> +
>> +    xfree(fwnode);
>> +}
> 
> ... those 2 helpers should go in asm/acpi.h.
Ok.
> 
>>   #ifdef CONFIG_ACPI
>>     enum acpi_interrupt_id {
>> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
>> index 43f2125..182b1a5 100644
>> --- a/xen/include/xen/pci.h
>> +++ b/xen/include/xen/pci.h
>> @@ -92,8 +92,16 @@ struct pci_dev {
>>   #define PT_FAULT_THRESHOLD 10
>>       } fault;
>>       u64 vf_rlen[6];
>> +#ifdef CONFIG_ARM
>> +    struct device dev;
>> +#endif
> 
> There are a part of PCI that is already per-arch. See arch_pci_dev in asm-arm/pci.h.
> 
> Please define the field dev in it rather than here.
Ok, I'll check if I can do this.
> 
>>   };
>>   +#ifdef CONFIG_ARM
>> +#define to_pci_dev(p) container_of(p, struct pci_dev,dev)
>> +#define pci_domain_nr(dev) dev->seg
>> +#endif
> 
> Similarly, this should be moved in asm-arm/pci.h.
Ok.
> 
>> +
>>   #define for_each_pdev(domain, pdev) \
>>       list_for_each_entry(pdev, &(domain->arch.pdev_list), domain_list)
>>  
> 
> Cheers,
> 

-- 
 Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

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