| Message ID | 1391794991-5919-11-git-send-email-julien.grall@linaro.org |
|---|---|
| State | RFC |
| Headers | show |
On Fri, 2014-02-07 at 17:43 +0000, Julien Grall wrote: > This patch contains the architecture to use IOMMU on ARM. There is no s/IOMMU/IOMMUs/ > IOMMU drivers on this patch. > > The code will run through the device tree and will initialize every IOMMUs. s/IOMMUs/IOMMU/ > It's possible to have multiple IOMMUs on the same platform, but they should s/should/must/ I think for now? > be handled with the same drivers. s/drivers/driver/ > For now, there is no support for using multiple iommu drivers at runtime. But this is in principal possible in the future, right? (I shudder to think what the designers of an SoC with multiple different SMMUs on it would have to be smoking...) You should mention somewhere that on ARM these are called SMMUs not IOMMUs. > Each new IOMMU drivers should contain: > > static const char * const myiommu_dt_compat[] __initcontst = "__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 This is the same as for any other driver, right? > @@ -568,6 +571,10 @@ fail: > > void arch_domain_destroy(struct domain *d) > { > + /* IOMMU page table is shared with P2M, always call > + * iommu_domain_destroy() before p2m_teardown(). It would be worth adding some commentary on the design we are using (decisions such as whether to share PTs with the stage 2 MMU) in the commit message as well. I suppose this requirement puts constraints on the SMMU hardware we can support. I think it is fine to live with that until someone shows up with an SMMU with pagetables that are incompatible with the stage 2 paging ones. > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 1f6d713..5a687d1 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(); /* setup iommu if available */ Comment is a bit redundant ;-) > diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c > new file mode 100644 > index 0000000..7cf36cd > --- /dev/null > +++ b/xen/drivers/passthrough/arm/iommu.c > @@ -0,0 +1,65 @@ > +/* > + * xen/drivers/passthrough/arm/iommu.c > + * > + * Generic IOMMU framework via the device tree > + * > + * Julien Grall <julien.grall@linaro.org> > + * Copyright (c) 2013 Linaro Limited. It's not 2013 any more... > 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_ */ Emacs magic block please. That was a surprisingly small and uncontroversial patch. Well done. Ian.
Hi Ian, On 02/19/2014 12:49 PM, Ian Campbell wrote: > On Fri, 2014-02-07 at 17:43 +0000, Julien Grall wrote: >> This patch contains the architecture to use IOMMU on ARM. There is no > > s/IOMMU/IOMMUs/ > >> IOMMU drivers on this patch. >> >> The code will run through the device tree and will initialize every IOMMUs. > > s/IOMMUs/IOMMU/ > >> It's possible to have multiple IOMMUs on the same platform, but they should > > s/should/must/ I think for now? Right. >> be handled with the same drivers. > > s/drivers/driver/ > >> For now, there is no support for using multiple iommu drivers at runtime. > > But this is in principal possible in the future, right? (I shudder to > think what the designers of an SoC with multiple different SMMUs on it > would have to be smoking...) In theory yes. I will wait that someone want to support a such platfomr on Xen before implement it :). > > You should mention somewhere that on ARM these are called SMMUs not > IOMMUs. I think SMMU is the name used for ARM IOMMU. Samsung and TI doesn't use the term SMMU. > >> Each new IOMMU drivers should contain: >> >> static const char * const myiommu_dt_compat[] __initcontst = > > "__initconst". Oops. Will fix it. >> { >> /* 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 > > This is the same as for any other driver, right? Yes, the only change is we need to specify DEVICE_IOMMU in DT_DEVICE_START. > >> @@ -568,6 +571,10 @@ fail: >> >> void arch_domain_destroy(struct domain *d) >> { >> + /* IOMMU page table is shared with P2M, always call >> + * iommu_domain_destroy() before p2m_teardown(). > > It would be worth adding some commentary on the design we are using > (decisions such as whether to share PTs with the stage 2 MMU) in the > commit message as well. I will do. > I suppose this requirement puts constraints on the SMMU hardware we can > support. I think it is fine to live with that until someone shows up > with an SMMU with pagetables that are incompatible with the stage 2 > paging ones. Adding support for a such hardware should not need too much code. >> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c >> index 1f6d713..5a687d1 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(); /* setup iommu if available */ > > Comment is a bit redundant ;-) Copied for x86/setup.c. I will remove it. > >> diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c >> new file mode 100644 >> index 0000000..7cf36cd >> --- /dev/null >> +++ b/xen/drivers/passthrough/arm/iommu.c >> @@ -0,0 +1,65 @@ >> +/* >> + * xen/drivers/passthrough/arm/iommu.c >> + * >> + * Generic IOMMU framework via the device tree >> + * >> + * Julien Grall <julien.grall@linaro.org> >> + * Copyright (c) 2013 Linaro Limited. > > It's not 2013 any more... I have started the implementation in 2013 and forgot to update it. > >> 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_ */ > > Emacs magic block please. I will do. > That was a surprisingly small and uncontroversial patch. Well done. The IOMMU framework in Xen was well designed. cheers,
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 c279a27..b3c0dda 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -557,6 +557,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: @@ -568,6 +571,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 6d9a801..1f845d7 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1091,6 +1091,8 @@ int construct_dom0(struct domain *d) } } + iommu_dom0_init(d); + return 0; } diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 85ca330..57304d0 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -401,12 +401,16 @@ static int create_p2m_entries(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..5a687d1 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(); /* setup iommu if available */ + smp_prepare_cpus(cpus); initialize_keytable(); diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile index 51e0a0d..aded1ea 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) += iommu_x86.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..7cf36cd --- /dev/null +++ b/xen/drivers/passthrough/arm/iommu.c @@ -0,0 +1,65 @@ +/* + * xen/drivers/passthrough/arm/iommu.c + * + * Generic IOMMU framework via the device tree + * + * Julien Grall <julien.grall@linaro.org> + * Copyright (c) 2013 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) +{ + return 0; +} + +void arch_iommu_domain_destroy(struct domain *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 af8c64b..15d814a 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: + */
This patch contains the architecture to use IOMMU on ARM. There is no IOMMU drivers on this patch. The code will run through the device tree and will initialize every IOMMUs. It's possible to have multiple IOMMUs on the same platform, but they should be handled with the same drivers. 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[] __initcontst = { /* 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> --- xen/arch/arm/Rules.mk | 1 + xen/arch/arm/domain.c | 7 ++++ xen/arch/arm/domain_build.c | 2 ++ 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 | 65 ++++++++++++++++++++++++++++++++++ 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, 133 insertions(+), 1 deletion(-) 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