linux-kernel - Re: [PATCH v7 24/27] x86: Add swiotlb force off support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121222023741.GF3468@phenom.dumpdata.com>
Date:	Fri, 21 Dec 2012 21:37:41 -0500
From:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To:	Yinghai Lu <yinghai@...nel.org>
Cc:	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...e.hu>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 24/27] x86: Add swiotlb force off support

On Mon, Dec 17, 2012 at 11:15:56PM -0800, Yinghai Lu wrote:
> So use could disable swiotlb from command line, even swiotlb support
> is compiled in.  Just like we have intel_iommu=on and intel_iommu=off.

You really need to spell out why this is useful.

> 
> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
> ---
>  Documentation/kernel-parameters.txt |    7 +++++++
>  arch/x86/kernel/pci-swiotlb.c       |   10 +++++-----
>  drivers/iommu/amd_iommu.c           |    1 +
>  include/linux/swiotlb.h             |    1 +
>  lib/swiotlb.c                       |    5 ++++-
>  5 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index ea8e5b4..2b37020 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2835,6 +2835,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  
>  	swiotlb=	[IA-64] Number of I/O TLB slabs
>  
> +	swiotlb=[force|off|on] [KNL] disable or enable swiotlb.
> +		force
> +		on
> +			Enable swiotlb.
> +		off
> +			Disable swiotlb.
> +
>  	switches=	[HW,M68k]
>  
>  	sysfs.deprecated=0|1 [KNL]
> diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c
> index 6f93eb7..80afd3b 100644
> --- a/arch/x86/kernel/pci-swiotlb.c
> +++ b/arch/x86/kernel/pci-swiotlb.c
> @@ -58,12 +58,12 @@ static struct dma_map_ops swiotlb_dma_ops = {
>   */
>  int __init pci_swiotlb_detect_override(void)
>  {
> -	int use_swiotlb = swiotlb | swiotlb_force;
> -
>  	if (swiotlb_force)
>  		swiotlb = 1;
> +	else if (swiotlb_force_off)
> +		swiotlb = 0;
>  
> -	return use_swiotlb;
> +	return swiotlb;
>  }
>  IOMMU_INIT_FINISH(pci_swiotlb_detect_override,
>  		  pci_xen_swiotlb_detect,
> @@ -76,9 +76,9 @@ IOMMU_INIT_FINISH(pci_swiotlb_detect_override,
>   */
>  int __init pci_swiotlb_detect_4gb(void)
>  {
> -	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
> +	/* don't initialize swiotlb if iommu=off (no_iommu=1) or force off */
>  #ifdef CONFIG_X86_64
> -	if (!no_iommu && max_pfn > MAX_DMA32_PFN)
> +	if (!no_iommu && !swiotlb_force_off && max_pfn > MAX_DMA32_PFN)
>  		swiotlb = 1;
>  #endif
>  	return swiotlb;
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 55074cb..4f370d3 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -3082,6 +3082,7 @@ int __init amd_iommu_init_dma_ops(void)
>  	unhandled = device_dma_ops_init();
>  	if (unhandled && max_pfn > MAX_DMA32_PFN) {
>  		/* There are unhandled devices - initialize swiotlb for them */
> +		WARN(swiotlb_force_off, "Please remove swiotlb=off\n");
>  		swiotlb = 1;
>  	}
>  
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 1d2506f..dc43968 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -8,6 +8,7 @@ struct dma_attrs;
>  struct scatterlist;
>  
>  extern int swiotlb_force;
> +extern int swiotlb_force_off;

I think the usage of the swiotlb_force and making it a flag is a better way.
>  
>  /*
>   * Maximum allowable number of contiguous slabs to map,
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index 958322e..3a0ec46 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -51,6 +51,7 @@
>  #define IO_TLB_MIN_SLABS ((1<<20) >> IO_TLB_SHIFT)
>  
>  int swiotlb_force;
> +int swiotlb_force_off;
>  
>  /*
>   * Used to do a quick range check in swiotlb_tbl_unmap_single and
> @@ -102,8 +103,10 @@ setup_io_tlb_npages(char *str)
>  	}
>  	if (*str == ',')
>  		++str;
> -	if (!strcmp(str, "force"))
> +	if (!strcmp(str, "force") || !strcmp(str, "on"))
>  		swiotlb_force = 1;
> +	if (!strcmp(str, "off"))
> +		swiotlb_force_off = 1;
>  
>  	return 1;
>  }
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/