diff mbox

[Xen-devel,v3,09/13] xen/passthrough: Introduce IOMMU ARM architecture

Message ID 1394552999-14171-10-git-send-email-julien.grall@linaro.org
State Superseded, archived
Headers show

Commit Message

Julien Grall March 11, 2014, 3:49 p.m. UTC
This patch contains the architecture to use IOMMUs on ARM. There is no
IOMMU drivers on this patch.

In this implementation, IOMMU page table will be shared with the P2M.

The code will run through the device tree and will initialize every IOMMU.
It's possible to have multiple IOMMUs on the same platform, but they must
be handled with the same driver. For now, there is no support for using
multiple iommu drivers at runtime.

Each new IOMMU drivers should contain:

static const char * const myiommu_dt_compat[] __initconst =
{
    /* list of device compatible with the drivers. Will be matched with
     * the "compatible" property on the device tree
     */
    NULL,
};

DT_DEVICE_START(myiommu, "MY IOMMU", DEVICE_IOMMU)
        .compatible = myiommu_compatible,
        .init = myiommu_init,
DT_DEVICE_END

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Xiantao Zhang <xiantao.zhang@intel.com>
Cc: Jan Beulich <jbeulich@suse.com>

---
    Changes in v3:
        - Call iommu_dt_domain_{init,destroy} function in arch code
    Changes in v2:
        - Fix typoes in commit message
        - Remove useless comment in arch/arm/setup.c
        - Update copyright date to 2014
        - Move iommu_dom0_init earlier
        - Call iommu_assign_dt_device in map_device when the device is
        protected by an IOMMU
---
 xen/arch/arm/Rules.mk                |    1 +
 xen/arch/arm/domain.c                |    7 ++++
 xen/arch/arm/domain_build.c          |   19 +++++++--
 xen/arch/arm/p2m.c                   |    4 ++
 xen/arch/arm/setup.c                 |    2 +
 xen/drivers/passthrough/Makefile     |    1 +
 xen/drivers/passthrough/arm/Makefile |    1 +
 xen/drivers/passthrough/arm/iommu.c  |   70 ++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/device.h         |    3 +-
 xen/include/asm-arm/domain.h         |    2 +
 xen/include/asm-arm/hvm/iommu.h      |   10 +++++
 xen/include/asm-arm/iommu.h          |   36 +++++++++++++++++
 12 files changed, 152 insertions(+), 4 deletions(-)
 create mode 100644 xen/drivers/passthrough/arm/Makefile
 create mode 100644 xen/drivers/passthrough/arm/iommu.c
 create mode 100644 xen/include/asm-arm/hvm/iommu.h
 create mode 100644 xen/include/asm-arm/iommu.h

Comments

