Re: [merged mm-stable] resource-kunit-add-test-case-for-region_intersects.patch removed from -mm tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 17, 2024 at 01:08:49AM -0700, Andrew Morton wrote:
> 
> The quilt patch titled
>      Subject: resource, kunit: add test case for region_intersects()
> has been removed from the -mm tree.  Its filename was
>      resource-kunit-add-test-case-for-region_intersects.patch
> 
> This patch was dropped because it was merged into the mm-stable branch
> of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> 
> ------------------------------------------------------
> From: Huang Ying <ying.huang@xxxxxxxxx>
> Subject: resource, kunit: add test case for region_intersects()
> Date: Fri, 6 Sep 2024 11:07:13 +0800
> 
> Patch series "resource: Fix region_intersects() vs
> add_memory_driver_managed()", v3.
> 
> The patchset fixes a bug of region_intersects() for systems with CXL
> memory.  The details of the bug can be found in [1/3].  To avoid similar
> bugs in the future.  A kunit test case for region_intersects() is added in
> [3/3].  [2/3] is a preparation patch for [3/3].
> 
> 
> This patch (of 3):
> 
> region_intersects() is important because it's used for /dev/mem permission
> checking.  To avoid possible bug of region_intersects() in the future, a
> kunit test case for region_intersects() is added.
> 
> Link: https://lkml.kernel.org/r/20240906030713.204292-1-ying.huang@xxxxxxxxx
> Link: https://lkml.kernel.org/r/20240906030713.204292-4-ying.huang@xxxxxxxxx
> Signed-off-by: "Huang, Ying" <ying.huang@xxxxxxxxx>
> Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
> Cc: David Hildenbrand <david@xxxxxxxxxx>
> Cc: Davidlohr Bueso <dave@xxxxxxxxxxxx>
> Cc: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>
> Cc: Dave Jiang <dave.jiang@xxxxxxxxx>
> Cc: Alison Schofield <alison.schofield@xxxxxxxxx>
> Cc: Vishal Verma <vishal.l.verma@xxxxxxxxx>
> Cc: Ira Weiny <ira.weiny@xxxxxxxxx>
> Cc: Alistair Popple <apopple@xxxxxxxxxx>
> Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Cc: Baoquan He <bhe@xxxxxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> ---
> 
>  kernel/resource.c       |   20 +++--
>  kernel/resource_kunit.c |  143 ++++++++++++++++++++++++++++++++++++++
>  lib/Kconfig.debug       |    1 
>  3 files changed, 158 insertions(+), 6 deletions(-)
> 
> --- a/kernel/resource.c~resource-kunit-add-test-case-for-region_intersects
> +++ a/kernel/resource.c
> @@ -1817,7 +1817,17 @@ EXPORT_SYMBOL(resource_list_free);
>  #ifdef CONFIG_GET_FREE_REGION
>  #define GFR_DESCENDING		(1UL << 0)
>  #define GFR_REQUEST_REGION	(1UL << 1)
> -#define GFR_DEFAULT_ALIGN (1UL << PA_SECTION_SHIFT)
> +#ifdef PA_SECTION_SHIFT
> +#define GFR_DEFAULT_ALIGN	(1UL << PA_SECTION_SHIFT)
> +#else
> +#define GFR_DEFAULT_ALIGN	PAGE_SIZE
> +#endif
> +
> +#ifdef MAX_PHYSMEM_BITS
> +#define MAX_PHYS_ADDR		((1ULL << MAX_PHYSMEM_BITS) - 1)
> +#else
> +#define MAX_PHYS_ADDR		(-1ULL)
> +#endif
>  
>  static resource_size_t gfr_start(struct resource *base, resource_size_t size,
>  				 resource_size_t align, unsigned long flags)
> @@ -1825,8 +1835,7 @@ static resource_size_t gfr_start(struct
>  	if (flags & GFR_DESCENDING) {
>  		resource_size_t end;
>  
> -		end = min_t(resource_size_t, base->end,
> -			    (1ULL << MAX_PHYSMEM_BITS) - 1);
> +		end = min_t(resource_size_t, base->end, MAX_PHYS_ADDR);
>  		return end - size + 1;
>  	}
>  
> @@ -1843,8 +1852,7 @@ static bool gfr_continue(struct resource
>  	 * @size did not wrap 0.
>  	 */
>  	return addr > addr - size &&
> -	       addr <= min_t(resource_size_t, base->end,
> -			     (1ULL << MAX_PHYSMEM_BITS) - 1);
> +		addr <= min_t(resource_size_t, base->end, MAX_PHYS_ADDR);
>  }
>  
>  static resource_size_t gfr_next(resource_size_t addr, resource_size_t size,
> @@ -2005,7 +2013,7 @@ struct resource *alloc_free_mem_region(s
>  	return get_free_mem_region(NULL, base, size, align, name,
>  				   IORES_DESC_NONE, flags);
>  }
> -EXPORT_SYMBOL_NS_GPL(alloc_free_mem_region, CXL);
> +EXPORT_SYMBOL_GPL(alloc_free_mem_region);
>  #endif /* CONFIG_GET_FREE_REGION */
>  
>  static int __init strict_iomem(char *str)
> --- a/kernel/resource_kunit.c~resource-kunit-add-test-case-for-region_intersects
> +++ a/kernel/resource_kunit.c
> @@ -7,6 +7,8 @@
>  #include <linux/ioport.h>
>  #include <linux/kernel.h>
>  #include <linux/string.h>
> +#include <linux/sizes.h>
> +#include <linux/mm.h>
>  
>  #define R0_START	0x0000
>  #define R0_END		0xffff
> @@ -137,9 +139,150 @@ static void resource_test_intersection(s
>  	} while (++i < ARRAY_SIZE(results_for_intersection));
>  }
>  
> +/*
> + * The test resource tree for region_intersects() test:
> + *
> + * BASE-BASE+1M-1 : Test System RAM 0
> + *		  # hole 0 (BASE+1M-BASE+2M)
> + * BASE+2M-BASE+3M-1 : Test CXL Window 0
> + * BASE+3M-BASE+4M-1 : Test System RAM 1
> + * BASE+4M-BASE+7M-1 : Test CXL Window 1
> + *   BASE+4M-BASE+5M-1 : Test System RAM 2
> + *     BASE+4M+128K-BASE+4M+256K-1: Test Code
> + *   BASE+5M-BASE+6M-1 : Test System RAM 3
> + */
> +#define RES_TEST_RAM0_OFFSET	0
> +#define RES_TEST_RAM0_SIZE	SZ_1M
> +#define RES_TEST_HOLE0_OFFSET	(RES_TEST_RAM0_OFFSET + RES_TEST_RAM0_SIZE)
> +#define RES_TEST_HOLE0_SIZE	SZ_1M
> +#define RES_TEST_WIN0_OFFSET	(RES_TEST_HOLE0_OFFSET + RES_TEST_HOLE0_SIZE)
> +#define RES_TEST_WIN0_SIZE	SZ_1M
> +#define RES_TEST_RAM1_OFFSET	(RES_TEST_WIN0_OFFSET + RES_TEST_WIN0_SIZE)
> +#define RES_TEST_RAM1_SIZE	SZ_1M
> +#define RES_TEST_WIN1_OFFSET	(RES_TEST_RAM1_OFFSET + RES_TEST_RAM1_SIZE)
> +#define RES_TEST_WIN1_SIZE	(SZ_1M * 3)
> +#define RES_TEST_RAM2_OFFSET	RES_TEST_WIN1_OFFSET
> +#define RES_TEST_RAM2_SIZE	SZ_1M
> +#define RES_TEST_CODE_OFFSET	(RES_TEST_RAM2_OFFSET + SZ_128K)
> +#define RES_TEST_CODE_SIZE	SZ_128K
> +#define RES_TEST_RAM3_OFFSET	(RES_TEST_RAM2_OFFSET + RES_TEST_RAM2_SIZE)
> +#define RES_TEST_RAM3_SIZE	SZ_1M
> +#define RES_TEST_TOTAL_SIZE	((RES_TEST_WIN1_OFFSET + RES_TEST_WIN1_SIZE))
> +
> +static void remove_free_resource(void *ctx)
> +{
> +	struct resource *res = (struct resource *)ctx;
> +
> +	remove_resource(res);
> +	kfree(res);
> +}
> +
> +static void resource_test_request_region(struct kunit *test, struct resource *parent,
> +					 resource_size_t start, resource_size_t size,
> +					 const char *name, unsigned long flags)
> +{
> +	struct resource *res;
> +
> +	res = __request_region(parent, start, size, name, flags);
> +	KUNIT_ASSERT_NOT_NULL(test, res);
> +	kunit_add_action_or_reset(test, remove_free_resource, res);
> +}
> +
> +static void resource_test_insert_resource(struct kunit *test, struct resource *parent,
> +					  resource_size_t start, resource_size_t size,
> +					  const char *name, unsigned long flags)
> +{
> +	struct resource *res;
> +
> +	res = kzalloc(sizeof(*res), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_NULL(test, res);
> +
> +	res->name = name;
> +	res->start = start;
> +	res->end = start + size - 1;
> +	res->flags = flags;
> +	if (insert_resource(parent, res)) {
> +		kfree(res);
> +		KUNIT_FAIL_AND_ABORT(test, "Fail to insert resource %pR\n", res);
> +	}
> +
> +	kunit_add_action_or_reset(test, remove_free_resource, res);
> +}
> +
> +static void resource_test_region_intersects(struct kunit *test)
> +{
> +	unsigned long flags =  IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> +	struct resource *parent;
> +	resource_size_t start;
> +
> +	/* Find an iomem_resource hole to hold test resources */
> +	parent = alloc_free_mem_region(&iomem_resource, RES_TEST_TOTAL_SIZE, SZ_1M,
> +				       "test resources");
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent);
> +	start = parent->start;
> +	kunit_add_action_or_reset(test, remove_free_resource, parent);
> +
> +	resource_test_request_region(test, parent, start + RES_TEST_RAM0_OFFSET,
> +				     RES_TEST_RAM0_SIZE, "Test System RAM 0", flags);
> +	resource_test_insert_resource(test, parent, start + RES_TEST_WIN0_OFFSET,
> +				      RES_TEST_WIN0_SIZE, "Test CXL Window 0",
> +				      IORESOURCE_MEM);
> +	resource_test_request_region(test, parent, start + RES_TEST_RAM1_OFFSET,
> +				     RES_TEST_RAM1_SIZE, "Test System RAM 1", flags);
> +	resource_test_insert_resource(test, parent, start + RES_TEST_WIN1_OFFSET,
> +				      RES_TEST_WIN1_SIZE, "Test CXL Window 1",
> +				      IORESOURCE_MEM);
> +	resource_test_request_region(test, parent, start + RES_TEST_RAM2_OFFSET,
> +				     RES_TEST_RAM2_SIZE, "Test System RAM 2", flags);
> +	resource_test_insert_resource(test, parent, start + RES_TEST_CODE_OFFSET,
> +				      RES_TEST_CODE_SIZE, "Test Code", flags);
> +	resource_test_request_region(test, parent, start + RES_TEST_RAM3_OFFSET,
> +				     RES_TEST_RAM3_SIZE, "Test System RAM 3", flags);
> +	kunit_release_action(test, remove_free_resource, parent);
> +
> +	KUNIT_EXPECT_EQ(test, REGION_INTERSECTS,
> +			region_intersects(start + RES_TEST_RAM0_OFFSET, PAGE_SIZE,
> +					  IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE));
> +	KUNIT_EXPECT_EQ(test, REGION_INTERSECTS,
> +			region_intersects(start + RES_TEST_RAM0_OFFSET +
> +					  RES_TEST_RAM0_SIZE - PAGE_SIZE, 2 * PAGE_SIZE,
> +					  IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE));
> +	KUNIT_EXPECT_EQ(test, REGION_DISJOINT,
> +			region_intersects(start + RES_TEST_HOLE0_OFFSET, PAGE_SIZE,
> +					  IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE));
> +	KUNIT_EXPECT_EQ(test, REGION_DISJOINT,
> +			region_intersects(start + RES_TEST_HOLE0_OFFSET +
> +					  RES_TEST_HOLE0_SIZE - PAGE_SIZE, 2 * PAGE_SIZE,
> +					  IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE));
> +	KUNIT_EXPECT_EQ(test, REGION_MIXED,
> +			region_intersects(start + RES_TEST_WIN0_OFFSET +
> +					  RES_TEST_WIN0_SIZE - PAGE_SIZE, 2 * PAGE_SIZE,
> +					  IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE));
> +	KUNIT_EXPECT_EQ(test, REGION_INTERSECTS,
> +			region_intersects(start + RES_TEST_RAM1_OFFSET +
> +					  RES_TEST_RAM1_SIZE - PAGE_SIZE, 2 * PAGE_SIZE,
> +					  IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE));
> +	KUNIT_EXPECT_EQ(test, REGION_INTERSECTS,
> +			region_intersects(start + RES_TEST_RAM2_OFFSET +
> +					  RES_TEST_RAM2_SIZE - PAGE_SIZE, 2 * PAGE_SIZE,
> +					  IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE));
> +	KUNIT_EXPECT_EQ(test, REGION_INTERSECTS,
> +			region_intersects(start + RES_TEST_CODE_OFFSET, PAGE_SIZE,
> +					  IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE));
> +	KUNIT_EXPECT_EQ(test, REGION_INTERSECTS,
> +			region_intersects(start + RES_TEST_RAM2_OFFSET,
> +					  RES_TEST_RAM2_SIZE + PAGE_SIZE,
> +					  IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE));
> +	KUNIT_EXPECT_EQ(test, REGION_MIXED,
> +			region_intersects(start + RES_TEST_RAM3_OFFSET,
> +					  RES_TEST_RAM3_SIZE + PAGE_SIZE,
> +					  IORESOURCE_SYSTEM_RAM, IORES_DESC_NONE));
> +}
> +
>  static struct kunit_case resource_test_cases[] = {
>  	KUNIT_CASE(resource_test_union),
>  	KUNIT_CASE(resource_test_intersection),
> +	KUNIT_CASE(resource_test_region_intersects),
>  	{}
>  };
>  
> --- a/lib/Kconfig.debug~resource-kunit-add-test-case-for-region_intersects
> +++ a/lib/Kconfig.debug
> @@ -2616,6 +2616,7 @@ config RESOURCE_KUNIT_TEST
>  	tristate "KUnit test for resource API" if !KUNIT_ALL_TESTS
>  	depends on KUNIT
>  	default KUNIT_ALL_TESTS
> +	select GET_FREE_REGION
>  	help
>  	  This builds the resource API unit test.
>  	  Tests the logic of API provided by resource.c and ioport.h.
> _
> 
> Patches currently in -mm which might be from ying.huang@xxxxxxxxx are
> 
> 

This change results in the following Kconfig warning and compiler
warnings (or errors with CONFIG_WERROR):

  $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- allmodconfig kernel/resource.o
  WARNING: unmet direct dependencies detected for GET_FREE_REGION
    Depends on [n]: SPARSEMEM [=n]
    Selected by [m]:
    - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]
  In file included from include/linux/ioport.h:15,
                   from kernel/resource.c:15:
  kernel/resource.c: In function 'gfr_start':
  include/linux/minmax.h:93:37: error: conversion from 'long long unsigned int' to 'resource_size_t' {aka 'unsigned int'} changes value from '18446744073709551615' to '4294967295' [-Werror=overflow]
     93 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
        |                                     ^
  include/linux/minmax.h:96:9: note: in expansion of macro '__cmp_once_unique'
     96 |         __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
        |         ^~~~~~~~~~~~~~~~~
  include/linux/minmax.h:213:27: note: in expansion of macro '__cmp_once'
    213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
        |                           ^~~~~~~~~~
  kernel/resource.c:1874:23: note: in expansion of macro 'min_t'
   1874 |                 end = min_t(resource_size_t, base->end, PHYSMEM_END);
        |                       ^~~~~
  kernel/resource.c: In function 'gfr_continue':
  include/linux/minmax.h:93:37: error: conversion from 'long long unsigned int' to 'resource_size_t' {aka 'unsigned int'} changes value from '18446744073709551615' to '4294967295' [-Werror=overflow]
     93 |         ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
        |                                     ^
  include/linux/minmax.h:96:9: note: in expansion of macro '__cmp_once_unique'
     96 |         __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
        |         ^~~~~~~~~~~~~~~~~
  include/linux/minmax.h:213:27: note: in expansion of macro '__cmp_once'
    213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
        |                           ^~~~~~~~~~
  kernel/resource.c:1891:24: note: in expansion of macro 'min_t'
   1891 |                addr <= min_t(resource_size_t, base->end, PHYSMEM_END);
        |                        ^~~~~
  cc1: all warnings being treated as errors

Arnd had a patch in another bug report to resolve the Kconfig warning,
which happens to resolve the compiler warning for allmodconfig, but I
suspect that a configuration with SPARSEMEM and !ARM_LPAE could still
trigger the compiler warnings.

https://lore.kernel.org/aaee4ddb-68a8-42ae-bb68-11ef991ada1c@xxxxxxxxxxxxxxxx/

Cheers,
Nathan




[Index of Archives]     [Kernel Archive]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]

  Powered by Linux