diff mbox

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

Message ID 1505954230-18892-6-git-send-email-sgoel@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Goel, Sameer Sept. 21, 2017, 12:37 a.m. UTC
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

Comments

Manish Jaggi Oct. 10, 2017, 12:36 p.m. UTC | #1
Hi Sameer,
On 9/21/2017 6:07 AM, 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.
Could you please refactor this patch into another set of two patches.
I am planning to rebase my IORT for Dom0 Hiding patch rework on this patch.
Thanks,
Manish
> 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>
> +#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 *))
> +
> +/* 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);					\
> +})
> +#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
> + * 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;
>   
> -		bus = to_pci_bus(dev);
> +		pci_dev = to_pci_dev(dev);
>   		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
> + */
> +#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
> + * 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;
>   }
>   
> -/**
> +/*
>    * 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
>   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
>   /**
>    * 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);
>   		u32 rid;
>   
> -		pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
> -				       &rid);
> +		rid = PCI_BDF2(pci_device->bus, pci_device->devfn);
>   
>   		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
> + * 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
>   /**
>    * 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)
>   {
>   	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);*/
>   }
> 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
>   #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
> @@ -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
>   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 */
>       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>
>   
>   #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);
> +}
>   #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
>   };
>   
> +#ifdef CONFIG_ARM
> +#define to_pci_dev(p) container_of(p, struct pci_dev,dev)
> +#define pci_domain_nr(dev) dev->seg
> +#endif
> +
>   #define for_each_pdev(domain, pdev) \
>       list_for_each_entry(pdev, &(domain->arch.pdev_list), domain_list)
>
Julien Grall Oct. 12, 2017, 2:06 p.m. UTC | #2
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/> ?

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

> +
> +/* 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.

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

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

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.

> + */
> +#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.

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

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

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

>   {
>   	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.

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

>   #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.

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

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

>   #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.

>   };
>   
> +#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.

> +
>   #define for_each_pdev(domain, pdev) \
>       list_for_each_entry(pdev, &(domain->arch.pdev_list), domain_list)
>   
> 

Cheers,
Julien Grall Oct. 12, 2017, 2:23 p.m. UTC | #3
Hi Sameer,

On 21/09/17 01:37, Sameer Goel wrote:
> @@ -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);
>   		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 forgot to answer on this bit. On the previous it was mentioned this 
was wrong, but still there.

Whilst I can understand that implementing pci_for_each_dma_alias could 
require some work in Xen, I don't want to see rid = PCI_BDF2(...) 
spreading the code. So what's the plan?

Cheers,
Goel, Sameer Oct. 19, 2017, 3 p.m. UTC | #4
On 10/10/2017 6:36 AM, Manish Jaggi wrote:
> Hi Sameer,
> On 9/21/2017 6:07 AM, 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.
> Could you please refactor this patch into another set of two patches.
> I am planning to rebase my IORT for Dom0 Hiding patch rework on this patch.

I will try to break this up. Lets discuss this a bit more next week.
> Thanks,
> Manish
>> 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>
>> +#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 *))
>> +
>> +/* 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);                    \
>> +})
>> +#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
>> + * 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;
>>   -        bus = to_pci_bus(dev);
>> +        pci_dev = to_pci_dev(dev);
>>           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
>> + */
>> +#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
>> + * 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;
>>   }
>>   -/**
>> +/*
>>    * 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
>>   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
>>   /**
>>    * 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);
>>           u32 rid;
>>   -        pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
>> -                       &rid);
>> +        rid = PCI_BDF2(pci_device->bus, pci_device->devfn);
>>             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
>> + * 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
>>   /**
>>    * 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)
>>   {
>>       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);*/
>>   }
>> 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
>>   #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
>> @@ -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
>>   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 */
>>       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>
>>     #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);
>> +}
>>   #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
>>   };
>>   +#ifdef CONFIG_ARM
>> +#define to_pci_dev(p) container_of(p, struct pci_dev,dev)
>> +#define pci_domain_nr(dev) dev->seg
>> +#endif
>> +
>>   #define for_each_pdev(domain, pdev) \
>>       list_for_each_entry(pdev, &(domain->arch.pdev_list), domain_list)
>>   
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
Goel, Sameer Oct. 19, 2017, 3:21 p.m. UTC | #5
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,
>
Manish Jaggi Oct. 20, 2017, 6:25 a.m. UTC | #6
On 10/19/2017 8:30 PM, Goel, Sameer wrote:
> On 10/10/2017 6:36 AM, Manish Jaggi wrote:
>> Hi Sameer,
>> On 9/21/2017 6:07 AM, 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.
>> Could you please refactor this patch into another set of two patches.
>> I am planning to rebase my IORT for Dom0 Hiding patch rework on this patch.
> I will try to break this up. Lets discuss this a bit more next week.
Please have a look at the draft design. [1]
[1] https://www.mail-archive.com/xen-devel@lists.xen.org/msg125951.html
>> Thanks,
>> Manish
>>> 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>
>>> +#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 *))
>>> +
>>> +/* 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);                    \
>>> +})
>>> +#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
>>> + * 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;
>>>    -        bus = to_pci_bus(dev);
>>> +        pci_dev = to_pci_dev(dev);
>>>            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
>>> + */
>>> +#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
>>> + * 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;
>>>    }
>>>    -/**
>>> +/*
>>>     * 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
>>>    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
>>>    /**
>>>     * 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);
>>>            u32 rid;
>>>    -        pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
>>> -                       &rid);
>>> +        rid = PCI_BDF2(pci_device->bus, pci_device->devfn);
>>>              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
>>> + * 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
>>>    /**
>>>     * 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)
>>>    {
>>>        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);*/
>>>    }
>>> 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
>>>    #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
>>> @@ -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
>>>    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 */
>>>        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>
>>>      #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);
>>> +}
>>>    #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
>>>    };
>>>    +#ifdef CONFIG_ARM
>>> +#define to_pci_dev(p) container_of(p, struct pci_dev,dev)
>>> +#define pci_domain_nr(dev) dev->seg
>>> +#endif
>>> +
>>>    #define for_each_pdev(domain, pdev) \
>>>        list_for_each_entry(pdev, &(domain->arch.pdev_list), domain_list)
>>>    
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> https://lists.xen.org/xen-devel
Julien Grall Oct. 24, 2017, 2:26 p.m. UTC | #7
Hi Sameer,

