Hi, This patch-set started out small to overwrite the default passthrough setting (through CONFIG_IOMMU_DEFAULT_PASSTHROUGH=y) when SME is active. But on the way to that Tom reminded me that the current ways to configure passthrough/no-passthrough modes for IOMMU on x86 is a mess. So I added a few more patches to clean that up a bit, getting rid of the iommu_pass_through variable on the way.This information is now kept only in iommu code, with helpers to change that setting from architecture code. And of course this patch-set still disables IOMMU Passthrough mode when SME is active even when CONFIG_IOMMU_DEFAULT_PASSTHROUGH=y is set. The reason for that change is that SME with passthrough mode turned out to be fragile with devices requiring SWIOTLB, mainly because SWIOTLB has a maximum allocation size of 256kb and a limit overall size of the bounce buffer. Therefore having IOMMU in translation mode by default is better when SME is active on a system. Please review. Thanks, Joerg Changes since v1: - Cleaned up the kernel command line parameters to configure passthrough/translated mode, getting rid of the global iommu_pass_through variable Joerg Roedel (10): iommu: Add helpers to set/get default domain type iommu/amd: Request passthrough mode from IOMMU core iommu/vt-d: Request passthrough mode from IOMMU core x86/dma: Get rid of iommu_pass_through ia64: Get rid of iommu_pass_through iommu: Remember when default domain type was set on kernel command line iommu: Print default domain type on boot iommu: Set default domain type at runtime iommu: Disable passthrough mode when SME is active Documentation: Update Documentation for iommu.passthrough .../admin-guide/kernel-parameters.txt | 2 +- arch/ia64/include/asm/iommu.h | 2 - arch/ia64/kernel/pci-dma.c | 2 - arch/x86/include/asm/iommu.h | 1 - arch/x86/kernel/pci-dma.c | 11 +-- drivers/iommu/amd_iommu.c | 6 +- drivers/iommu/intel-iommu.c | 2 +- drivers/iommu/iommu.c | 83 +++++++++++++++++-- include/linux/iommu.h | 16 ++++ 9 files changed, 101 insertions(+), 24 deletions(-) -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
From: Joerg Roedel <jroedel@suse.de> Add a couple of functions to allow changing the default domain type from architecture code and a function for iommu drivers to request whether the default domain is passthrough. Signed-off-by: Joerg Roedel <jroedel@suse.de> --- drivers/iommu/iommu.c | 16 ++++++++++++++++ include/linux/iommu.h | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 0c674d80c37f..f187e85a074b 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2196,6 +2196,22 @@ int iommu_request_dma_domain_for_dev(struct device *dev) return request_default_domain_for_dev(dev, IOMMU_DOMAIN_DMA); } +void iommu_set_default_passthrough(void) +{ + iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY; +} + +void iommu_set_default_translated(void) +{ + iommu_def_domain_type = IOMMU_DOMAIN_DMA; +} + +bool iommu_default_passthrough(void) +{ + return iommu_def_domain_type == IOMMU_DOMAIN_IDENTITY; +} +EXPORT_SYMBOL_GPL(iommu_default_passthrough); + const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode) { const struct iommu_ops *ops = NULL; diff --git a/include/linux/iommu.h b/include/linux/iommu.h index fdc355ccc570..58c3e3e5f157 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -413,6 +413,9 @@ extern void iommu_get_resv_regions(struct device *dev, struct list_head *list); extern void iommu_put_resv_regions(struct device *dev, struct list_head *list); extern int iommu_request_dm_for_dev(struct device *dev); extern int iommu_request_dma_domain_for_dev(struct device *dev); +extern void iommu_set_default_passthrough(void); +extern void iommu_set_default_translated(void); +extern bool iommu_default_passthrough(void); extern struct iommu_resv_region * iommu_alloc_resv_region(phys_addr_t start, size_t length, int prot, enum iommu_resv_type type); @@ -694,6 +697,19 @@ static inline int iommu_request_dma_domain_for_dev(struct device *dev) return -ENODEV; } +static inline void iommu_set_default_passthrough(void) +{ +} + +static inline void iommu_set_default_translated(void) +{ +} + +static inline bool iommu_default_passthrough(void) +{ + return true; +} + static inline int iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group) { -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
From: Joerg Roedel <jroedel@suse.de> Get rid of the iommu_pass_through variable and request passthrough mode via the new iommu core function. Signed-off-by: Joerg Roedel <jroedel@suse.de> --- drivers/iommu/amd_iommu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c index b607a92791d3..7434b34d7a94 100644 --- a/drivers/iommu/amd_iommu.c +++ b/drivers/iommu/amd_iommu.c @@ -436,7 +436,7 @@ static int iommu_init_device(struct device *dev) * invalid address), we ignore the capability for the device so * it'll be forced to go into translation mode. */ - if ((iommu_pass_through || !amd_iommu_force_isolation) && + if ((iommu_default_passthrough() || !amd_iommu_force_isolation) && dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) { struct amd_iommu *iommu; @@ -2226,7 +2226,7 @@ static int amd_iommu_add_device(struct device *dev) BUG_ON(!dev_data); - if (iommu_pass_through || dev_data->iommu_v2) + if (dev_data->iommu_v2) iommu_request_dm_for_dev(dev); /* Domains are initialized for this device - have a look what we ended up with */ @@ -2805,7 +2805,7 @@ int __init amd_iommu_init_api(void) int __init amd_iommu_init_dma_ops(void) { - swiotlb = (iommu_pass_through || sme_me_mask) ? 1 : 0; + swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0; iommu_detected = 1; if (amd_iommu_unmap_flush) -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
From: Joerg Roedel <jroedel@suse.de> Get rid of the iommu_pass_through variable and request passthrough mode via the new iommu core function. Signed-off-by: Joerg Roedel <jroedel@suse.de> --- drivers/iommu/intel-iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index bdaed2da8a55..234bc2b55c59 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -3267,7 +3267,7 @@ static int __init init_dmars(void) iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH); } - if (iommu_pass_through) + if (iommu_default_passthrough()) iommu_identity_mapping |= IDENTMAP_ALL; #ifdef CONFIG_INTEL_IOMMU_BROKEN_GFX_WA -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
From: Joerg Roedel <jroedel@suse.de> This variable has no users anymore. Remove it and tell the IOMMU code via its new functions about requested DMA modes. Signed-off-by: Joerg Roedel <jroedel@suse.de> --- arch/x86/include/asm/iommu.h | 1 - arch/x86/kernel/pci-dma.c | 11 +++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/arch/x86/include/asm/iommu.h b/arch/x86/include/asm/iommu.h index baedab8ac538..b91623d521d9 100644 --- a/arch/x86/include/asm/iommu.h +++ b/arch/x86/include/asm/iommu.h @@ -4,7 +4,6 @@ extern int force_iommu, no_iommu; extern int iommu_detected; -extern int iommu_pass_through; /* 10 seconds */ #define DMAR_OPERATION_TIMEOUT ((cycles_t) tsc_khz*10*1000) diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c index f62b498b18fb..a6fd479d4a71 100644 --- a/arch/x86/kernel/pci-dma.c +++ b/arch/x86/kernel/pci-dma.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include <linux/dma-direct.h> #include <linux/dma-debug.h> +#include <linux/iommu.h> #include <linux/dmar.h> #include <linux/export.h> #include <linux/memblock.h> @@ -43,12 +44,6 @@ int iommu_detected __read_mostly = 0; * It is also possible to disable by default in kernel config, and enable with * iommu=nopt at boot time. */ -#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH -int iommu_pass_through __read_mostly = 1; -#else -int iommu_pass_through __read_mostly; -#endif - extern struct iommu_table_entry __iommu_table[], __iommu_table_end[]; void __init pci_iommu_alloc(void) @@ -120,9 +115,9 @@ static __init int iommu_setup(char *p) swiotlb = 1; #endif if (!strncmp(p, "pt", 2)) - iommu_pass_through = 1; + iommu_set_default_passthrough(); if (!strncmp(p, "nopt", 4)) - iommu_pass_through = 0; + iommu_set_default_translated(); gart_parse_options(p); -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
From: Joerg Roedel <jroedel@suse.de> This variable has no users anymore so it can be removed. Signed-off-by: Joerg Roedel <jroedel@suse.de> --- arch/ia64/include/asm/iommu.h | 2 -- arch/ia64/kernel/pci-dma.c | 2 -- 2 files changed, 4 deletions(-) diff --git a/arch/ia64/include/asm/iommu.h b/arch/ia64/include/asm/iommu.h index 7429a72f3f92..92aceef63710 100644 --- a/arch/ia64/include/asm/iommu.h +++ b/arch/ia64/include/asm/iommu.h @@ -8,10 +8,8 @@ extern void no_iommu_init(void); #ifdef CONFIG_INTEL_IOMMU extern int force_iommu, no_iommu; -extern int iommu_pass_through; extern int iommu_detected; #else -#define iommu_pass_through (0) #define no_iommu (1) #define iommu_detected (0) #endif diff --git a/arch/ia64/kernel/pci-dma.c b/arch/ia64/kernel/pci-dma.c index fe988c49f01c..f5d49cd3fbb0 100644 --- a/arch/ia64/kernel/pci-dma.c +++ b/arch/ia64/kernel/pci-dma.c @@ -22,8 +22,6 @@ int force_iommu __read_mostly = 1; int force_iommu __read_mostly; #endif -int iommu_pass_through; - static int __init pci_iommu_init(void) { if (iommu_detected) -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
From: Joerg Roedel <jroedel@suse.de> Introduce an extensible concept to remember when certain configuration settings for the IOMMU code have been set on the kernel command line. This will be used later to prevent overwriting these settings with other defaults. Signed-off-by: Joerg Roedel <jroedel@suse.de> --- drivers/iommu/iommu.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index f187e85a074b..e1feb4061b8b 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -32,6 +32,7 @@ static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY; static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA; #endif static bool iommu_dma_strict __read_mostly = true; +static u32 iommu_cmd_line __read_mostly; struct iommu_group { struct kobject kobj; @@ -68,6 +69,18 @@ static const char * const iommu_group_resv_type_string[] = { [IOMMU_RESV_SW_MSI] = "msi", }; +#define IOMMU_CMD_LINE_DMA_API (1 << 0) + +static void iommu_set_cmd_line_dma_api(void) +{ + iommu_cmd_line |= IOMMU_CMD_LINE_DMA_API; +} + +static bool __maybe_unused iommu_cmd_line_dma_api(void) +{ + return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API); +} + #define IOMMU_GROUP_ATTR(_name, _mode, _show, _store) \ struct iommu_group_attribute iommu_group_attr_##_name = \ __ATTR(_name, _mode, _show, _store) @@ -165,6 +178,8 @@ static int __init iommu_set_def_domain_type(char *str) if (ret) return ret; + iommu_set_cmd_line_dma_api(); + iommu_def_domain_type = pt ? IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA; return 0; } -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
From: Joerg Roedel <jroedel@suse.de> Introduce a subsys_initcall for IOMMU code and use it to print the default domain type at boot. Signed-off-by: Joerg Roedel <jroedel@suse.de> --- drivers/iommu/iommu.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index e1feb4061b8b..233bc22b487e 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -93,12 +93,40 @@ struct iommu_group_attribute iommu_group_attr_##_name = \ static LIST_HEAD(iommu_device_list); static DEFINE_SPINLOCK(iommu_device_lock); +/* + * Use a function instead of an array here because the domain-type is a + * bit-field, so an array would waste memory. + */ +static const char *iommu_domain_type_str(unsigned int t) +{ + switch (t) { + case IOMMU_DOMAIN_BLOCKED: + return "Blocked"; + case IOMMU_DOMAIN_IDENTITY: + return "Passthrough"; + case IOMMU_DOMAIN_UNMANAGED: + return "Unmanaged"; + case IOMMU_DOMAIN_DMA: + return "Translated"; + default: + return "Unknown"; + } +} + +static int __init iommu_subsys_init(void) +{ + pr_info("Default domain type: %s\n", + iommu_domain_type_str(iommu_def_domain_type)); + + return 0; +} +subsys_initcall(iommu_subsys_init); + int iommu_device_register(struct iommu_device *iommu) { spin_lock(&iommu_device_lock); list_add_tail(&iommu->list, &iommu_device_list); spin_unlock(&iommu_device_lock); - return 0; } -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
From: Joerg Roedel <jroedel@suse.de> Set the default domain-type at runtime, not at compile-time. This keeps default domain type setting in one place when we have to change it at runtime. Signed-off-by: Joerg Roedel <jroedel@suse.de> --- drivers/iommu/iommu.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 233bc22b487e..96cc7cc8ab21 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -26,11 +26,8 @@ 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; -#else -static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA; -#endif + +static unsigned int iommu_def_domain_type __read_mostly; static bool iommu_dma_strict __read_mostly = true; static u32 iommu_cmd_line __read_mostly; @@ -76,7 +73,7 @@ static void iommu_set_cmd_line_dma_api(void) iommu_cmd_line |= IOMMU_CMD_LINE_DMA_API; } -static bool __maybe_unused iommu_cmd_line_dma_api(void) +static bool iommu_cmd_line_dma_api(void) { return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API); } @@ -115,8 +112,18 @@ static const char *iommu_domain_type_str(unsigned int t) static int __init iommu_subsys_init(void) { - pr_info("Default domain type: %s\n", - iommu_domain_type_str(iommu_def_domain_type)); + bool cmd_line = iommu_cmd_line_dma_api(); + + if (!cmd_line) { + if (IS_ENABLED(CONFIG_IOMMU_DEFAULT_PASSTHROUGH)) + iommu_set_default_passthrough(); + else + iommu_set_default_translated(); + } + + pr_info("Default domain type: %s %s\n", + iommu_domain_type_str(iommu_def_domain_type), + cmd_line ? "(set via kernel command line)" : ""); return 0; } -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
From: Joerg Roedel <jroedel@suse.de> Using Passthrough mode when SME is active causes certain devices to use the SWIOTLB bounce buffer. The bounce buffer code has an upper limit of 256kb for the size of DMA allocations, which is too small for certain devices and causes them to fail. With this patch we enable IOMMU by default when SME is active in the system, making the default configuration work for more systems than it does now. Users that don't want IOMMUs to be enabled still can disable them with kernel parameters. Signed-off-by: Joerg Roedel <jroedel@suse.de> --- drivers/iommu/iommu.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 96cc7cc8ab21..4bc9025a9975 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -119,6 +119,11 @@ static int __init iommu_subsys_init(void) iommu_set_default_passthrough(); else iommu_set_default_translated(); + + if (iommu_default_passthrough() && sme_active()) { + pr_info("SME detected - Disabling default IOMMU Passthrough\n"); + iommu_set_default_translated(); + } } pr_info("Default domain type: %s %s\n", -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
From: Joerg Roedel <jroedel@suse.de> This kernel parameter now takes also effect on X86. Signed-off-by: Joerg Roedel <jroedel@suse.de> --- Documentation/admin-guide/kernel-parameters.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 47d981a86e2f..2d5dfa46e88a 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1811,7 +1811,7 @@ synchronously. iommu.passthrough= - [ARM64] Configure DMA to bypass the IOMMU by default. + [ARM64, X86] Configure DMA to bypass the IOMMU by default. Format: { "0" | "1" } 0 - Use IOMMU translation for DMA. 1 - Bypass the IOMMU for DMA. -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hi Joerg, On 8/14/19 9:38 PM, Joerg Roedel wrote: > From: Joerg Roedel <jroedel@suse.de> > > Get rid of the iommu_pass_through variable and request > passthrough mode via the new iommu core function. > > Signed-off-by: Joerg Roedel <jroedel@suse.de> Looks good to me. Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com> > --- > drivers/iommu/intel-iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > index bdaed2da8a55..234bc2b55c59 100644 > --- a/drivers/iommu/intel-iommu.c > +++ b/drivers/iommu/intel-iommu.c > @@ -3267,7 +3267,7 @@ static int __init init_dmars(void) > iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH); > } > > - if (iommu_pass_through) > + if (iommu_default_passthrough()) > iommu_identity_mapping |= IDENTMAP_ALL; > > #ifdef CONFIG_INTEL_IOMMU_BROKEN_GFX_WA > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hi, On 8/14/19 9:38 PM, Joerg Roedel wrote: > From: Joerg Roedel <jroedel@suse.de> > > This variable has no users anymore. Remove it and tell the > IOMMU code via its new functions about requested DMA modes. > > Signed-off-by: Joerg Roedel <jroedel@suse.de> This will also simplify the procedures in iommu_probe_device() on x86 platforms. Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com> > --- > arch/x86/include/asm/iommu.h | 1 - > arch/x86/kernel/pci-dma.c | 11 +++-------- > 2 files changed, 3 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/include/asm/iommu.h b/arch/x86/include/asm/iommu.h > index baedab8ac538..b91623d521d9 100644 > --- a/arch/x86/include/asm/iommu.h > +++ b/arch/x86/include/asm/iommu.h > @@ -4,7 +4,6 @@ > > extern int force_iommu, no_iommu; > extern int iommu_detected; > -extern int iommu_pass_through; > > /* 10 seconds */ > #define DMAR_OPERATION_TIMEOUT ((cycles_t) tsc_khz*10*1000) > diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c > index f62b498b18fb..a6fd479d4a71 100644 > --- a/arch/x86/kernel/pci-dma.c > +++ b/arch/x86/kernel/pci-dma.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > #include <linux/dma-direct.h> > #include <linux/dma-debug.h> > +#include <linux/iommu.h> > #include <linux/dmar.h> > #include <linux/export.h> > #include <linux/memblock.h> > @@ -43,12 +44,6 @@ int iommu_detected __read_mostly = 0; > * It is also possible to disable by default in kernel config, and enable with > * iommu=nopt at boot time. > */ > -#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH > -int iommu_pass_through __read_mostly = 1; > -#else > -int iommu_pass_through __read_mostly; > -#endif > - > extern struct iommu_table_entry __iommu_table[], __iommu_table_end[]; > > void __init pci_iommu_alloc(void) > @@ -120,9 +115,9 @@ static __init int iommu_setup(char *p) > swiotlb = 1; > #endif > if (!strncmp(p, "pt", 2)) > - iommu_pass_through = 1; > + iommu_set_default_passthrough(); > if (!strncmp(p, "nopt", 4)) > - iommu_pass_through = 0; > + iommu_set_default_translated(); > > gart_parse_options(p); > > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hi, On 8/14/19 9:38 PM, Joerg Roedel wrote: > From: Joerg Roedel <jroedel@suse.de> > > Introduce an extensible concept to remember when certain > configuration settings for the IOMMU code have been set on > the kernel command line. > > This will be used later to prevent overwriting these > settings with other defaults. > > Signed-off-by: Joerg Roedel <jroedel@suse.de> > --- > drivers/iommu/iommu.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index f187e85a074b..e1feb4061b8b 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -32,6 +32,7 @@ static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY; > static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA; > #endif > static bool iommu_dma_strict __read_mostly = true; > +static u32 iommu_cmd_line __read_mostly; > > struct iommu_group { > struct kobject kobj; > @@ -68,6 +69,18 @@ static const char * const iommu_group_resv_type_string[] = { > [IOMMU_RESV_SW_MSI] = "msi", > }; > > +#define IOMMU_CMD_LINE_DMA_API (1 << 0) Prefer BIT() micro? > + > +static void iommu_set_cmd_line_dma_api(void) > +{ > + iommu_cmd_line |= IOMMU_CMD_LINE_DMA_API; > +} > + > +static bool __maybe_unused iommu_cmd_line_dma_api(void) > +{ > + return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API); > +} > + > #define IOMMU_GROUP_ATTR(_name, _mode, _show, _store) \ > struct iommu_group_attribute iommu_group_attr_##_name = \ > __ATTR(_name, _mode, _show, _store) > @@ -165,6 +178,8 @@ static int __init iommu_set_def_domain_type(char *str) > if (ret) > return ret; > > + iommu_set_cmd_line_dma_api(); IOMMU command line is also set in other places, for example, iommu_setup() (arch/x86/kernel/pci-dma.c). Need to call this there as well? Best regards, Lu Baolu > + > iommu_def_domain_type = pt ? IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA; > return 0; > } > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hi, On 8/14/19 9:38 PM, Joerg Roedel wrote: > From: Joerg Roedel <jroedel@suse.de> > > Introduce a subsys_initcall for IOMMU code and use it to > print the default domain type at boot. > > Signed-off-by: Joerg Roedel <jroedel@suse.de> > --- > drivers/iommu/iommu.c | 30 +++++++++++++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index e1feb4061b8b..233bc22b487e 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -93,12 +93,40 @@ struct iommu_group_attribute iommu_group_attr_##_name = \ > static LIST_HEAD(iommu_device_list); > static DEFINE_SPINLOCK(iommu_device_lock); > > +/* > + * Use a function instead of an array here because the domain-type is a > + * bit-field, so an array would waste memory. > + */ > +static const char *iommu_domain_type_str(unsigned int t) > +{ > + switch (t) { > + case IOMMU_DOMAIN_BLOCKED: > + return "Blocked"; > + case IOMMU_DOMAIN_IDENTITY: > + return "Passthrough"; > + case IOMMU_DOMAIN_UNMANAGED: > + return "Unmanaged"; > + case IOMMU_DOMAIN_DMA: > + return "Translated"; > + default: > + return "Unknown"; > + } > +} Run scripts/checkpatch.pl: ERROR: switch and case should be at the same indent #28: FILE: drivers/iommu/iommu.c:102: + switch (t) { + case IOMMU_DOMAIN_BLOCKED: [...] + case IOMMU_DOMAIN_IDENTITY: [...] + case IOMMU_DOMAIN_UNMANAGED: [...] + case IOMMU_DOMAIN_DMA: [...] + default: Best regards, Lu Baolu > + > +static int __init iommu_subsys_init(void) > +{ > + pr_info("Default domain type: %s\n", > + iommu_domain_type_str(iommu_def_domain_type)); > + > + return 0; > +} > +subsys_initcall(iommu_subsys_init); > + > int iommu_device_register(struct iommu_device *iommu) > { > spin_lock(&iommu_device_lock); > list_add_tail(&iommu->list, &iommu_device_list); > spin_unlock(&iommu_device_lock); > - > return 0; > } > > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hi, On 8/14/19 9:38 PM, Joerg Roedel wrote: > From: Joerg Roedel <jroedel@suse.de> > > Set the default domain-type at runtime, not at compile-time. > This keeps default domain type setting in one place when we > have to change it at runtime. > > Signed-off-by: Joerg Roedel <jroedel@suse.de> > --- > drivers/iommu/iommu.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index 233bc22b487e..96cc7cc8ab21 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -26,11 +26,8 @@ > > 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; > -#else > -static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA; > -#endif > + > +static unsigned int iommu_def_domain_type __read_mostly; > static bool iommu_dma_strict __read_mostly = true; > static u32 iommu_cmd_line __read_mostly; > > @@ -76,7 +73,7 @@ static void iommu_set_cmd_line_dma_api(void) > iommu_cmd_line |= IOMMU_CMD_LINE_DMA_API; > } > > -static bool __maybe_unused iommu_cmd_line_dma_api(void) > +static bool iommu_cmd_line_dma_api(void) > { > return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API); > } > @@ -115,8 +112,18 @@ static const char *iommu_domain_type_str(unsigned int t) > > static int __init iommu_subsys_init(void) > { > - pr_info("Default domain type: %s\n", > - iommu_domain_type_str(iommu_def_domain_type)); > + bool cmd_line = iommu_cmd_line_dma_api(); > + > + if (!cmd_line) { > + if (IS_ENABLED(CONFIG_IOMMU_DEFAULT_PASSTHROUGH)) > + iommu_set_default_passthrough(); > + else > + iommu_set_default_translated(); This overrides kernel parameters parsed in iommu_setup(), for example, iommu=pt won't work anymore. Best regards, Lu Baolu > + } > + > + pr_info("Default domain type: %s %s\n", > + iommu_domain_type_str(iommu_def_domain_type), > + cmd_line ? "(set via kernel command line)" : ""); > > return 0; > } > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Hey Lu Baolu, thanks for your review! On Thu, Aug 15, 2019 at 01:01:57PM +0800, Lu Baolu wrote: > > +#define IOMMU_CMD_LINE_DMA_API (1 << 0) > > Prefer BIT() micro? Yes, I'll change that. > > + iommu_set_cmd_line_dma_api(); > > IOMMU command line is also set in other places, for example, > iommu_setup() (arch/x86/kernel/pci-dma.c). Need to call this there as > well? You are right, I'll better add a 'bool cmd_line' parameter to the iommu_set_default_*() functions and tell the IOMMU core this way. That will also fix iommu=pt/nopt. Thanks, Joerg _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu