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

Re: [Xen-devel] [RFC v3 4/4] xen/iommu: smmu-v3: Add Xen specific code to enable the ported driver





On 12/05/2017 11:26 PM, Goel, Sameer wrote:


On 12/5/2017 7:17 AM, Julien Grall wrote:
Hello,

On 05/12/17 03:59, Sameer Goel wrote:
This driver follows an approach similar to smmu driver. The intent here
is to reuse as much Linux code as possible.
- Glue code has been introduced in headers to bridge the API calls.
- Called Linux functions from the Xen IOMMU function calls.
- Xen modifications are preceded by /*Xen: comment */
- New config items for SMMUv3 and legacy SMMU have been defined.

There are no reason to touch legacy SMMU in this patch. Please move that 
outside of it.
Ok.


Signed-off-by: Sameer Goel <sgoel@xxxxxxxxxxxxxx>
---
   xen/drivers/Kconfig                    |   2 +
   xen/drivers/passthrough/arm/Kconfig    |  14 +
   xen/drivers/passthrough/arm/Makefile   |   3 +-
   xen/drivers/passthrough/arm/arm_smmu.h | 189 ++++++++++
   xen/drivers/passthrough/arm/smmu-v3.c  | 619 
++++++++++++++++++++++++++++++---
   5 files changed, 768 insertions(+), 59 deletions(-)
   create mode 100644 xen/drivers/passthrough/arm/Kconfig
   create mode 100644 xen/drivers/passthrough/arm/arm_smmu.h

diff --git a/xen/drivers/Kconfig b/xen/drivers/Kconfig
index bc3a54f..6126553 100644
--- a/xen/drivers/Kconfig
+++ b/xen/drivers/Kconfig
@@ -12,4 +12,6 @@ source "drivers/pci/Kconfig"
     source "drivers/video/Kconfig"
   +source "drivers/passthrough/arm/Kconfig"
+
   endmenu
diff --git a/xen/drivers/passthrough/arm/Kconfig 
b/xen/drivers/passthrough/arm/Kconfig
new file mode 100644
index 0000000..9ac4cea
--- /dev/null
+++ b/xen/drivers/passthrough/arm/Kconfig
@@ -0,0 +1,14 @@
+
+config ARM_SMMU
+    bool "ARM SMMU v1/2 support"
+    depends on ARM_64

Why? SMMUv1 and SMMUv2 supports Arm 32-bit.

+    help
+     Support for implementations of the ARM System MMU architecture. (1/2)

I am not sure to understand the (1/2) after the final point.

+
+config ARM_SMMU_v3
+    bool "ARM SMMUv3 Support"
+    depends on ARM_64
+    help
+     Support for implementations of the ARM System MMU architecture
+     version 3.
+
diff --git a/xen/drivers/passthrough/arm/Makefile 
b/xen/drivers/passthrough/arm/Makefile
index f4cd26e..5b3eb15 100644
--- a/xen/drivers/passthrough/arm/Makefile
+++ b/xen/drivers/passthrough/arm/Makefile
@@ -1,2 +1,3 @@
   obj-y += iommu.o
-obj-y += smmu.o
+obj-$(CONFIG_ARM_SMMU) += smmu.o
+obj-$(CONFIG_ARM_SMMU_v3) += smmu-v3.o
diff --git a/xen/drivers/passthrough/arm/arm_smmu.h 
b/xen/drivers/passthrough/arm/arm_smmu.h
new file mode 100644
index 0000000..b5e161f
--- /dev/null
+++ b/xen/drivers/passthrough/arm/arm_smmu.h

I don't think there are any value to use Linux coding style in this header. It 
contains Xen stubs.

I would also have expected this new file to come in a separate patch with the 
modification associated in SMMUv2. This would make easier to see what could be 
common.
That makes sense. I was holding it back till I post the first actual patch and 
just wanted to put out the SMMUv3patches.


@@ -0,0 +1,189 @@
+/******************************************************************************
+ * ./arm_smmu.h
+ *
+ * Common compatibility defines and data_structures for porting arm smmu
+ * drivers from Linux.

[...]

+static struct resource *platform_get_resource(struct platform_device *pdev,
+                          unsigned int type,
+                          unsigned int num)
+{
+    /*
+     * The resource is only used between 2 calls of platform_get_resource.
+     * It's quite ugly but it's avoid to add too much code in the part
+     * imported from Linux
+     */
+    static struct resource res;
+    struct acpi_iort_node *iort_node;
+    struct acpi_iort_smmu_v3 *node_smmu_data;
+    int ret = 0;
+
+    res.type = type;
+
+    switch (type) {
+    case IORESOURCE_MEM:
+        if (pdev->type == DEV_ACPI) {
+            ret = 1;
+            iort_node = pdev->acpi_node;
+            node_smmu_data =
+                (struct acpi_iort_smmu_v3 *)iort_node->node_data;

Above you say: "Common compatibility defines and data_structures for porting arm 
smmu driver from Linux". But this code is clearly SMMUv3.

It is. I will pull this in the SMMUv3 driver.
+
+            if (node_smmu_data != NULL) {
+                res.addr = node_smmu_data->base_address;
+                res.size = SZ_128K;
+                ret = 0;
+            }
+        } else {
+            ret = dt_device_get_address(dev_to_dt(pdev), num,
+                            &res.addr, &res.size);
+        }
+
+        return ((ret) ? NULL : &res);
+
+    case IORESOURCE_IRQ:
+        /* ACPI case not implemented as there is no use case for it */
+        ret = platform_get_irq(dev_to_dt(pdev), num);
+
+        if (ret < 0)
+            return NULL;
+
+        res.addr = ret;
+        res.size = 1;
+
+        return &res;
+
+    default:
+        return NULL;
+    }
+}
+
+static int platform_get_irq_byname(struct platform_device *pdev, const char 
*name)
+{
+    const struct dt_property *dtprop;
+    struct acpi_iort_node *iort_node;
+    struct acpi_iort_smmu_v3 *node_smmu_data;
+    int ret = 0;
+
+    if (pdev->type == DEV_ACPI) {
+        iort_node = pdev->acpi_node;
+        node_smmu_data = (struct acpi_iort_smmu_v3 *)iort_node->node_data;

Ditto.

+
+        if (node_smmu_data != NULL) {
+            if (!strcmp(name, "eventq"))
+                ret = node_smmu_data->event_gsiv;
+            else if (!strcmp(name, "priq"))
+                ret = node_smmu_data->pri_gsiv;
+            else if (!strcmp(name, "cmdq-sync"))
+                ret = node_smmu_data->sync_gsiv;
+            else if (!strcmp(name, "gerror"))
+                ret = node_smmu_data->gerr_gsiv;
+            else
+                ret = -EINVAL;
+        }
+    } else {
+        dtprop = dt_find_property(dev_to_dt(pdev), "interrupt-names", NULL);
+        if (!dtprop)
+            return -EINVAL;
+
+        if (!dtprop->value)
+            return -ENODATA;
+    }
+
+    return ret;
+}
+
+/* Xen: Stub out DMA domain related functions */

