[PATCH] iommu/iova: Add support for IOVA max alignment tuning

[PATCH] iommu/iova: Add support for IOVA max alignment tuning

[Georgi Djakov]

IOVAs are aligned to the smallest PAGE_SIZE order, where the requested
IOVA can fit. But this might not work for all use-cases. It can cause
IOVA fragmentation in some multimedia and 8K video use-cases that may
require larger buffers to be allocated and mapped.

When the above allocation pattern is used with the current alignment
scheme, the IOVA space could be quickly exhausted for 32bit devices.

In order to get better IOVA space utilization and reduce fragmentation,
a new kernel command line parameter is introduced to make the alignment
limit configurable by the user during boot.

Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  8 ++++++++
 drivers/iommu/iova.c                            | 26 ++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ad94a2aa9819..630246dc691f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2056,6 +2056,14 @@
 			  forcing Dual Address Cycle for PCI cards supporting
 			  greater than 32-bit addressing.
 
+	iommu.max_align_shift=
+			[ARM64, X86] Limit the alignment of IOVAs to a maximum
+			PAGE_SIZE order. Larger IOVAs will be aligned to this
+			specified order. The order is expressed as a power of
+			two multiplied by the PAGE_SIZE.
+			Format: { "4" | "5" | "6" | "7" | "8" | "9" }
+			Default: 9
+
 	iommu.strict=	[ARM64, X86] Configure TLB invalidation behaviour
 			Format: { "0" | "1" }
 			0 - Lazy mode.
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 9e8bc802ac05..5a8c86871735 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -15,6 +15,9 @@
 /* The anchor node sits above the top of the usable address space */
 #define IOVA_ANCHOR	~0UL
 
+#define IOMMU_DEFAULT_IOVA_MAX_ALIGN_SHIFT	9
+static unsigned long iommu_max_align_shift __read_mostly = IOMMU_DEFAULT_IOVA_MAX_ALIGN_SHIFT;
+
 static bool iova_rcache_insert(struct iova_domain *iovad,
 			       unsigned long pfn,
 			       unsigned long size);
@@ -27,6 +30,27 @@ static void free_iova_rcaches(struct iova_domain *iovad);
 static void fq_destroy_all_entries(struct iova_domain *iovad);
 static void fq_flush_timeout(struct timer_list *t);
 