On 19/10/17 16:21, Goel, Sameer wrote:
> On 10/12/2017 8:06 AM, Julien Grall wrote:
>>> +
>>> +/* 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.

I don't think there was any pushback on changing the common code. Jan 
Beulich validly requested to move that change in a separate patch.

>>
>>> +#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 :).

I agree this can be confusing to read. But that change is not justified 
for the goal you want to achieve. E.g importing the code from Linux and 
keep in sync in the future.

With a comment on top of the definitions, you could explain when pci_bus 
= pci_dev at the moment.

[...]

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

That should go in a separate commit in that case with probably all stub 
for PCI you added (see the pci.h).

The reason behind is those changes are not directly related to this 
patch. Patch should be kept simple and do one thing only. This makes 
easier to review.

Cheers,
Manish Jaggi Nov. 8, 2017, 2:41 p.m. UTC | #8
Hi Sameer

On 9/21/2017 6:07 AM, 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>
Followup of the discussions we had on iort parsing and querying streamID 
and deviceId based on RID.
I have extended your patchset with a patch that provides an alternative
way of parsing iort into maps : {rid-streamid}, {rid-deviceID)
which can directly be looked up for searching streamId for a rid. This
will remove the need to traverse iort table again.

The test patch just describes the proposed flow and how the parsing and
query code might fit in. I have not tested it.
The code only compiles.

https://github.com/mjaggi-cavium/xen-wip/commit/df006d64bdbb5c8344de5a710da8bf64c9e8edd5
(This repo has all 7 of your patches + test code patch merged.

Note: The commit text of the patch describes the basic flow /assumptions 
/ usage of functions.
Please see the code along with the v2 design draft.
[RFC] [Draft Design v2] ACPI/IORT Support in Xen.
https://lists.xen.org/archives/html/xen-devel/2017-11/msg00512.html

I seek your advice on this. Please provide your feedback.

Thanks
Manish


> ---
>   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>
> +#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 *))
> +
> +/* 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);					\
> +})
> +#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
> + * 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;
>   
> -		bus = to_pci_bus(dev);
> +		pci_dev = to_pci_dev(dev);
>   		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
> + */
> +#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
> + * 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;
>   }
>   
> -/**
> +/*
>    * 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
>   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
>   /**
>    * 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);
>   		u32 rid;
>   
> -		pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
> -				       &rid);
> +		rid = PCI_BDF2(pci_device->bus, pci_device->devfn);
>   
>   		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
> + * 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
>   /**
>    * 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)
>   {
>   	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);*/
>   }
> 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
>   #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
> @@ -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
>   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 */
>       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>
>   
>   #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);
> +}
>   #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
>   };
>   
> +#ifdef CONFIG_ARM
> +#define to_pci_dev(p) container_of(p, struct pci_dev,dev)
> +#define pci_domain_nr(dev) dev->seg
> +#endif
> +
>   #define for_each_pdev(domain, pdev) \
>       list_for_each_entry(pdev, &(domain->arch.pdev_list), domain_list)
>
Goel, Sameer Nov. 15, 2017, 1:27 a.m. UTC | #9
On 11/8/2017 7:41 AM, Manish Jaggi wrote:
> Hi Sameer
> 
> On 9/21/2017 6:07 AM, 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>
> Followup of the discussions we had on iort parsing and querying streamID and deviceId based on RID.
> I have extended your patchset with a patch that provides an alternative
> way of parsing iort into maps : {rid-streamid}, {rid-deviceID)
> which can directly be looked up for searching streamId for a rid. This
> will remove the need to traverse iort table again.
> 
> The test patch just describes the proposed flow and how the parsing and
> query code might fit in. I have not tested it.
> The code only compiles.
> 
> https://github.com/mjaggi-cavium/xen-wip/commit/df006d64bdbb5c8344de5a710da8bf64c9e8edd5
> (This repo has all 7 of your patches + test code patch merged.
> 
> Note: The commit text of the patch describes the basic flow /assumptions / usage of functions.
> Please see the code along with the v2 design draft.
> [RFC] [Draft Design v2] ACPI/IORT Support in Xen.
> https://lists.xen.org/archives/html/xen-devel/2017-11/msg00512.html
> 
> I seek your advice on this. Please provide your feedback.
I responded back on the other thread. I think we are fixing something that is not broken. I will try to post a couple of new RFCs and let's discuss this with incremental changes on the mailing list.

Thanks,
Sameer
> 
> Thanks
> Manish
> 
> 
>> ---
>>   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>
>> +#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 *))
>> +
>> +/* 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);                    \
>> +})
>> +#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
>> + * 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;
>>   -        bus = to_pci_bus(dev);
>> +        pci_dev = to_pci_dev(dev);
>>           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
>> + */
>> +#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
>> + * 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;
>>   }
>>   -/**
>> +/*
>>    * 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
>>   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
>>   /**
>>    * 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);
>>           u32 rid;
>>   -        pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
>> -                       &rid);
>> +        rid = PCI_BDF2(pci_device->bus, pci_device->devfn);
>>             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
>> + * 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
>>   /**
>>    * 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)
>>   {
>>       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);*/
>>   }
>> 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
>>   #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
>> @@ -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
>>   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 */
>>       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>
>>     #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);
>> +}
>>   #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
>>   };
>>   +#ifdef CONFIG_ARM
>> +#define to_pci_dev(p) container_of(p, struct pci_dev,dev)
>> +#define pci_domain_nr(dev) dev->seg
>> +#endif
>> +
>>   #define for_each_pdev(domain, pdev) \
>>       list_for_each_entry(pdev, &(domain->arch.pdev_list), domain_list)
>>   
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
Julien Grall Nov. 15, 2017, 8:58 a.m. UTC | #10
Hi Sameer,