I don't think 'Xen:' is necessary as this file contains Xen stubs.
Ok.

+#define iommu_get_dma_cookie(dom) 0
+#define iommu_put_dma_cookie(dom) 0
+
+static void __iomem *devm_ioremap_resource(struct device *dev,
+                       struct resource *res)
+{
+    void __iomem *ptr;
+
+    if (!res || res->type != IORESOURCE_MEM) {
+        dev_err(dev, "Invalid resource\n");
+        return ERR_PTR(-EINVAL);
+    }
+
+    ptr = ioremap_nocache(res->addr, res->size);
+    if (!ptr) {
+        dev_err(dev,
+            "ioremap failed (addr 0x%"PRIx64" size 0x%"PRIx64")\n",
+            res->addr, res->size);
+        return ERR_PTR(-ENOMEM);
+    }
+
+    return ptr;
+}
+
+/* Xen: Dummy iommu_domain */
+struct iommu_domain {
+    /* Runtime SMMU configuration for this iommu_domain */
+    struct arm_smmu_domain        *priv;
+    unsigned int            type;

What are the values for type?

+
+    atomic_t ref;
+    /* Used to link iommu_domain contexts for a same domain.

/*
  * Used ...
  */

+     * There is at least one per-SMMU to used by the domain.
+     */
+    struct list_head        list;
+};
+/* Xen: Domain type definitions. Not really needed for Xen, defining to port

/*
  * Xen: ...

+ * Linux code as-is
+ */
+#define IOMMU_DOMAIN_UNMANAGED 0
+#define IOMMU_DOMAIN_DMA 1
+#define IOMMU_DOMAIN_IDENTITY 2
+
+/* Xen: Describes information required for a Xen domain */
+struct arm_smmu_xen_domain {
+    spinlock_t            lock;
+    /* List of iommu domains associated to this domain */
+    struct list_head        iommu_domains;
+};
+
+/*
+ * Xen: Information about each device stored in dev->archdata.iommu
+ *
+ * The dev->archdata.iommu stores the iommu_domain (runtime configuration of
+ * the SMMU).
+ */
+struct arm_smmu_xen_device {
+    struct iommu_domain *domain;
+};
+
+#endif /* __ARM_SMMU_H__ */

Missing emacs magic.

diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
b/xen/drivers/passthrough/arm/smmu-v3.c
index e67ba6c..c6c1b99 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -18,28 +18,38 @@
    * Author: Will Deacon <will.deacon@xxxxxxx>
    *
    * This driver is powered by bad coffee and bombay mix.
+ *
+ *
+ * Based on Linux drivers/iommu/arm-smmu-v3.c
+ * => commit 7aa8619a66aea52b145e04cbab4f8d6a4e5f3f3b
+ *
+ * Xen modifications:
+ * Sameer Goel <sameer.goel@xxxxxxxxxx>
+ * Copyright (C) 2017, The Linux Foundation, All rights reserved.
+ *
    */
   -#include <linux/acpi.h>
-#include <linux/acpi_iort.h>
-#include <linux/delay.h>
-#include <linux/dma-iommu.h>
-#include <linux/err.h>
-#include <linux/interrupt.h>
-#include <linux/iommu.h>
-#include <linux/iopoll.h>
-#include <linux/module.h>
-#include <linux/msi.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/of_iommu.h>
-#include <linux/of_platform.h>
-#include <linux/pci.h>
-#include <linux/platform_device.h>
-
-#include <linux/amba/bus.h>
-
-#include "io-pgtable.h"
+#include <xen/acpi.h>
+#include <xen/config.h>
+#include <xen/delay.h>
+#include <xen/errno.h>
+#include <xen/err.h>
+#include <xen/irq.h>
+#include <xen/lib.h>
+#include <xen/linux_compat.h>
+#include <xen/list.h>
+#include <xen/mm.h>
+#include <xen/rbtree.h>
+#include <xen/sched.h>
+#include <xen/sizes.h>
+#include <xen/vmap.h>
+#include <acpi/acpi_iort.h>
+#include <asm/atomic.h>
+#include <asm/device.h>
+#include <asm/io.h>
+#include <asm/platform.h>
+
+#include "arm_smmu.h" /* Not a self contained header. So last in the list */
     /* MMIO registers */
   #define ARM_SMMU_IDR0            0x0
@@ -423,9 +433,12 @@
   #endif
     static bool disable_bypass;
