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

Re: [Xen-devel] [RFC v4 8/8] drivers/passthrough/arm: Refactor code for arm smmu drivers



Hi Sameer,


On 12/19/2017 08:47 AM, Sameer Goel wrote:
Pull common defines for SMMU drivers in a local header.

Signed-off-by: Sameer Goel <sameer.goel@xxxxxxxxxx>
---
  xen/drivers/passthrough/arm/arm_smmu.h | 113 +++++++++++++++++++++++++++++++++
  xen/drivers/passthrough/arm/smmu-v3.c  |  96 ++--------------------------
  xen/drivers/passthrough/arm/smmu.c     | 104 +-----------------------------
  3 files changed, 121 insertions(+), 192 deletions(-)
  create mode 100644 xen/drivers/passthrough/arm/arm_smmu.h

diff --git a/xen/drivers/passthrough/arm/arm_smmu.h 
b/xen/drivers/passthrough/arm/arm_smmu.h
new file mode 100644
index 0000000000..70f97e7d50
--- /dev/null
+++ b/xen/drivers/passthrough/arm/arm_smmu.h
@@ -0,0 +1,113 @@
+/******************************************************************************
+ * arm_smmu.h
+ *
+ * Common compatibility defines and data_structures for porting arm smmu
+ * drivers from Linux.
+ *
+ * Copyright (c) 2017 Linaro Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __ARM_SMMU_H__
+#define __ARM_SMMU_H__
+
+/* Helpers to get device MMIO and IRQs */
+struct resource {
+    u64 addr;
+    u64 size;
+    unsigned int type;
+};
+
+#define resource_size(res) ((res)->size)
+
+#define platform_device device
+
+#define IORESOURCE_MEM 0
+#define IORESOURCE_IRQ 1
+
+/* Stub out DMA domain related functions */
+#define iommu_get_dma_cookie(dom) 0
+#define iommu_put_dma_cookie(dom)
+
+#define VA_BITS     0   /* Only used for configuring stage-1 input size */
+
+#define MODULE_DEVICE_TABLE(type, name)
+#define module_param_named(name, value, type, perm)
+#define MODULE_PARM_DESC(_parm, desc)
+
+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;
+}
+
+/*
+ * Domain type definitions. Not really needed for Xen, defining to port
+ * Linux code as-is
+ */
+#define IOMMU_DOMAIN_UNMANAGED 0
+#define IOMMU_DOMAIN_DMA 1
+#define IOMMU_DOMAIN_IDENTITY 2
+
+/* Xen: Compatibility define for iommu_domain_geometry.*/
+struct iommu_domain_geometry {
+    dma_addr_t aperture_start; /* First address that can be mapped    */
+    dma_addr_t aperture_end;   /* Last address that can be mapped     */
+    bool force_aperture;       /* DMA only allowed in mappable range? */
+};
+
+/* Xen: Dummy iommu_domain */
+struct iommu_domain {
+    /* Runtime SMMU configuration for this iommu_domain */
+    struct arm_smmu_domain      *priv;
Do we need to call it priv? IMHO as there are too many structures wtih _domain and in the bigger goal of making the code intuitive
can we remove priv to something more verbose as smmu_domain.

+    unsigned int            type;
+
+    /* Dummy compatibility defines */
+    unsigned long pgsize_bitmap;
+    struct iommu_domain_geometry geometry;
+
+    atomic_t ref;
+    /* Used to link iommu_domain contexts for a same domain.
+     * There is at least one per-SMMU to used by the domain.
+     */
+    struct list_head        list;
+};
+
+/* 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        contexts;
Could we use a more verbose name, How about %s/contexts/iommu_domain_contexts/g ?
+};
+
+#endif /* __ARM_SMMU_H__ */
+
diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
b/xen/drivers/passthrough/arm/smmu-v3.c
index 3488184ad4..6e705f63a3 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -49,20 +49,7 @@
  #include <asm/io.h>
  #include <asm/platform.h>