+static unsigned long limit_align_shift(struct iova_domain *iovad, unsigned long shift)
+{
+	unsigned long max_align_shift;
+
+	max_align_shift = iommu_max_align_shift + PAGE_SHIFT - iova_shift(iovad);
+	return min_t(unsigned long, max_align_shift, shift);
+}
+
+static int __init iommu_set_def_max_align_shift(char *str)
+{
+	unsigned long max_align_shift;
+
+	int ret = kstrtoul(str, 10, &max_align_shift);
+
+	if (!ret)
+		iommu_max_align_shift = max_align_shift;
+
+	return 0;
+}
+early_param("iommu.max_align_shift", iommu_set_def_max_align_shift);
+
 static int iova_cpuhp_dead(unsigned int cpu, struct hlist_node *node)
 {
 	struct iova_domain *iovad;
@@ -242,7 +266,7 @@ static int __alloc_and_insert_iova_range(struct iova_domain *iovad,
 	unsigned long high_pfn = limit_pfn, low_pfn = iovad->start_pfn;
 
 	if (size_aligned)
-		align_mask <<= fls_long(size - 1);
+		align_mask <<= limit_align_shift(iovad, fls_long(size - 1));
 
 	/* Walk the tree backwards */
 	spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/iova: Add support for IOVA max alignment tuning

[Robin Murphy]

On 2021-10-13 19:11, Georgi Djakov wrote:
> IOVAs are aligned to the smallest PAGE_SIZE order, where the requested
> IOVA can fit. But this might not work for all use-cases. It can cause
> IOVA fragmentation in some multimedia and 8K video use-cases that may
> require larger buffers to be allocated and mapped.
> 
> When the above allocation pattern is used with the current alignment
> scheme, the IOVA space could be quickly exhausted for 32bit devices.
> 
> In order to get better IOVA space utilization and reduce fragmentation,
> a new kernel command line parameter is introduced to make the alignment
> limit configurable by the user during boot.
> 
> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com>
> ---
>   Documentation/admin-guide/kernel-parameters.txt |  8 ++++++++
>   drivers/iommu/iova.c                            | 26 ++++++++++++++++++++++++-

I see no good reason for the IOVA layer to lie to its callers. If they 
don't need an aligned IOVA, they shouldn't ask for one in the first 
place. If callers still need some intermediate degree of alignment then 
the IOVA API might want to grow something more expressive than "bool 
size_aligned", but it should still be the callers' responsibility to 
pass an appropriate value.

Thanks,
Robin.

>   2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index ad94a2aa9819..630246dc691f 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2056,6 +2056,14 @@
>   			  forcing Dual Address Cycle for PCI cards supporting
>   			  greater than 32-bit addressing.
>   
> +	iommu.max_align_shift=
> +			[ARM64, X86] Limit the alignment of IOVAs to a maximum
> +			PAGE_SIZE order. Larger IOVAs will be aligned to this
> +			specified order. The order is expressed as a power of
> +			two multiplied by the PAGE_SIZE.
> +			Format: { "4" | "5" | "6" | "7" | "8" | "9" }
> +			Default: 9
> +
>   	iommu.strict=	[ARM64, X86] Configure TLB invalidation behaviour
>   			Format: { "0" | "1" }
>   			0 - Lazy mode.
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index 9e8bc802ac05..5a8c86871735 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -15,6 +15,9 @@
>   /* The anchor node sits above the top of the usable address space */
>   #define IOVA_ANCHOR	~0UL
>   
> +#define IOMMU_DEFAULT_IOVA_MAX_ALIGN_SHIFT	9
> +static unsigned long iommu_max_align_shift __read_mostly = IOMMU_DEFAULT_IOVA_MAX_ALIGN_SHIFT;
> +
>   static bool iova_rcache_insert(struct iova_domain *iovad,
>   			       unsigned long pfn,
>   			       unsigned long size);
> @@ -27,6 +30,27 @@ static void free_iova_rcaches(struct iova_domain *iovad);
>   static void fq_destroy_all_entries(struct iova_domain *iovad);
>   static void fq_flush_timeout(struct timer_list *t);
>   
> +static unsigned long limit_align_shift(struct iova_domain *iovad, unsigned long shift)
> +{
> +	unsigned long max_align_shift;
> +
> +	max_align_shift = iommu_max_align_shift + PAGE_SHIFT - iova_shift(iovad);
> +	return min_t(unsigned long, max_align_shift, shift);
> +}
> +
> +static int __init iommu_set_def_max_align_shift(char *str)
> +{
> +	unsigned long max_align_shift;
> +
> +	int ret = kstrtoul(str, 10, &max_align_shift);
> +
> +	if (!ret)
> +		iommu_max_align_shift = max_align_shift;
> +
> +	return 0;
> +}
> +early_param("iommu.max_align_shift", iommu_set_def_max_align_shift);
> +
>   static int iova_cpuhp_dead(unsigned int cpu, struct hlist_node *node)
>   {
>   	struct iova_domain *iovad;
> @@ -242,7 +266,7 @@ static int __alloc_and_insert_iova_range(struct iova_domain *iovad,
>   	unsigned long high_pfn = limit_pfn, low_pfn = iovad->start_pfn;
>   
>   	if (size_aligned)
> -		align_mask <<= fls_long(size - 1);
> +		align_mask <<= limit_align_shift(iovad, fls_long(size - 1));
>   
>   	/* Walk the tree backwards */
>   	spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/iova: Add support for IOVA max alignment tuning

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2990 bytes --]

Hi Georgi,

I love your patch! Yet something to improve:

[auto build test ERROR on joro-iommu/next]
[also build test ERROR on hnaz-mm/master linus/master v5.15-rc5 next-20211013]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Georgi-Djakov/iommu-iova-Add-support-for-IOVA-max-alignment-tuning/20211014-021248
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: i386-randconfig-a003-20211013 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project b6a8c695542b2987eb9a203d5663a0740cb4725f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/dc4cf3ea2ffff2a4c30a8a2c395e830ea37dd819
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Georgi-Djakov/iommu-iova-Add-support-for-IOVA-max-alignment-tuning/20211014-021248
        git checkout dc4cf3ea2ffff2a4c30a8a2c395e830ea37dd819
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/iommu/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

>> drivers/iommu/iova.c:52:13: error: expected parameter declarator
   early_param("iommu.max_align_shift", iommu_set_def_max_align_shift);
               ^
>> drivers/iommu/iova.c:52:13: error: expected ')'
   drivers/iommu/iova.c:52:12: note: to match this '('
   early_param("iommu.max_align_shift", iommu_set_def_max_align_shift);
              ^
>> drivers/iommu/iova.c:52:1: warning: declaration specifier missing, defaulting to 'int'
   early_param("iommu.max_align_shift", iommu_set_def_max_align_shift);
   ^
   int
>> drivers/iommu/iova.c:52:12: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
   early_param("iommu.max_align_shift", iommu_set_def_max_align_shift);
              ^
                                                                     void
   1 warning and 3 errors generated.


vim +52 drivers/iommu/iova.c

    40	
    41	static int __init iommu_set_def_max_align_shift(char *str)
    42	{
    43		unsigned long max_align_shift;
    44	
    45		int ret = kstrtoul(str, 10, &max_align_shift);
    46	
    47		if (!ret)
    48			iommu_max_align_shift = max_align_shift;
    49	
    50		return 0;
    51	}
  > 52	early_param("iommu.max_align_shift", iommu_set_def_max_align_shift);
    53	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36730 bytes --]

[-- Attachment #3: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu