[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <010d3cbd-ef74-ad21-c735-0af8b18955e6@huawei.com>
Date: Fri, 12 Apr 2019 11:26:57 +0100
From: John Garry <john.garry@...wei.com>
To: Zhen Lei <thunder.leizhen@...wei.com>,
Jean-Philippe Brucker <jean-philippe.brucker@....com>,
Robin Murphy <robin.murphy@....com>,
"Will Deacon" <will.deacon@....com>,
Joerg Roedel <joro@...tes.org>,
Jonathan Corbet <corbet@....net>,
linux-doc <linux-doc@...r.kernel.org>,
Sebastian Ott <sebott@...ux.ibm.com>,
Gerald Schaefer <gerald.schaefer@...ibm.com>,
"Martin Schwidefsky" <schwidefsky@...ibm.com>,
Heiko Carstens <heiko.carstens@...ibm.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Paul Mackerras <paulus@...ba.org>,
"Michael Ellerman" <mpe@...erman.id.au>,
Tony Luck <tony.luck@...el.com>,
Fenghua Yu <fenghua.yu@...el.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
"H . Peter Anvin" <hpa@...or.com>,
David Woodhouse <dwmw2@...radead.org>,
iommu <iommu@...ts.linux-foundation.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-s390 <linux-s390@...r.kernel.org>,
linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
x86 <x86@...nel.org>, linux-ia64 <linux-ia64@...r.kernel.org>
CC: Hanjun Guo <guohanjun@...wei.com>
Subject: Re: [PATCH v5 1/6] iommu: add generic boot option iommu.dma_mode
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 <thunder.leizhen@...wei.com>
> ---
> 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;
>
Powered by blists - more mailing lists