-
-/* Xen: Helpers to get device MMIO and IRQs */
-struct resource {
-       u64 addr;
-       u64 size;
-       unsigned int type;
-};
-
-#define resource_size(res) ((res)->size)
-
-#define platform_device device
-
-#define IORESOURCE_MEM 0
-#define IORESOURCE_IRQ 1
+#include "arm_smmu.h" /* Not a self contained header. So last in the list */
static struct resource *platform_get_resource(struct platform_device *pdev,
                                              unsigned int type,
@@ -192,81 +179,10 @@ void dmam_free_coherent(struct device *dev, size_t size, 
void *vaddr,
        xfree(vaddr);
  }
-/* Xen: Stub out DMA domain related functions */
-#define iommu_get_dma_cookie(dom) 0
-#define iommu_put_dma_cookie(dom)
-
-/* Xen: Stub out module param related function */
-#define module_param_named(a, b, c, d)
-#define MODULE_PARM_DESC(a, b)
-
  #define dma_set_mask_and_coherent(d, b) 0
#define of_dma_is_coherent(n) 0 -#define MODULE_DEVICE_TABLE(type, name)
-#define of_device_id dt_device_match
-
-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: Compatibility define for iommu_domain_geometry.*/
-struct iommu_domain_geometry {
-       dma_addr_t aperture_start; /* First address that can be mapped    */
-       dma_addr_t aperture_end;   /* Last address that can be mapped     */
-       bool force_aperture;       /* DMA only allowed in mappable range? */
-};
-
-
-/* Xen: Type definitions for iommu_domain */
-#define IOMMU_DOMAIN_UNMANAGED 0
-#define IOMMU_DOMAIN_DMA 1
-#define IOMMU_DOMAIN_IDENTITY 2
-
-/* Xen: Dummy iommu_domain */
-struct iommu_domain {
-       /* Runtime SMMU configuration for this iommu_domain */
-       struct arm_smmu_domain          *priv;
-       unsigned int type;
-
-       /* Dummy compatibility defines */
-       unsigned long pgsize_bitmap;
-       struct iommu_domain_geometry geometry;
-
-       atomic_t ref;
-       /*
-        * Used to link iommu_domain contexts for a same domain.
-        * There is at least one per-SMMU to used by the domain.
-        */
-       struct list_head                list;
-};
-
-
-/* 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
   *
@@ -3396,7 +3312,7 @@ static int __must_check arm_smmu_iotlb_flush_all(struct 
domain *d)
        struct iommu_domain *cfg;
spin_lock(&smmu_domain->lock);
-       list_for_each_entry(cfg, &smmu_domain->iommu_domains, list) {
+       list_for_each_entry(cfg, &smmu_domain->contexts, list) {
                /*
                 * Only invalidate the context when SMMU is present.
                 * This is because the context initialization is delayed
@@ -3435,7 +3351,7 @@ static struct iommu_domain *arm_smmu_get_domain(struct 
domain *d,
         * Loop through the &xen_domain->contexts to locate a context
         * assigned to this SMMU
         */
-       list_for_each_entry(domain, &xen_domain->iommu_domains, list) {
+       list_for_each_entry(domain, &xen_domain->contexts, list) {
                smmu_domain = to_smmu_domain(domain);
                if (smmu_domain->smmu == smmu)
                        return domain;
@@ -3489,7 +3405,7 @@ static int arm_smmu_assign_dev(struct domain *d, u8 devfn,
                arm_smmu->s2_cfg.domain = d;
/* Chain the new context to the domain */
-               list_add(&domain->list, &xen_domain->iommu_domains);
+               list_add(&domain->list, &xen_domain->contexts);
} @@ -3569,7 +3485,7 @@ static int arm_smmu_iommu_domain_init(struct domain *d)
                return -ENOMEM;
spin_lock_init(&xen_domain->lock);
-       INIT_LIST_HEAD(&xen_domain->iommu_domains);
+       INIT_LIST_HEAD(&xen_domain->contexts);
dom_iommu(d)->arch.priv = xen_domain; @@ -3584,7 +3500,7 @@ 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));
+       ASSERT(list_empty(&xen_domain->contexts));
        xfree(xen_domain);
  }
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index ad956d5b8d..4c04391e21 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -41,6 +41,7 @@
  #include <xen/irq.h>
  #include <xen/lib.h>
  #include <xen/list.h>
+#include <xen/linux_compat.h>
  #include <xen/mm.h>
  #include <xen/vmap.h>
  #include <xen/rbtree.h>
@@ -51,36 +52,13 @@
  #include <asm/io.h>
  #include <asm/platform.h>
+#include "arm_smmu.h" /* Not a self contained header. So last in the list */
  /* Xen: The below defines are redefined within the file. Undef it */
  #undef SCTLR_AFE
  #undef SCTLR_TRE
  #undef SCTLR_M
  #undef TTBCR_EAE
