Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] swiotlb: Allow swiotlb to live at pre-defined address
To:     Alexander Graf <graf@amazon.com>, iommu@lists.linux-foundation.org
Cc:     Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, x86@kernel.org,
        Christoph Hellwig <hch@lst.de>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        Robin Murphy <robin.murphy@arm.com>, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>,
        dwmw@amazon.com, benh@amazon.com, alcioa@amazon.com,
        aggh@amazon.com, aagch@amazon.com, dhr@amazon.com
References: <20200326162922.27085-1-graf@amazon.com>
From:   Jan Kiszka <jan.kiszka@web.de>
Message-ID: <9ff68753-20d1-62b1-6250-91ed4beb1bde@web.de>
Date:   Fri, 27 Mar 2020 10:58:43 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <20200326162922.27085-1-graf@amazon.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 26.03.20 17:29, Alexander Graf wrote:
> The swiotlb is a very convenient fallback mechanism for bounce buffering=
 of
> DMAable data. It is usually used for the compatibility case where device=
s
> can only DMA to a "low region".
>
> However, in some scenarios this "low region" may be bound even more
> heavily. For example, there are embedded system where only an SRAM regio=
n
> is shared between device and CPU. There are also heterogeneous computing
> scenarios where only a subset of RAM is cache coherent between the
> components of the system. There are partitioning hypervisors, where
> a "control VM" that implements device emulation has limited view into a
> partition's memory for DMA capabilities due to safety concerns.
>
> This patch adds a command line driven mechanism to move all DMA memory i=
nto
> a predefined shared memory region which may or may not be part of the
> physical address layout of the Operating System.
>
> Ideally, the typical path to set this configuration would be through Dev=
ice
> Tree or ACPI, but neither of the two mechanisms is standardized yet. Als=
o,
> in the x86 MicroVM use case, we have neither ACPI nor Device Tree, but
> instead configure the system purely through kernel command line options.
>
> I'm sure other people will find the functionality useful going forward
> though and extend it to be triggered by DT/ACPI in the future.
>
> Signed-off-by: Alexander Graf <graf@amazon.com>
> ---
>   Documentation/admin-guide/kernel-parameters.txt |  3 +-
>   Documentation/x86/x86_64/boot-options.rst       |  4 ++-
>   kernel/dma/swiotlb.c                            | 46 +++++++++++++++++=
++++++--
>   3 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documenta=
tion/admin-guide/kernel-parameters.txt
> index c07815d230bc..d085d55c3cbe 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4785,11 +4785,12 @@
>   			it if 0 is given (See Documentation/admin-guide/cgroup-v1/memory.rs=
t)
>
>   	swiotlb=3D	[ARM,IA-64,PPC,MIPS,X86]
> -			Format: { <int> | force | noforce }
> +			Format: { <int> | force | noforce | addr=3D<phys addr> }
>   			<int> -- Number of I/O TLB slabs
>   			force -- force using of bounce buffers even if they
>   			         wouldn't be automatically used by the kernel
>   			noforce -- Never use bounce buffers (for debugging)
> +			addr=3D<phys addr> -- Try to allocate SWIOTLB at defined address
>
>   	switches=3D	[HW,M68k]
>
> diff --git a/Documentation/x86/x86_64/boot-options.rst b/Documentation/x=
86/x86_64/boot-options.rst
> index 2b98efb5ba7f..ca46c57b68c9 100644
> --- a/Documentation/x86/x86_64/boot-options.rst
> +++ b/Documentation/x86/x86_64/boot-options.rst
> @@ -297,11 +297,13 @@ iommu options only relevant to the AMD GART hardwa=
re IOMMU:
>   iommu options only relevant to the software bounce buffering (SWIOTLB)=
 IOMMU
>   implementation:
>
> -    swiotlb=3D<pages>[,force]
> +    swiotlb=3D<pages>[,force][,addr=3D<phys addr>]
>         <pages>
>           Prereserve that many 128K pages for the software IO bounce buf=
fering.
>         force
>           Force all IO through the software TLB.
> +      addr=3D<phys addr>
> +        Try to allocate SWIOTLB at defined address
>
>   Settings for the IBM Calgary hardware IOMMU currently found in IBM
>   pSeries and xSeries machines
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index c19379fabd20..83da0caa2f93 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -46,6 +46,7 @@
>   #include <linux/init.h>
>   #include <linux/memblock.h>
>   #include <linux/iommu-helper.h>
> +#include <linux/io.h>
>
>   #define CREATE_TRACE_POINTS
>   #include <trace/events/swiotlb.h>
> @@ -102,6 +103,12 @@ unsigned int max_segment;
>   #define INVALID_PHYS_ADDR (~(phys_addr_t)0)
>   static phys_addr_t *io_tlb_orig_addr;
>
> +/*
> + * The TLB phys addr may be defined on the command line. Store it here =
if it is.
> + */
> +static phys_addr_t io_tlb_addr =3D INVALID_PHYS_ADDR;
> +
> +
>   /*
>    * Protect the above data structures in the map and unmap calls
>    */
> @@ -119,11 +126,23 @@ setup_io_tlb_npages(char *str)
>   	}
>   	if (*str =3D=3D ',')
>   		++str;
> -	if (!strcmp(str, "force")) {
> +	if (!strncmp(str, "force", 5)) {
>   		swiotlb_force =3D SWIOTLB_FORCE;
> -	} else if (!strcmp(str, "noforce")) {
> +		str +=3D 5;
> +	} else if (!strncmp(str, "noforce", 7)) {
>   		swiotlb_force =3D SWIOTLB_NO_FORCE;
>   		io_tlb_nslabs =3D 1;
> +		str +=3D 7;
> +	}
> +
> +	if (*str =3D=3D ',')
> +		++str;
> +	if (!strncmp(str, "addr=3D", 5)) {
> +		char *addrstr =3D str + 5;
> +
> +		io_tlb_addr =3D kstrtoul(addrstr, 0, &str);
> +		if (addrstr =3D=3D str)
> +			io_tlb_addr =3D INVALID_PHYS_ADDR;
>   	}
>
>   	return 0;
> @@ -239,6 +258,25 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigne=
d long nslabs, int verbose)
>   	return 0;
>   }
>
> +static int __init swiotlb_init_io(int verbose, unsigned long bytes)
> +{
> +	unsigned __iomem char *vstart;
> +
> +	if (io_tlb_addr =3D=3D INVALID_PHYS_ADDR)
> +		return -EINVAL;
> +
> +	vstart =3D memremap(io_tlb_addr, bytes, MEMREMAP_WB);
> +	if (!vstart)
> +		return -EINVAL;
> +
> +	if (swiotlb_init_with_tbl(vstart, io_tlb_nslabs, verbose)) {
> +		memunmap(vstart);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>   /*
>    * Statically reserve bounce buffer space and initialize bounce buffer=
 data
>    * structures for the software IO TLB used to implement the DMA API.
> @@ -257,6 +295,10 @@ swiotlb_init(int verbose)
>
>   	bytes =3D io_tlb_nslabs << IO_TLB_SHIFT;
>
> +	/* Map IO TLB from device memory */
> +	if (!swiotlb_init_io(verbose, bytes))
> +		return;
> +
>   	/* Get IO TLB memory from the low pages */
>   	vstart =3D memblock_alloc_low(PAGE_ALIGN(bytes), PAGE_SIZE);
>   	if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, verbose))
>

To make this useful also for shared-memory based devices, it should not
only have a command-line independent interface. Multi-instance support
would be needed so that you can attach swiotlb with individual address
ranges to devices that need it and leave it alone for other that do not
(e.g. passed-through guest devices).

Jan