+
+#if 0 /* Xen: Not applicable for Xen */
   module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
   MODULE_PARM_DESC(disable_bypass,
       "Disable bypass streams such that incoming transactions from devices that are 
not attached to an iommu domain will report an abort back to the device and will not be allowed to pass through 
the SMMU.");
+#endif

Can't you stub module_param_namde and MODULE_PARM_DESC to avoid #if 0?

     enum pri_resp {
       PRI_RESP_DENY,
@@ -433,6 +446,7 @@ enum pri_resp {
       PRI_RESP_SUCC,
   };
   +#if 0 /* Xen: No MSI support in this iteration */
   enum arm_smmu_msi_index {
       EVTQ_MSI_INDEX,
       GERROR_MSI_INDEX,
@@ -457,6 +471,7 @@ static phys_addr_t arm_smmu_msi_cfg[ARM_SMMU_MAX_MSIS][3] = 
{
           ARM_SMMU_PRIQ_IRQ_CFG2,
       },
   };
+#endif
     struct arm_smmu_cmdq_ent {
       /* Common fields */
@@ -561,6 +576,8 @@ struct arm_smmu_s2_cfg {
       u16                vmid;
       u64                vttbr;
       u64                vtcr;
+    /* Xen: Domain associated to this configuration */
+    struct domain            *domain;
   };
     struct arm_smmu_strtab_ent {
@@ -635,9 +652,21 @@ struct arm_smmu_device {
       struct arm_smmu_strtab_cfg    strtab_cfg;
         /* IOMMU core code handle */
+#if 0 /*Xen: Generic iommu_device ref not needed here */
       struct iommu_device        iommu;
+#endif
+    /* Xen: Need to keep a list of SMMU devices */
+    struct list_head                devices;
   };
   +/* Xen: Keep a list of devices associated with this driver */
+static DEFINE_SPINLOCK(arm_smmu_devices_lock);
+static LIST_HEAD(arm_smmu_devices);
+/* Xen: Helper for finding a device using fwnode */
+static
+struct arm_smmu_device *arm_smmu_get_by_fwnode(struct fwnode_handle *fwnode);
+
+
   /* SMMU private data for each master */
   struct arm_smmu_master_data {
       struct arm_smmu_device        *smmu;
@@ -654,7 +683,7 @@ enum arm_smmu_domain_stage {
     struct arm_smmu_domain {
       struct arm_smmu_device        *smmu;
-    struct mutex            init_mutex; /* Protects smmu pointer */
+    mutex            init_mutex; /* Protects smmu pointer */
         struct io_pgtable_ops        *pgtbl_ops;
   @@ -961,6 +990,7 @@ static void arm_smmu_cmdq_issue_cmd(struct 
arm_smmu_device *smmu,
       spin_unlock_irqrestore(&smmu->cmdq.lock, flags);
   }
   +#if 0 /*Xen: Comment out functions that set up S1 translations */

Why? I do agree that the code will not be used by Xen, but I would prefer if 
you minimize the number of #ifdef.

   /* Context descriptor manipulation functions */
   static u64 arm_smmu_cpu_tcr_to_cd(u64 tcr)
   {
@@ -1003,6 +1033,7 @@ static void arm_smmu_write_ctx_desc(struct 
arm_smmu_device *smmu,
         cfg->cdptr[3] = cpu_to_le64(cfg->cd.mair << CTXDESC_CD_3_MAIR_SHIFT);
   }
+#endif
     /* Stream table manipulation functions */
   static void
@@ -1164,6 +1195,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device 
*smmu, u32 sid)
       void *strtab;
       struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
       struct arm_smmu_strtab_l1_desc *desc = &cfg->l1_desc[sid >> 
STRTAB_SPLIT];
+    u32 alignment = 0;

It is not necassary to initialize alignment. Also we are trying to limit the 
use of u* in favor of uint32_t.

         if (desc->l2ptr)
           return 0;
@@ -1172,14 +1204,16 @@ static int arm_smmu_init_l2_strtab(struct 
arm_smmu_device *smmu, u32 sid)
       strtab = &cfg->strtab[(sid >> STRTAB_SPLIT) * STRTAB_L1_DESC_DWORDS];
         desc->span = STRTAB_SPLIT + 1;
-    desc->l2ptr = dmam_alloc_coherent(smmu->dev, size, &desc->l2ptr_dma,
-                      GFP_KERNEL | __GFP_ZERO);
+    /* Alignment picked from ARM SMMU arch version 3.x. L1ST.L2Ptr */
+    alignment = 1 << ((5 + (desc->span - 1)));
+    desc->l2ptr = _xzalloc(size, alignment);
       if (!desc->l2ptr) {
           dev_err(smmu->dev,
               "failed to allocate l2 stream table for SID %u\n",
               sid);
           return -ENOMEM;
       }
+    desc->l2ptr_dma = virt_to_maddr(desc->l2ptr);
         arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
       arm_smmu_write_strtab_l1_desc(strtab, desc);
@@ -1232,7 +1266,7 @@ static void arm_smmu_handle_ppr(struct arm_smmu_device 
*smmu, u64 *evt)
         dev_info(smmu->dev, "unexpected PRI request received:\n");
       dev_info(smmu->dev,
-         "\tsid 0x%08x.0x%05x: [%u%s] %sprivileged %s%s%s access at iova 
0x%016llx\n",
+         "\tsid 0x%08x.0x%05x: [%u%s] %sprivileged %s%s%s access at iova %#" PRIx64 
"\n",
            sid, ssid, grpid, last ? "L" : "",
            evt[0] & PRIQ_0_PERM_PRIV ? "" : "un",
            evt[0] & PRIQ_0_PERM_READ ? "R" : "",
@@ -1346,6 +1380,8 @@ static irqreturn_t arm_smmu_combined_irq_handler(int irq, 
void *dev)
   {
       arm_smmu_gerror_handler(irq, dev);
       arm_smmu_cmdq_sync_handler(irq, dev);
+    /*Xen: No threaded irq. So call the required function from here */
+    arm_smmu_combined_irq_thread(irq, dev);
       return IRQ_WAKE_THREAD;
   }
   @@ -1358,11 +1394,49 @@ static void __arm_smmu_tlb_sync(struct 
arm_smmu_device *smmu)
       arm_smmu_cmdq_issue_cmd(smmu, &cmd);
   }
   +static void arm_smmu_evtq_thread_xen(int irq, void *dev,
+                       struct cpu_user_regs *regs)
+{
+    arm_smmu_evtq_thread(irq, dev);
+}
+
+static void arm_smmu_priq_thread_xen(int irq, void *dev,
+                       struct cpu_user_regs *regs)
+{
+    arm_smmu_priq_thread(irq, dev);
+}
+
+static void arm_smmu_cmdq_sync_handler_xen(int irq, void *dev,
+                       struct cpu_user_regs *regs)
+{
+    arm_smmu_cmdq_sync_handler(irq, dev);
+}
+
+static void arm_smmu_gerror_handler_xen(int irq, void *dev,
+                       struct cpu_user_regs *regs)
+{
+    arm_smmu_gerror_handler(irq, dev);
+}
+
+static void arm_smmu_combined_irq_handler_xen(int irq, void *dev,
+                       struct cpu_user_regs *regs)
+{
+    arm_smmu_combined_irq_handler(irq, dev);
+}
+

Missing:
/* Xen: .... */

+#define arm_smmu_evtq_thread arm_smmu_evtq_thread_xen
+#define arm_smmu_priq_thread arm_smmu_priq_thread_xen
+#define arm_smmu_cmdq_sync_handler arm_smmu_cmdq_sync_handler_xen
+#define arm_smmu_gerror_handler arm_smmu_gerror_handler_xen
+#define arm_smmu_combined_irq_handler arm_smmu_combined_irq_handler_xen
+
+#if 0 /*Xen: Unused function */
   static void arm_smmu_tlb_sync(void *cookie)
   {
       struct arm_smmu_domain *smmu_domain = cookie;
       __arm_smmu_tlb_sync(smmu_domain->smmu);
   }
+#endif
     static void arm_smmu_tlb_inv_context(void *cookie)
   {
@@ -1383,6 +1457,7 @@ static void arm_smmu_tlb_inv_context(void *cookie)
       __arm_smmu_tlb_sync(smmu);
   }
   +#if 0 /*Xen: Unused functionality */
   static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
                         size_t granule, bool leaf, void *cookie)
   {
@@ -1427,6 +1502,7 @@ static bool arm_smmu_capable(enum iommu_cap cap)
           return false;
       }
   }
+#endif
     static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
   {
@@ -1474,6 +1550,7 @@ static void arm_smmu_bitmap_free(unsigned long *map, int 
idx)
       clear_bit(idx, map);
   }
   +#if 0
   static void arm_smmu_domain_free(struct iommu_domain *domain)
   {
       struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
@@ -1502,7 +1579,23 @@ static void arm_smmu_domain_free(struct iommu_domain 
*domain)
         kfree(smmu_domain);
   }
+#endif
+
+static void arm_smmu_domain_free(struct iommu_domain *domain)
+{
+    struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+    struct arm_smmu_device *smmu = smmu_domain->smmu;
+    struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
+    /*
+     * Xen: Remove the free functions that are not used and code related
+     * to S1 translation. We just need to free the domain and vmid here.
+     */

Can you please give a reason to remove stage-1 code? This is not in the spririt 
of a verbatim port and I still can't see why you can't keep it.

I was just clearing it out as it was not used. I will put it back in.


+    if (cfg->vmid)
+        arm_smmu_bitmap_free(smmu->vmid_map, cfg->vmid);
+    kfree(smmu_domain);
+}
   +#if 0 /*Xen: The finalize domain functions are not needed in current form */
   static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
                          struct io_pgtable_cfg *pgtbl_cfg)
   {
@@ -1551,16 +1644,41 @@ static int arm_smmu_domain_finalise_s2(struct 
arm_smmu_domain *smmu_domain,
       cfg->vtcr    = pgtbl_cfg->arm_lpae_s2_cfg.vtcr;
       return 0;
   }
+#endif
+
+static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain)
+{
+    int vmid;
+    struct arm_smmu_device *smmu = smmu_domain->smmu;
+    struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
+
+    vmid = arm_smmu_bitmap_alloc(smmu->vmid_map, smmu->vmid_bits);
+    if (vmid < 0)
+        return vmid;
+
+    /* Xen: Get the ttbr and vtcr values

/*
  * Xen: ...

But why do you need to duplicate the function when you can just replace the 2 
lines that needs to be modified?

+     * vttbr: This is a shared value with the domain page table
+     * vtcr: The TCR settings are the same as CPU since he page
s/he/the/

+     * tables are shared
+     */
+
+    cfg->vmid    = vmid;
+    cfg->vttbr    = page_to_maddr(cfg->domain->arch.p2m.root);
+    cfg->vtcr    = READ_SYSREG32(VTCR_EL2) & STRTAB_STE_2_VTCR_MASK;

I still think this is really fragile. You at least need a comment on the other 
side (e.g where VTCR_EL2 is written) to explain you are relying the value in 
other places.
I can add the comment.


+    return 0;
+}
     static int arm_smmu_domain_finalise(struct iommu_domain *domain)
   {
       int ret;
+#if 0    /* Xen: pgtbl_cfg not needed. So modify the function as needed */
       unsigned long ias, oas;
       enum io_pgtable_fmt fmt;
       struct io_pgtable_cfg pgtbl_cfg;
       struct io_pgtable_ops *pgtbl_ops;
       int (*finalise_stage_fn)(struct arm_smmu_domain *,
                    struct io_pgtable_cfg *);
+#endif
       struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
       struct arm_smmu_device *smmu = smmu_domain->smmu;
   @@ -1575,6 +1693,7 @@ static int arm_smmu_domain_finalise(struct 
iommu_domain *domain)
       if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S2))
           smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
   +#if 0
       switch (smmu_domain->stage) {
       case ARM_SMMU_DOMAIN_S1:
           ias = VA_BITS;
@@ -1616,7 +1735,9 @@ static int arm_smmu_domain_finalise(struct iommu_domain 
*domain)
       ret = finalise_stage_fn(smmu_domain, &pgtbl_cfg);
       if (ret < 0)
           free_io_pgtable_ops(pgtbl_ops);
+#endif
   +    ret = arm_smmu_domain_finalise_s2(smmu_domain);
       return ret;
   }
   @@ -1709,7 +1830,9 @@ static int arm_smmu_attach_dev(struct iommu_domain 
*domain, struct device *dev)
       } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
           ste->s1_cfg = &smmu_domain->s1_cfg;
           ste->s2_cfg = NULL;
+#if 0 /*Xen: S1 configuratio not needed */

What would be the issue to let this code uncommented

           arm_smmu_write_ctx_desc(smmu, ste->s1_cfg);
+#endif
       } else {
           ste->s1_cfg = NULL;
           ste->s2_cfg = &smmu_domain->s2_cfg;
@@ -1721,6 +1844,7 @@ out_unlock:
       return ret;
   }
    > +#if 0

/* Xen: ... */

   static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
               phys_addr_t paddr, size_t size, int prot)
   {
@@ -1772,6 +1896,7 @@ struct arm_smmu_device *arm_smmu_get_by_fwnode(struct 
fwnode_handle *fwnode)
       put_device(dev);
       return dev ? dev_get_drvdata(dev) : NULL;
   }