On 11/15/2017 01:27 AM, Goel, Sameer wrote:
> 
> 
> On 11/8/2017 7:41 AM, Manish Jaggi wrote:
>> Hi Sameer
>>
>> On 9/21/2017 6:07 AM, 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>
>> Followup of the discussions we had on iort parsing and querying streamID and deviceId based on RID.
>> I have extended your patchset with a patch that provides an alternative
>> way of parsing iort into maps : {rid-streamid}, {rid-deviceID)
>> which can directly be looked up for searching streamId for a rid. This
>> will remove the need to traverse iort table again.
>>
>> The test patch just describes the proposed flow and how the parsing and
>> query code might fit in. I have not tested it.
>> The code only compiles.
>>
>> https://github.com/mjaggi-cavium/xen-wip/commit/df006d64bdbb5c8344de5a710da8bf64c9e8edd5
>> (This repo has all 7 of your patches + test code patch merged.
>>
>> Note: The commit text of the patch describes the basic flow /assumptions / usage of functions.
>> Please see the code along with the v2 design draft.
>> [RFC] [Draft Design v2] ACPI/IORT Support in Xen.
>> https://lists.xen.org/archives/html/xen-devel/2017-11/msg00512.html
>>
>> I seek your advice on this. Please provide your feedback.
> I responded back on the other thread. I think we are fixing something that is not broken. I will try to post a couple of new RFCs and let's discuss this with incremental changes on the mailing list.

That other thread was a separate mailing list. For the benefits of the 
rest of the community it would be nice if you can post the summary here.

However, nobody said the code was broken. IORT will be used in various 
place in Xen and Manish is looking whether we can parse only once and 
for all the IORT. I think it is latest design is promising for that.

Cheers,
diff mbox

Patch

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>
+#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 *))
+
+/* 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);					\
+})
+#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
+ * 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;
 
-		bus = to_pci_bus(dev);
+		pci_dev = to_pci_dev(dev);
 		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
+ */
+#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
+ * 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;
 }
 
-/**
+/*
  * 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
 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
 /**
  * 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);
 		u32 rid;
 
-		pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
-				       &rid);
+		rid = PCI_BDF2(pci_device->bus, pci_device->devfn);
 
 		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
+ * 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
 /**
  * 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)
 {
 	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);*/
 }
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
 #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
@@ -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
 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 */
     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>
 
 #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);
+}
 #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
 };
 
+#ifdef CONFIG_ARM
+#define to_pci_dev(p) container_of(p, struct pci_dev,dev)
+#define pci_domain_nr(dev) dev->seg
+#endif
+
 #define for_each_pdev(domain, pdev) \
     list_for_each_entry(pdev, &(domain->arch.pdev_list), domain_list)