Ian Campbell March 18, 2014, 4:40 p.m. UTC | #1
On Tue, 2014-03-11 at 15:49 +0000, Julien Grall wrote:
> @@ -754,7 +766,7 @@ static int map_device(struct domain *d, const struct dt_device_node *dev)
>  }
>  
>  static int handle_node(struct domain *d, struct kernel_info *kinfo,
> -                       const struct dt_device_node *node)
> +                       struct dt_device_node *node)
>  {
>      static const struct dt_device_match skip_matches[] __initconst =
>      {
> @@ -775,7 +787,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>          DT_MATCH_TIMER,
>          { /* sentinel */ },
>      };
> -    const struct dt_device_node *child;
> +    struct dt_device_node *child;

Why do these consts become unwanted?

> diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
> new file mode 100644
> index 0000000..b0bd71d
> --- /dev/null
> +++ b/xen/drivers/passthrough/arm/iommu.c
[...]
> +int __init iommu_hardware_setup(void)
> +{
> +    struct dt_device_node *np;
> +    int rc;
> +    unsigned int num_iommus = 0;
> +
> +    dt_for_each_device_node(dt_host, np)

I can't find dt_host in this or any of the previous patches.

> +    {
> +        rc = device_init(np, DEVICE_IOMMU, NULL);
> +        if ( !rc )
> +            num_iommus++;
> +    }
> +
> +    return ( num_iommus > 0 ) ? 0 : -ENODEV;
> +}
> +
> +int arch_iommu_domain_init(struct domain *d)
> +{
> +    int ret;
> +
> +    ret = iommu_dt_domain_init(d);
> +
> +    return ret;

return iommu_dt-domain_init(d);
?

> diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
> new file mode 100644
> index 0000000..81eec83
> --- /dev/null
> +++ b/xen/include/asm-arm/iommu.h
> [...]
> +#define domain_hvm_iommu(d) (&d->arch.hvm_domain.hvm_iommu)

Does this macro give us the freedom to avoid the term "hvm" a bit and
use d->arch.iommu?

Ian.
Julien Grall March 18, 2014, 7:58 p.m. UTC | #2
Hi Ian,

On 03/18/2014 04:40 PM, Ian Campbell wrote:
> On Tue, 2014-03-11 at 15:49 +0000, Julien Grall wrote:
>> @@ -754,7 +766,7 @@ static int map_device(struct domain *d, const struct dt_device_node *dev)
>>  }
>>  
>>  static int handle_node(struct domain *d, struct kernel_info *kinfo,
>> -                       const struct dt_device_node *node)
>> +                       struct dt_device_node *node)
>>  {
>>      static const struct dt_device_match skip_matches[] __initconst =
>>      {
>> @@ -775,7 +787,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>>          DT_MATCH_TIMER,
>>          { /* sentinel */ },
>>      };
>> -    const struct dt_device_node *child;
>> +    struct dt_device_node *child;
> 
> Why do these consts become unwanted?

Because map_device now calls iommu_assign_dt_device which will update
next_assigned in the structure dt_device_node.

>> diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
>> new file mode 100644
>> index 0000000..b0bd71d
>> --- /dev/null
>> +++ b/xen/drivers/passthrough/arm/iommu.c
> [...]
>> +int __init iommu_hardware_setup(void)
>> +{
>> +    struct dt_device_node *np;
>> +    int rc;
>> +    unsigned int num_iommus = 0;
>> +
>> +    dt_for_each_device_node(dt_host, np)
> 
> I can't find dt_host in this or any of the previous patches.

dt_host was defined a while ago by the device tree code (see
xen/include/xen/device_tree.h).

>> +    {
>> +        rc = device_init(np, DEVICE_IOMMU, NULL);
>> +        if ( !rc )
>> +            num_iommus++;
>> +    }
>> +
>> +    return ( num_iommus > 0 ) ? 0 : -ENODEV;
>> +}
>> +
>> +int arch_iommu_domain_init(struct domain *d)
>> +{
>> +    int ret;
>> +
>> +    ret = iommu_dt_domain_init(d);
>> +
>> +    return ret;
> 
> return iommu_dt-domain_init(d);
> ?

I will do the change in the next version.

>> diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
>> new file mode 100644
>> index 0000000..81eec83
>> --- /dev/null
>> +++ b/xen/include/asm-arm/iommu.h
>> [...]
>> +#define domain_hvm_iommu(d) (&d->arch.hvm_domain.hvm_iommu)
> 
> Does this macro give us the freedom to avoid the term "hvm" a bit and
> use d->arch.iommu?

It's possible, I just blindly copied from x86.

Regards,
Ian Campbell March 19, 2014, 10:29 a.m. UTC | #3
On Tue, 2014-03-18 at 19:58 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 03/18/2014 04:40 PM, Ian Campbell wrote:
> > On Tue, 2014-03-11 at 15:49 +0000, Julien Grall wrote:
> >> @@ -754,7 +766,7 @@ static int map_device(struct domain *d, const struct dt_device_node *dev)
> >>  }
> >>  
> >>  static int handle_node(struct domain *d, struct kernel_info *kinfo,
> >> -                       const struct dt_device_node *node)
> >> +                       struct dt_device_node *node)
> >>  {
> >>      static const struct dt_device_match skip_matches[] __initconst =
> >>      {
> >> @@ -775,7 +787,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
> >>          DT_MATCH_TIMER,
> >>          { /* sentinel */ },
> >>      };
> >> -    const struct dt_device_node *child;
> >> +    struct dt_device_node *child;
> > 
> > Why do these consts become unwanted?
> 
> Because map_device now calls iommu_assign_dt_device which will update
> next_assigned in the structure dt_device_node.

OK, makes sense.

> >> diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
> >> new file mode 100644
> >> index 0000000..b0bd71d
> >> --- /dev/null
> >> +++ b/xen/drivers/passthrough/arm/iommu.c
> > [...]
> >> +int __init iommu_hardware_setup(void)
> >> +{
> >> +    struct dt_device_node *np;
> >> +    int rc;
> >> +    unsigned int num_iommus = 0;
> >> +
> >> +    dt_for_each_device_node(dt_host, np)
> > 
> > I can't find dt_host in this or any of the previous patches.
> 
> dt_host was defined a while ago by the device tree code (see
> xen/include/xen/device_tree.h).

Doh, I didn't think to look at the existing code ;-)

> >> diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
> >> new file mode 100644
> >> index 0000000..81eec83
> >> --- /dev/null
> >> +++ b/xen/include/asm-arm/iommu.h
> >> [...]
> >> +#define domain_hvm_iommu(d) (&d->arch.hvm_domain.hvm_iommu)
> > 
> > Does this macro give us the freedom to avoid the term "hvm" a bit and
> > use d->arch.iommu?
> 
> It's possible, I just blindly copied from x86.

Ian.
diff mbox

Patch

diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 57f2eb1..1703551 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -9,6 +9,7 @@ 
 HAS_DEVICE_TREE := y
 HAS_VIDEO := y
 HAS_ARM_HDLCD := y
+HAS_PASSTHROUGH := y
 
 CFLAGS += -I$(BASEDIR)/include
 
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 8f20fdf..c42a1c6 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -550,6 +550,9 @@  int arch_domain_create(struct domain *d, unsigned int domcr_flags)
     if ( (d->domain_id == 0) && (rc = domain_vuart_init(d)) )
         goto fail;
 
+    if ( (rc = iommu_domain_init(d)) != 0 )
+        goto fail;
+
     return 0;
 
 fail:
@@ -561,6 +564,10 @@  fail:
 
 void arch_domain_destroy(struct domain *d)
 {
+    /* IOMMU page table is shared with P2M, always call
+     * iommu_domain_destroy() before p2m_teardown().
+     */
+    iommu_domain_destroy(d);
     p2m_teardown(d);
     domain_vgic_free(d);
     domain_vuart_free(d);
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 32861aa..229954b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -669,7 +669,7 @@  static int make_timer_node(const struct domain *d, void *fdt,
 }
 
 /* Map the device in the domain */
-static int map_device(struct domain *d, const struct dt_device_node *dev)
+static int map_device(struct domain *d, struct dt_device_node *dev)
 {
     unsigned int nirq;
     unsigned int naddr;
@@ -684,6 +684,18 @@  static int map_device(struct domain *d, const struct dt_device_node *dev)
 
     DPRINT("%s nirq = %d naddr = %u\n", dt_node_full_name(dev), nirq, naddr);
 
+    if ( dt_device_is_protected(dev) )
+    {
+        DPRINT("%s setup iommu\n", dt_node_full_name(dev));
+        res = iommu_assign_dt_device(d, dev);
+        if ( res )
+        {
+            printk(XENLOG_ERR "Failed to setup the IOMMU for %s\n",
+                   dt_node_full_name(dev));
+            return res;
+        }
+    }
+
     /* Map IRQs */
     for ( i = 0; i < nirq; i++ )
     {
@@ -754,7 +766,7 @@  static int map_device(struct domain *d, const struct dt_device_node *dev)
 }
 
 static int handle_node(struct domain *d, struct kernel_info *kinfo,
-                       const struct dt_device_node *node)
+                       struct dt_device_node *node)
 {
     static const struct dt_device_match skip_matches[] __initconst =
     {
@@ -775,7 +787,7 @@  static int handle_node(struct domain *d, struct kernel_info *kinfo,
         DT_MATCH_TIMER,
         { /* sentinel */ },
     };
-    const struct dt_device_node *child;
+    struct dt_device_node *child;
     int res;
     const char *name;
     const char *path;
@@ -1008,6 +1020,7 @@  int construct_dom0(struct domain *d)
     kinfo.unassigned_mem = dom0_mem;
 
     allocate_memory(d, &kinfo);
+    iommu_dom0_init(d);
 
     rc = kernel_prepare(&kinfo);
     if ( rc < 0 )
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d00c882..d8ed0de 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -412,12 +412,16 @@  static int apply_p2m_changes(struct domain *d,
 
     if ( flush )
     {
+        unsigned long sgfn = paddr_to_pfn(start_gpaddr);
+        unsigned long egfn = paddr_to_pfn(end_gpaddr);
+
         /* At the beginning of the function, Xen is updating VTTBR
          * with the domain where the mappings are created. In this
          * case it's only necessary to flush TLBs on every CPUs with
          * the current VMID (our domain).
          */
         flush_tlb();
+        iommu_iotlb_flush(d, sgfn, egfn - sgfn);
     }
 
     if ( op == ALLOCATE || op == INSERT )
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 1f6d713..a771c30 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -725,6 +725,8 @@  void __init start_xen(unsigned long boot_phys_offset,
     local_irq_enable();
     local_abort_enable();
 
+    iommu_setup();
+
     smp_prepare_cpus(cpus);
 
     initialize_keytable();
diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
index 5a0a35e..16e9027 100644
--- a/xen/drivers/passthrough/Makefile
+++ b/xen/drivers/passthrough/Makefile
@@ -1,6 +1,7 @@ 
 subdir-$(x86) += vtd
 subdir-$(x86) += amd
 subdir-$(x86_64) += x86
+subdir-$(arm) += arm
 
 obj-y += iommu.o
 obj-$(x86) += io.o
diff --git a/xen/drivers/passthrough/arm/Makefile b/xen/drivers/passthrough/arm/Makefile
new file mode 100644
index 0000000..0484b79
--- /dev/null
+++ b/xen/drivers/passthrough/arm/Makefile
@@ -0,0 +1 @@ 
+obj-y += iommu.o
diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c
new file mode 100644
index 0000000..b0bd71d
--- /dev/null
+++ b/xen/drivers/passthrough/arm/iommu.c
@@ -0,0 +1,70 @@ 
+/*
+ * xen/drivers/passthrough/arm/iommu.c
+ *
+ * Generic IOMMU framework via the device tree
+ *
+ * Julien Grall <julien.grall@linaro.org>
+ * Copyright (c) 2014 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.
+ */
+
+#include <xen/lib.h>
+#include <xen/iommu.h>
+#include <xen/device_tree.h>
+#include <asm/device.h>
+
+static const struct iommu_ops *iommu_ops;
+
+const struct iommu_ops *iommu_get_ops(void)
+{
+    return iommu_ops;
+}
+
+void __init iommu_set_ops(const struct iommu_ops *ops)
+{
+    BUG_ON(ops == NULL);
+
+    if ( iommu_ops && iommu_ops != ops )
+        printk("WARNING: IOMMU ops already set to a different value\n");
+
+    iommu_ops = ops;
+}
+
+int __init iommu_hardware_setup(void)
+{
+    struct dt_device_node *np;
+    int rc;
+    unsigned int num_iommus = 0;
+
+    dt_for_each_device_node(dt_host, np)
+    {
+        rc = device_init(np, DEVICE_IOMMU, NULL);
+        if ( !rc )
+            num_iommus++;
+    }
+
+    return ( num_iommus > 0 ) ? 0 : -ENODEV;
+}
+
+int arch_iommu_domain_init(struct domain *d)
+{
+    int ret;
+
+    ret = iommu_dt_domain_init(d);
+
+    return ret;
+}
+
+void arch_iommu_domain_destroy(struct domain *d)
+{
+    iommu_dt_domain_destroy(d);
+}
diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
index 9e47ca6..ed04344 100644
--- a/xen/include/asm-arm/device.h
+++ b/xen/include/asm-arm/device.h
@@ -6,7 +6,8 @@ 
 
 enum device_type
 {
-    DEVICE_SERIAL
+    DEVICE_SERIAL,
+    DEVICE_IOMMU,
 };
 
 struct device_desc {
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index bc20a15..ad6587a 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -9,6 +9,7 @@ 
 #include <asm/vfp.h>
 #include <public/hvm/params.h>
 #include <xen/serial.h>
+#include <xen/hvm/iommu.h>
 
 /* Represents state corresponding to a block of 32 interrupts */
 struct vgic_irq_rank {
@@ -72,6 +73,7 @@  struct pending_irq
 struct hvm_domain
 {
     uint64_t              params[HVM_NR_PARAMS];
+    struct hvm_iommu      hvm_iommu;
 }  __cacheline_aligned;
 
 #ifdef CONFIG_ARM_64
diff --git a/xen/include/asm-arm/hvm/iommu.h b/xen/include/asm-arm/hvm/iommu.h
new file mode 100644
index 0000000..461c8cf
--- /dev/null
+++ b/xen/include/asm-arm/hvm/iommu.h
@@ -0,0 +1,10 @@ 
+#ifndef __ASM_ARM_HVM_IOMMU_H_
+#define __ASM_ARM_HVM_IOMMU_H_
+
+struct arch_hvm_iommu
+{
+    /* Private information for the IOMMU drivers */
+    void *priv;
+};
+
+#endif /* __ASM_ARM_HVM_IOMMU_H_ */
diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
new file mode 100644
index 0000000..81eec83
--- /dev/null
+++ b/xen/include/asm-arm/iommu.h
@@ -0,0 +1,36 @@ 
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+*/
+#ifndef __ARCH_ARM_IOMMU_H__
+#define __ARCH_ARM_IOMMU_H__
+
+/* Always share P2M Table between the CPU and the IOMMU */
+#define iommu_use_hap_pt(d) (1)
+#define domain_hvm_iommu(d) (&d->arch.hvm_domain.hvm_iommu)
+
+const struct iommu_ops *iommu_get_ops(void);
+void __init iommu_set_ops(const struct iommu_ops *ops);
+
+int __init iommu_hardware_setup(void);
+
+#endif /* __ARCH_ARM_IOMMU_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */