From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH v5 1/6] iommu: add generic boot option iommu.dma_mode References: <20190409125308.18304-1-thunder.leizhen@huawei.com> <20190409125308.18304-2-thunder.leizhen@huawei.com> From: John Garry Message-ID: <010d3cbd-ef74-ad21-c735-0af8b18955e6@huawei.com> Date: Fri, 12 Apr 2019 11:26:57 +0100 MIME-Version: 1.0 In-Reply-To: <20190409125308.18304-2-thunder.leizhen@huawei.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-doc-owner@vger.kernel.org List-Archive: List-Post: To: Zhen Lei , Jean-Philippe Brucker , Robin Murphy , Will Deacon , Joerg Roedel , Jonathan Corbet , linux-doc , Sebastian Ott , Gerald Schaefer , Martin Schwidefsky , Heiko Carstens , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Tony Luck , Fenghua Yu , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H . Peter Anvin" , David Woodhouse , iommu , linux-kernel , linux-s390 , linuxppc-dev , x86 , linux-ia64 Cc: Hanjun Guo List-ID: On 09/04/2019 13:53, Zhen Lei wrote: > Currently the IOMMU dma contains 3 modes: passthrough, lazy, strict. The > passthrough mode bypass the IOMMU, the lazy mode defer the invalidation > of hardware TLBs, and the strict mode invalidate IOMMU hardware TLBs > synchronously. The three modes are mutually exclusive. But the current > boot options are confused, such as: iommu.passthrough and iommu.strict, > because they are no good to be coexist. So add iommu.dma_mode. > > Signed-off-by: Zhen Lei > --- > Documentation/admin-guide/kernel-parameters.txt | 19 ++++++++ > drivers/iommu/iommu.c | 59 ++++++++++++++++++++----- > include/linux/iommu.h | 5 +++ > 3 files changed, 71 insertions(+), 12 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index 2b8ee90bb64470d..f7766f8ac8b9084 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -1811,6 +1811,25 @@ > 1 - Bypass the IOMMU for DMA. > unset - Use value of CONFIG_IOMMU_DEFAULT_PASSTHROUGH. > > + iommu.dma_mode= Configure default dma mode. if unset, use the value > + of CONFIG_IOMMU_DEFAULT_PASSTHROUGH to determine > + passthrough or not. To me, for unset it's unclear what we default to. So if unset and also CONFIG_IOMMU_DEFAULT_PASSTHROUGH is not set, do we get lazy or strict mode? (note: I'm ignoring backwards compatibility and interaction of iommu.strict and .passthorugh also, more below). Could we considering introducing config DEFAULT_IOMMU_DMA_MODE, similar to DEFAULT_IOSCHED? > + Note: For historical reasons, ARM64/S390/PPC/X86 have > + their specific options. Currently, only ARM64 support > + this boot option, and hope other ARCHs to use this as > + generic boot option. > + passthrough > + Configure DMA to bypass the IOMMU by default. > + lazy > + Request that DMA unmap operations use deferred > + invalidation of hardware TLBs, for increased > + throughput at the cost of reduced device isolation. > + Will fall back to strict mode if not supported by > + the relevant IOMMU driver. > + strict > + DMA unmap operations invalidate IOMMU hardware TLBs > + synchronously. > + > io7= [HW] IO7 for Marvel based alpha systems > See comment before marvel_specify_io7 in > arch/alpha/kernel/core_marvel.c. > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index 109de67d5d727c2..df1ce8e22385b48 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -38,12 +38,13 @@ > > static struct kset *iommu_group_kset; > static DEFINE_IDA(iommu_group_ida); > + > #ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH > -static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY; > +#define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_PASSTHROUGH > #else > -static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA; > +#define IOMMU_DEFAULT_DMA_MODE IOMMU_DMA_MODE_STRICT > #endif > -static bool iommu_dma_strict __read_mostly = true; > +static int iommu_default_dma_mode __read_mostly = IOMMU_DEFAULT_DMA_MODE; > > struct iommu_callback_data { > const struct iommu_ops *ops; > @@ -147,20 +148,51 @@ static int __init iommu_set_def_domain_type(char *str) > int ret; > > ret = kstrtobool(str, &pt); > - if (ret) > - return ret; > + if (!ret && pt) > + iommu_default_dma_mode = IOMMU_DMA_MODE_PASSTHROUGH; > > - iommu_def_domain_type = pt ? IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA; > - return 0; > + return ret; > } > early_param("iommu.passthrough", iommu_set_def_domain_type); > > static int __init iommu_dma_setup(char *str) > { > - return kstrtobool(str, &iommu_dma_strict); > + bool strict; > + int ret; > + > + ret = kstrtobool(str, &strict); > + if (!ret) > + iommu_default_dma_mode = strict ? > + IOMMU_DMA_MODE_STRICT : IOMMU_DMA_MODE_LAZY; > + > + return ret; > } > early_param("iommu.strict", iommu_dma_setup); > > +static int __init iommu_dma_mode_setup(char *str) > +{ > + if (!str) > + goto fail; > + > + if (!strncmp(str, "passthrough", 11)) > + iommu_default_dma_mode = IOMMU_DMA_MODE_PASSTHROUGH; > + else if (!strncmp(str, "lazy", 4)) > + iommu_default_dma_mode = IOMMU_DMA_MODE_LAZY; > + else if (!strncmp(str, "strict", 6)) > + iommu_default_dma_mode = IOMMU_DMA_MODE_STRICT; > + else > + goto fail; > + > + pr_info("Force dma mode to be %d\n", iommu_default_dma_mode); What happens if the cmdline option iommu.dma_mode is passed multiple times? We get mutliple - possibily conflicting - prints, right? And do we need to have backwards compatibility, such that the setting for iommu.strict or iommu.passthrough trumps iommu.dma_mode, regardless of order? > + > + return 0; > + > +fail: > + pr_debug("Boot option iommu.dma_mode is incorrect, ignored\n"); > + return -EINVAL; > +} > +early_param("iommu.dma_mode", iommu_dma_mode_setup); > + > static ssize_t iommu_group_attr_show(struct kobject *kobj, > struct attribute *__attr, char *buf) > { > @@ -1102,14 +1134,17 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev) > */ > if (!group->default_domain) { > struct iommu_domain *dom; > + int def_domain_type = > + (iommu_default_dma_mode == IOMMU_DMA_MODE_PASSTHROUGH) > + ? IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA; > > - dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type); > - if (!dom && iommu_def_domain_type != IOMMU_DOMAIN_DMA) { > + dom = __iommu_domain_alloc(dev->bus, def_domain_type); > + if (!dom && def_domain_type != IOMMU_DOMAIN_DMA) { > dom = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_DMA); > if (dom) { > dev_warn(dev, > "failed to allocate default IOMMU domain of type %u; falling back to IOMMU_DOMAIN_DMA", > - iommu_def_domain_type); > + def_domain_type); > } > } > > @@ -1117,7 +1152,7 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev) > if (!group->domain) > group->domain = dom; > > - if (dom && !iommu_dma_strict) { > + if (dom && (iommu_default_dma_mode == IOMMU_DMA_MODE_LAZY)) { > int attr = 1; > iommu_domain_set_attr(dom, > DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE, > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index ffbbc7e39ceeba3..c3f4e3416176496 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -42,6 +42,11 @@ > */ > #define IOMMU_PRIV (1 << 5) > > + > +#define IOMMU_DMA_MODE_STRICT 0x0 > +#define IOMMU_DMA_MODE_LAZY 0x1 > +#define IOMMU_DMA_MODE_PASSTHROUGH 0x2 > + > struct iommu_ops; > struct iommu_group; > struct bus_type; >