+#endif
     static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)
   {
@@ -1782,8 +1907,9 @@ static bool arm_smmu_sid_in_range(struct arm_smmu_device 
*smmu, u32 sid)
         return sid < limit;
   }
-

Please don't remove newline.

+#if 0
   static struct iommu_ops arm_smmu_ops;
+#endif
     static int arm_smmu_add_device(struct device *dev)
   {
@@ -1791,9 +1917,12 @@ static int arm_smmu_add_device(struct device *dev)
       struct arm_smmu_device *smmu;
       struct arm_smmu_master_data *master;
       struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+#if 0 /*Xen: iommu_group is not needed */
       struct iommu_group *group;
+#endif
   -    if (!fwspec || fwspec->ops != &arm_smmu_ops)
+    /* Xen: fwspec->ops are not needed */
+    if (!fwspec)
           return -ENODEV;
       /*
        * We _can_ actually withstand dodgy bus code re-calling add_device()
@@ -1830,6 +1959,12 @@ static int arm_smmu_add_device(struct device *dev)
           }
       }
   +#if 0
+/*
+ * Xen: Do not need an iommu group as the stream data is carried by the SMMU
+ * master device object
+ */

This is better to put before #if 0. So IDE will still show the comment even 
when #if 0 is fold.

+
       group = iommu_group_get_for_dev(dev);
       if (!IS_ERR(group)) {
           iommu_group_put(group);
@@ -1837,8 +1972,16 @@ static int arm_smmu_add_device(struct device *dev)
       }
         return PTR_ERR_OR_ZERO(group);
+#endif
+    return 0;
   }
   +/*
+ * Xen: We can potentially support this function and destroy a device. This
+ * will be relevant for PCI hotplug. So, will be implemented as needed after
+ * passthrough support is available.
+ */
+#if 0
   static void arm_smmu_remove_device(struct device *dev)
   {
       struct iommu_fwspec *fwspec = dev->iommu_fwspec;
@@ -1974,7 +2117,7 @@ static struct iommu_ops arm_smmu_ops = {
       .put_resv_regions    = arm_smmu_put_resv_regions,
       .pgsize_bitmap        = -1UL, /* Restricted during device attach */
   };
-

Ditto for the newline. I know I didn't mention it in every place in the 
previous series. But I would have expected you to apply my comments everywhere.

+#endif
   /* Probing and initialisation functions */
   static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
                      struct arm_smmu_queue *q,
@@ -1984,13 +2127,19 @@ static int arm_smmu_init_one_queue(struct 
arm_smmu_device *smmu,
   {
       size_t qsz = ((1 << q->max_n_shift) * dwords) << 3;
   -    q->base = dmam_alloc_coherent(smmu->dev, qsz, &q->base_dma, GFP_KERNEL);
+    /* The SMMU cache coherency property is always set. Since we are sharing 
the CPU translation tables

/*
  * ...

+     * just make a regular allocation.

I am not sure to understand it. AFAIU, q is for the command queue. So how 
sharing the CPU translation tables will help here?

Furthermore, I don't understand how you can say cache coherency property is 
always set? When I look at the driver, it seems to be able to handle 
non-coherent memory. So where do you modify that?

+     */
+    q->base = _xzalloc(qsz, sizeof(void *));
+
       if (!q->base) {
           dev_err(smmu->dev, "failed to allocate queue (0x%zx bytes)\n",
               qsz);
           return -ENOMEM;
       }
   +    q->base_dma = virt_to_maddr(q->base);
+
       q->prod_reg    = arm_smmu_page1_fixup(prod_off, smmu);
       q->cons_reg    = arm_smmu_page1_fixup(cons_off, smmu);
       q->ent_dwords    = dwords;
@@ -2056,6 +2205,7 @@ static int arm_smmu_init_strtab_2lvl(struct 
arm_smmu_device *smmu)
       u64 reg;
       u32 size, l1size;
       struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
+    u32 alignment;
         /* Calculate the L1 size, capped to the SIDSIZE. */
       size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
@@ -2069,14 +2219,17 @@ static int arm_smmu_init_strtab_2lvl(struct 
arm_smmu_device *smmu)
                size, smmu->sid_bits);
         l1size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
-    strtab = dmam_alloc_coherent(smmu->dev, l1size, &cfg->strtab_dma,
-                     GFP_KERNEL | __GFP_ZERO);
+    alignment = max_t(u32, cfg->num_l1_ents, 64);

Same as before. I know I didn't go through the rest of the code. But you could 
have at least applied my comments on alignment here too. E.g where does the 64 
come from?

But, it looks like to me you want to create a function dmam_alloc_coherent that 
will do the allocation for you. This could be used in a few places within file 
driver...
dmam_alloc_coherent uses the allocation size as the alignment. This is not as 
per spec. But that being said I am fine replicating the code from Linux. That 
will make my life easier :).


+    strtab = _xzalloc(l1size, l1size);
+
       if (!strtab) {
           dev_err(smmu->dev,
               "failed to allocate l1 stream table (%u bytes)\n",
               size);
           return -ENOMEM;
       }
+
+    cfg->strtab_dma = virt_to_maddr(strtab);
       cfg->strtab = strtab;
         /* Configure strtab_base_cfg for 2 levels */
@@ -2098,14 +2251,16 @@ static int arm_smmu_init_strtab_linear(struct 
arm_smmu_device *smmu)
       struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
         size = (1 << smmu->sid_bits) * (STRTAB_STE_DWORDS << 3);
-    strtab = dmam_alloc_coherent(smmu->dev, size, &cfg->strtab_dma,
-                     GFP_KERNEL | __GFP_ZERO);

... such as here.

+    strtab = _xzalloc(size, size);

Hmmm, _xzalloc contains the following assert:

ASSERT((align & (align - 1)) == 0);

How are you sure the size will always honor this check?
I can add another check or add a comment. Till now the size has passed this 
check.

+
       if (!strtab) {
           dev_err(smmu->dev,
               "failed to allocate linear stream table (%u bytes)\n",
               size);
           return -ENOMEM;
       }
+
+    cfg->strtab_dma = virt_to_maddr(strtab);
       cfg->strtab = strtab;
       cfg->num_l1_ents = 1 << smmu->sid_bits;
   @@ -2182,6 +2337,7 @@ static int arm_smmu_update_gbpa(struct arm_smmu_device 
*smmu, u32 set, u32 clr)
                         1, ARM_SMMU_POLL_TIMEOUT_US);
   }
   +#if 0 /* Xen: There is no MSI support as yet */
   static void arm_smmu_free_msis(void *data)
   {
       struct device *dev = data;
@@ -2247,36 +2403,39 @@ static void arm_smmu_setup_msis(struct arm_smmu_device 
*smmu)
       /* Add callback to free MSIs on teardown */
       devm_add_action(dev, arm_smmu_free_msis, dev);
   }
+#endif
     static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu)
   {
       int irq, ret;
   +#if 0 /*Xen: Cannot setup msis for now */
       arm_smmu_setup_msis(smmu);
+#endif
         /* Request interrupt lines */
       irq = smmu->evtq.q.irq;
       if (irq) {
-        ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
-                        arm_smmu_evtq_thread,
-                        IRQF_ONESHOT,
-                        "arm-smmu-v3-evtq", smmu);
+        irq_set_type(irq, IRQ_TYPE_EDGE_BOTH);

Why do you need to set the IRQ type? Can't it be found from the firmware tables?

+        ret = request_irq(irq, arm_smmu_evtq_thread,
+                        0, "arm-smmu-v3-evtq", smmu);

Please create a stub for devm_request_threaded_irq.

           if (ret < 0)
               dev_warn(smmu->dev, "failed to enable evtq irq\n");
       }
         irq = smmu->cmdq.q.irq;
       if (irq) {
-        ret = devm_request_irq(smmu->dev, irq,
-                       arm_smmu_cmdq_sync_handler, 0,
-                       "arm-smmu-v3-cmdq-sync", smmu);
+        irq_set_type(irq, IRQ_TYPE_EDGE_BOTH);
+        ret = request_irq(irq, arm_smmu_cmdq_sync_handler,
+                0, "arm-smmu-v3-cmdq-sync", smmu);

Ditto.

           if (ret < 0)
               dev_warn(smmu->dev, "failed to enable cmdq-sync irq\n");
       }
         irq = smmu->gerr_irq;
       if (irq) {
-        ret = devm_request_irq(smmu->dev, irq, arm_smmu_gerror_handler,
+        irq_set_type(irq, IRQ_TYPE_EDGE_BOTH);
+        ret = request_irq(irq, arm_smmu_gerror_handler,
                          0, "arm-smmu-v3-gerror", smmu);

Ditto.

           if (ret < 0)
               dev_warn(smmu->dev, "failed to enable gerror irq\n");
@@ -2284,12 +2443,13 @@ static void arm_smmu_setup_unique_irqs(struct 
arm_smmu_device *smmu)
         if (smmu->features & ARM_SMMU_FEAT_PRI) {
           irq = smmu->priq.q.irq;
+        irq_set_type(irq, IRQ_TYPE_EDGE_BOTH);
           if (irq) {
-            ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
-                            arm_smmu_priq_thread,
-                            IRQF_ONESHOT,
-                            "arm-smmu-v3-priq",
-                            smmu);
+            ret = request_irq(irq,
+                      arm_smmu_priq_thread,
+                      0,
+                      "arm-smmu-v3-priq",
+                      smmu);

Ditto.

               if (ret < 0)
                   dev_warn(smmu->dev,
                        "failed to enable priq irq\n");
@@ -2316,11 +2476,11 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
*smmu)
            * Cavium ThunderX2 implementation doesn't not support unique
            * irq lines. Use single irq line for all the SMMUv3 interrupts.
            */
-        ret = devm_request_threaded_irq(smmu->dev, irq,
-                    arm_smmu_combined_irq_handler,
-                    arm_smmu_combined_irq_thread,
-                    IRQF_ONESHOT,
-                    "arm-smmu-v3-combined-irq", smmu);
+        ret = request_irq(irq,
+                  arm_smmu_combined_irq_handler,
+                  0,
+                  "arm-smmu-v3-combined-irq",
+                  smmu);

Ditto. And here a good example where I a stub is good. You set the IRQ type 
everywere but not for this one.

           if (ret < 0)
               dev_warn(smmu->dev, "failed to enable combined irq\n");
       } else
@@ -2542,8 +2702,11 @@ static int arm_smmu_device_hw_probe(struct 
arm_smmu_device *smmu)
           smmu->features |= ARM_SMMU_FEAT_STALLS;
       }
   +#if 0/* Xen: Do not enable Stage 1 translations */

This is just saying stage-1 is available. So why do you care so much to disable 
it? This is just adding more #if 0, we managed to get away in SMMUv1 by leaving 
the code as it.

+
       if (reg & IDR0_S1P)
           smmu->features |= ARM_SMMU_FEAT_TRANS_S1;
+#endif
         if (reg & IDR0_S2P)
           smmu->features |= ARM_SMMU_FEAT_TRANS_S2;
@@ -2616,10 +2779,12 @@ static int arm_smmu_device_hw_probe(struct 
arm_smmu_device *smmu)
       if (reg & IDR5_GRAN4K)
           smmu->pgsize_bitmap |= SZ_4K | SZ_2M | SZ_1G;
   +#if 0 /* Xen: SMMU ops do not have a pgsize_bitmap member for Xen */
       if (arm_smmu_ops.pgsize_bitmap == -1UL)
           arm_smmu_ops.pgsize_bitmap = smmu->pgsize_bitmap;
       else
           arm_smmu_ops.pgsize_bitmap |= smmu->pgsize_bitmap;
+#endif
         /* Output address size */
       switch (reg & IDR5_OAS_MASK << IDR5_OAS_SHIFT) {
@@ -2646,10 +2811,12 @@ static int arm_smmu_device_hw_probe(struct 
arm_smmu_device *smmu)
           smmu->oas = 48;
       }
   +#if 0 /* Xen: There is no support for DMA mask */

Stub it?

       /* Set the DMA mask for our table walker */
       if (dma_set_mask_and_coherent(smmu->dev, DMA_BIT_MASK(smmu->oas)))
           dev_warn(smmu->dev,
                "failed to set DMA mask for table walker\n");