-/* Alias to Xen device tree helpers */
-#define device_node dt_device_node
-#define of_phandle_args dt_phandle_args
-#define of_device_id dt_device_match
-#define of_match_node dt_match_node
-#define of_property_read_u32(np, pname, out) (!dt_property_read_u32(np, pname, 
out))
-#define of_property_read_bool dt_property_read_bool
-#define of_parse_phandle_with_args dt_parse_phandle_with_args
-
-/* Xen: Helpers to get device MMIO and IRQs */
-struct resource
-{
-       u64 addr;
-       u64 size;
-       unsigned int type;
-};
-
-#define resource_size(res) (res)->size;
-
-#define platform_device device
-
-#define IORESOURCE_MEM 0
-#define IORESOURCE_IRQ 1
-
  static struct resource *platform_get_resource(struct platform_device *pdev,
                                              unsigned int type,
                                              unsigned int num)
@@ -118,58 +96,6 @@ static struct resource *platform_get_resource(struct 
platform_device *pdev,
/* Xen: Helpers for IRQ functions */
  #define request_irq(irq, func, flags, name, dev) request_irq(irq, flags, 
func, name, dev)
-#define free_irq release_irq
-
-enum irqreturn {
-       IRQ_NONE        = (0 << 0),
-       IRQ_HANDLED     = (1 << 0),
-};
-
-typedef enum irqreturn irqreturn_t;
-
-/* Device logger functions
- * TODO: Handle PCI
- */
-#define dev_print(dev, lvl, fmt, ...)                                          
\
-        printk(lvl "smmu: %s: " fmt, dt_node_full_name(dev_to_dt(dev)), ## 
__VA_ARGS__)
-
-#define dev_dbg(dev, fmt, ...) dev_print(dev, XENLOG_DEBUG, fmt, ## 
__VA_ARGS__)
-#define dev_notice(dev, fmt, ...) dev_print(dev, XENLOG_INFO, fmt, ## 
__VA_ARGS__)
-#define dev_warn(dev, fmt, ...) dev_print(dev, XENLOG_WARNING, fmt, ## 
__VA_ARGS__)
-#define dev_err(dev, fmt, ...) dev_print(dev, XENLOG_ERR, fmt, ## __VA_ARGS__)
-
-#define dev_err_ratelimited(dev, fmt, ...)                                     
\
-        dev_print(dev, XENLOG_ERR, fmt, ## __VA_ARGS__)
-
-#define dev_name(dev) dt_node_full_name(dev_to_dt(dev))
-
-/* Alias to Xen allocation helpers */
-#define kfree xfree
-#define kmalloc(size, flags)           _xmalloc(size, sizeof(void *))
-#define kzalloc(size, flags)           _xzalloc(size, sizeof(void *))
-#define devm_kzalloc(dev, size, flags) _xzalloc(size, sizeof(void *))
-#define kmalloc_array(size, n, flags)  _xmalloc_array(size, sizeof(void *), n)
-
-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 doesn't handle IOMMU fault */
  #define report_iommu_fault(...)       1
@@ -196,32 +122,6 @@ static inline int pci_for_each_dma_alias(struct pci_dev 
*pdev,
  #define PHYS_MASK_SHIFT               PADDR_BITS
  typedef paddr_t phys_addr_t;
-#define VA_BITS 0 /* Only used for configuring stage-1 input size */
-
-#define MODULE_DEVICE_TABLE(type, name)
-#define module_param_named(name, value, type, perm)
-#define MODULE_PARM_DESC(_parm, desc)
-
-/* Xen: Dummy iommu_domain */
-struct iommu_domain
-{
-       /* Runtime SMMU configuration for this iommu_domain */
-       struct arm_smmu_domain          *priv;
-
-       atomic_t ref;
-       /* Used to link iommu_domain contexts for a same domain.
-        * There is at least one per-SMMU to used by the domain.
-        * */
-       struct list_head                list;
-};
-
-/* Xen: Describes informations required for a Xen domain */
-struct arm_smmu_xen_domain {
-       spinlock_t                      lock;
-       /* List of context (i.e iommu_domain) associated to this domain */
-       struct list_head                contexts;
-};
-
  /*
   * Xen: Information about each device stored in dev->archdata.iommu
   *


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