+#endif
         smmu->ias = max(smmu->ias, smmu->oas);
   @@ -2680,7 +2847,8 @@ static int arm_smmu_device_acpi_probe(struct 
platform_device *pdev,
       struct device *dev = smmu->dev;
       struct acpi_iort_node *node;
   -    node = *(struct acpi_iort_node **)dev_get_platdata(dev);
+    /* Xen: Modification to get iort_node */
+    node = (struct acpi_iort_node *)dev->acpi_node;
         /* Retrieve SMMUv3 specific data */
       iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
@@ -2703,7 +2871,7 @@ static inline int arm_smmu_device_acpi_probe(struct 
platform_device *pdev,
   static int arm_smmu_device_dt_probe(struct platform_device *pdev,
                       struct arm_smmu_device *smmu)
   {
-    struct device *dev = &pdev->dev;
+    struct device *dev = pdev;
       u32 cells;
       int ret = -EINVAL;
   @@ -2716,8 +2884,8 @@ static int arm_smmu_device_dt_probe(struct 
platform_device *pdev,
         parse_driver_options(smmu);
   -    if (of_dma_is_coherent(dev->of_node))
-        smmu->features |= ARM_SMMU_FEAT_COHERENCY;
+    /* Xen: Set the COHERNECY feature */
+    smmu->features |= ARM_SMMU_FEAT_COHERENCY;

This looks like completely wrong. You should only do it when the firmware 
tables say it is fine.

         return ret;
   }
@@ -2734,9 +2902,11 @@ static int arm_smmu_device_probe(struct platform_device 
*pdev)
   {
       int irq, ret;
       struct resource *res;
+#if 0 /*Xen: Do not need to setup sysfs */
       resource_size_t ioaddr;
+#endif
       struct arm_smmu_device *smmu;
-    struct device *dev = &pdev->dev;
+    struct device *dev = pdev;/* Xen: dev is ignored */
       bool bypass;
         smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
@@ -2763,8 +2933,9 @@ static int arm_smmu_device_probe(struct platform_device 
*pdev)
           dev_err(dev, "MMIO region too small (%pr)\n", res);
           return -EINVAL;
       }
+#if 0 /*Xen: Do not need to setup sysfs */
       ioaddr = res->start;
-

Again the newline.

+#endif
       smmu->base = devm_ioremap_resource(dev, res);
       if (IS_ERR(smmu->base))
           return PTR_ERR(smmu->base);
@@ -2802,13 +2973,16 @@ static int arm_smmu_device_probe(struct platform_device 
*pdev)
           return ret;
         /* Record our private device structure */
+#if 0 /* Xen: SMMU is not treated a a platform device*/
       platform_set_drvdata(pdev, smmu);
-

Again the newline.

+#endif
       /* Reset the device */
       ret = arm_smmu_device_reset(smmu, bypass);
       if (ret)
           return ret;
   +/* Xen: Not creating an IOMMU device list for Xen */
+#if 0
       /* And we're up. Go go go! */
       ret = iommu_device_sysfs_add(&smmu->iommu, dev, NULL,
                        "smmu3.%pa", &ioaddr);
@@ -2844,9 +3018,18 @@ static int arm_smmu_device_probe(struct platform_device 
*pdev)
           if (ret)
               return ret;
       }
+#endif
+    /*
+     * Xen: Keep a list of all probed devices. This will be used to query
+     * the smmu devices based on the fwnode.
+     */
+    INIT_LIST_HEAD(&smmu->devices);
+    spin_lock(&arm_smmu_devices_lock);
+    list_add(&smmu->devices, &arm_smmu_devices);
+    spin_unlock(&arm_smmu_devices_lock);
       return 0;
   }
-

Again the newline removed and /* Xen ... */
+#if 0
   static int arm_smmu_device_remove(struct platform_device *pdev)
   {
       struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
@@ -2860,6 +3043,10 @@ static void arm_smmu_device_shutdown(struct 
platform_device *pdev)
   {
       arm_smmu_device_remove(pdev);
   }
+#endif
+
+#define MODULE_DEVICE_TABLE(type, name)
+#define of_device_id dt_device_match

That should be define on top.

     static const struct of_device_id arm_smmu_of_match[] = {
       { .compatible = "arm,smmu-v3", },
@@ -2867,6 +3054,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
   };
   MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
   +#if 0
   static struct platform_driver arm_smmu_driver = {
       .driver    = {
           .name        = "arm-smmu-v3",
@@ -2883,3 +3071,318 @@ IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", NULL);
   MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
   MODULE_AUTHOR("Will Deacon <will.deacon@xxxxxxx>");
   MODULE_LICENSE("GPL v2");
+#endif
+
+/***** Start of Xen specific code *****/
+
+static int __must_check arm_smmu_iotlb_flush_all(struct domain *d)
+{
+    struct arm_smmu_xen_domain *smmu_domain = dom_iommu(d)->arch.priv;
+    struct iommu_domain *cfg;
+
+    spin_lock(&smmu_domain->lock);
+    list_for_each_entry(cfg, &smmu_domain->iommu_domains, list) {
+        /*
+         * Only invalidate the context when SMMU is present.
+         * This is because the context initialization is delayed
+         * until a master has been added.
+         */
+        if (unlikely(!ACCESS_ONCE(cfg->priv->smmu)))
+            continue;
+        arm_smmu_tlb_inv_context(cfg->priv);
+    }
+    spin_unlock(&smmu_domain->lock);
+    return 0;
+}
+
+static int __must_check arm_smmu_iotlb_flush(struct domain *d,
+                         unsigned long gfn,
+                         unsigned int page_count)
+{
+    return arm_smmu_iotlb_flush_all(d);
+}
+
+static struct iommu_domain *arm_smmu_get_domain(struct domain *d,
+                        struct device *dev)
+{
+    struct iommu_domain *domain;
+    struct arm_smmu_xen_domain *xen_domain;
+    struct arm_smmu_device *smmu;
+    struct arm_smmu_domain *smmu_domain;
+
+    xen_domain = dom_iommu(d)->arch.priv;
+
+    smmu = arm_smmu_get_by_fwnode(dev->iommu_fwspec->iommu_fwnode);
+    if (!smmu)
+        return NULL;
+
+    /*
+     * Loop through the &xen_domain->contexts to locate a context
+     * assigned to this SMMU
+     */
+    list_for_each_entry(domain, &xen_domain->iommu_domains, list) {
+        smmu_domain = to_smmu_domain(domain);
+        if (smmu_domain->smmu == smmu)
+            return domain;
+    }
+
+    return NULL;
+}
+
+static void arm_smmu_destroy_iommu_domain(struct iommu_domain *domain)
+{
+    list_del(&domain->list);
+    arm_smmu_domain_free(domain);
+}
+
+static int arm_smmu_assign_dev(struct domain *d, u8 devfn,
+                   struct device *dev, u32 flag)
+{
+    int ret = 0;
+    struct iommu_domain *domain;
+    struct arm_smmu_xen_domain *xen_domain;
+    struct arm_smmu_domain *arm_smmu;
+
+    xen_domain = dom_iommu(d)->arch.priv;
+
+    if (!dev->archdata.iommu) {
+        dev->archdata.iommu = xzalloc(struct arm_smmu_xen_device);
+        if (!dev->archdata.iommu)
+            return -ENOMEM;
+    }
+
+    ret = arm_smmu_add_device(dev);
+    if (ret)
+        return ret;
+
+    spin_lock(&xen_domain->lock);
+
+    /*
+     * Check to see if an iommu_domain already exists for this xen domain
+     * under the same SMMU
+     */
+    domain = arm_smmu_get_domain(d, dev);
+    if (!domain) {
+
+        domain = arm_smmu_domain_alloc(IOMMU_DOMAIN_DMA);
+        if (!domain) {
+            ret = -ENOMEM;
+            goto out;
+        }
+
+        arm_smmu = to_smmu_domain(domain);
+        arm_smmu->s2_cfg.domain = d;
+
+        /* Chain the new context to the domain */
+        list_add(&domain->list, &xen_domain->iommu_domains);
+
+    }
+
+    ret = arm_smmu_attach_dev(domain, dev);
+    if (ret) {
+        if (domain->ref.counter == 0)
+            arm_smmu_destroy_iommu_domain(domain);
+    } else {
+        atomic_inc(&domain->ref);
+    }
+
+out:
+    spin_unlock(&xen_domain->lock);
+    return ret;
+}
+
+static int arm_smmu_deassign_dev(struct domain *d, struct device *dev)
+{
+    struct iommu_domain *domain = arm_smmu_get_domain(d, dev);
+    struct arm_smmu_xen_domain *xen_domain;
+    struct arm_smmu_domain *arm_smmu = to_smmu_domain(domain);
+
+    xen_domain = dom_iommu(d)->arch.priv;
+
+    if (!arm_smmu || arm_smmu->s2_cfg.domain != d) {
+        dev_err(dev, " not attached to domain %d\n", d->domain_id);
+        return -ESRCH;
+    }
+
+    spin_lock(&xen_domain->lock);
+
+    arm_smmu_detach_dev(dev);
+    atomic_dec(&domain->ref);
+
+    if (domain->ref.counter == 0)
+        arm_smmu_destroy_iommu_domain(domain);
+
+    spin_unlock(&xen_domain->lock);
+
+
+
+    return 0;
+}
+
+static int arm_smmu_reassign_dev(struct domain *s, struct domain *t,
+                 u8 devfn,  struct device *dev)
+{
+    int ret = 0;
+
+    /* Don't allow remapping on other domain than hwdom */
+    if (t && t != hardware_domain)
+        return -EPERM;
+
+    if (t == s)
+        return 0;
+
+    ret = arm_smmu_deassign_dev(s, dev);
+    if (ret)
+        return ret;
+
+    if (t) {
+        /* No flags are defined for ARM. */
+        ret = arm_smmu_assign_dev(t, devfn, dev, 0);
+        if (ret)
+            return ret;
+    }
+
+    return 0;
+}
+
+static int arm_smmu_iommu_domain_init(struct domain *d)
+{
+    struct arm_smmu_xen_domain *xen_domain;
+
+    xen_domain = xzalloc(struct arm_smmu_xen_domain);
+    if (!xen_domain)
+        return -ENOMEM;
+
+    spin_lock_init(&xen_domain->lock);
+    INIT_LIST_HEAD(&xen_domain->iommu_domains);
+
+    dom_iommu(d)->arch.priv = xen_domain;
+
+    return 0;
+}
+
+static void __hwdom_init arm_smmu_iommu_hwdom_init(struct domain *d)
+{
+}
+
+static void arm_smmu_iommu_domain_teardown(struct domain *d)
+{
+    struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
+
+    ASSERT(list_empty(&xen_domain->iommu_domains));
+    xfree(xen_domain);
+}
+
+static int __must_check arm_smmu_map_page(struct domain *d, unsigned long gfn,
+            unsigned long mfn, unsigned int flags)
+{
+    p2m_type_t t;
+
+    /*
+     * Grant mappings can be used for DMA requests. The dev_bus_addr
+     * returned by the hypercall is the MFN (not the IPA). For device
+     * protected by an IOMMU, Xen needs to add a 1:1 mapping in the domain
+     * p2m to allow DMA request to work.
+     * This is only valid when the domain is directed mapped. Hence this
+     * function should only be used by gnttab code with gfn == mfn.
+     */
+    BUG_ON(!is_domain_direct_mapped(d));
+    BUG_ON(mfn != gfn);
+
+    /* We only support readable and writable flags */
+    if (!(flags & (IOMMUF_readable | IOMMUF_writable)))
+        return -EINVAL;
+
+    t = (flags & IOMMUF_writable) ? p2m_iommu_map_rw : p2m_iommu_map_ro;
+
+    /*
+     * The function guest_physmap_add_entry replaces the current mapping
+     * if there is already one...
+     */
+    return guest_physmap_add_entry(d, _gfn(gfn), _mfn(mfn), 0, t);
+}
+
+static int __must_check arm_smmu_unmap_page(struct domain *d, unsigned long 
gfn)
+{
+    /*
+     * This function should only be used by gnttab code when the domain
+     * is direct mapped
+     */
+    if (!is_domain_direct_mapped(d))
+        return -EINVAL;
+
+    return guest_physmap_remove_page(d, _gfn(gfn), _mfn(gfn), 0);
+}
+
+static const struct iommu_ops arm_smmu_iommu_ops = {
+    .init = arm_smmu_iommu_domain_init,
+    .hwdom_init = arm_smmu_iommu_hwdom_init,
+    .teardown = arm_smmu_iommu_domain_teardown,
+    .iotlb_flush = arm_smmu_iotlb_flush,
+    .iotlb_flush_all = arm_smmu_iotlb_flush_all,
+    .assign_device = arm_smmu_assign_dev,
+    .reassign_device = arm_smmu_reassign_dev,
+    .map_page = arm_smmu_map_page,
+    .unmap_page = arm_smmu_unmap_page,
+};
+
+static
+struct arm_smmu_device *arm_smmu_get_by_fwnode(struct fwnode_handle *fwnode)
+{
+    struct arm_smmu_device *smmu = NULL;
+
+    spin_lock(&arm_smmu_devices_lock);
+    list_for_each_entry(smmu, &arm_smmu_devices, devices) {
+        if (smmu->dev->fwnode == fwnode)
+            break;
+    }
+    spin_unlock(&arm_smmu_devices_lock);
+
+    return smmu;
+}
+
+static __init int arm_smmu_dt_init(struct dt_device_node *dev,
+                   const void *data)
+{
+    int rc;
+
+    /*
+     * Even if the device can't be initialized, we don't want to
+     * give the SMMU device to dom0.
+     */
+    dt_device_set_used_by(dev, DOMID_XEN);
+
+    rc = arm_smmu_device_probe(dt_to_dev(dev));
+    if (rc)
+        return rc;
+
+    iommu_set_ops(&arm_smmu_iommu_ops);
+
+    return 0;
+}
+
+DT_DEVICE_START(smmuv3, "ARM SMMU V3", DEVICE_IOMMU)
+    .dt_match = arm_smmu_of_match,
+    .init = arm_smmu_dt_init,
+DT_DEVICE_END
+
+#ifdef CONFIG_ACPI
+/* Set up the IOMMU */
+static int __init arm_smmu_acpi_init(const void *data)
+{
+    int rc;
+    rc = arm_smmu_device_probe((struct device *)data);
+
+    if (rc)
+        return rc;
+
+    iommu_set_ops(&arm_smmu_iommu_ops);
+    return 0;
+}
+
+ACPI_DEVICE_START(asmmuv3, "ARM SMMU V3", DEVICE_IOMMU)
+    .class_type = ACPI_IORT_NODE_SMMU_V3,
+    .init = arm_smmu_acpi_init,
+ACPI_DEVICE_END
+
+#endif

Cheers,


I'll fix the newlines as needed. I thought I had got them all but it seems a 
few were still missed.
Thanks,
Sameer


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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