linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ACPI: ioremap: avoid redundant rounding to OS page size
@ 2020-08-17 12:04 Ard Biesheuvel
  2020-08-17 12:40 ` Christoph Hellwig
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-08-17 12:04 UTC (permalink / raw)
  To: linux-acpi
  Cc: lorenzo.pieralisi, catalin.marinas, rjw, will, Ard Biesheuvel,
	linux-arm-kernel, lenb

The arm64 implementation of acpi_os_ioremap() was recently updated to
tighten the checks around which parts of memory are permitted to be
mapped by ACPI code, which generally only needs access to memory regions
that are statically described by firmware, and any attempts to access
memory that is in active use by the OS is generally a bug or a hacking
attempt. This tightening is based on the EFI memory map, which describes
all memory in the system.

The AArch64 architecture permits page sizes of 16k and 64k in addition
to the EFI default, which is 4k, which means that the EFI memory map may
describe regions that cannot be mapped seamlessly if the OS page size is
greater than 4k. This is usually not a problem, given that the EFI spec
does not permit memory regions requiring different memory attributes to
share a 64k page frame, and so the usual rounding to page size performed
by ioremap() is sufficient to deal with this. However, this rounding does
complicate our EFI memory map permission check, due to the loss of
information that occurs when several small regions share a single 64k
page frame (where rounding each of them will result in the same 64k
single page region).

However, due to the fact that the region check occurs *before* the call
to ioremap() where the necessary rounding is performed, we can deal
with this issue simply by removing the redundant rounding performed by
acpi_os_map_iomem(), as it appears to be the only place where the
arguments to a call to acpi_os_ioremap() are rounded up. So omit the
rounding in the call, and instead, apply the necessary offset to the
result of kmap().

Fixes: 1583052d111f ("arm64/acpi: disallow AML memory opregions to access kernel memory")
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/acpi/osl.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 6ad8cb05f672..55dbdbbae3be 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -289,7 +289,8 @@ static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
 	if (should_use_kmap(pfn)) {
 		if (pg_sz > PAGE_SIZE)
 			return NULL;
-		return (void __iomem __force *)kmap(pfn_to_page(pfn));
+		pg_off &= ~PAGE_MASK;
+		return (void __iomem __force *)(kmap(pfn_to_page(pfn)) + pg_off);
 	} else
 		return acpi_os_ioremap(pg_off, pg_sz);
 }
@@ -350,7 +351,7 @@ void __iomem __ref
 
 	pg_off = round_down(phys, PAGE_SIZE);
 	pg_sz = round_up(phys + size, PAGE_SIZE) - pg_off;
-	virt = acpi_map(pg_off, pg_sz);
+	virt = acpi_map(phys, size);
 	if (!virt) {
 		mutex_unlock(&acpi_ioremap_lock);
 		kfree(map);
@@ -358,7 +359,7 @@ void __iomem __ref
 	}
 
 	INIT_LIST_HEAD(&map->list);
-	map->virt = virt;
+	map->virt = (void *)((unsigned long)virt & PAGE_MASK);
 	map->phys = pg_off;
 	map->size = pg_sz;
 	map->track.refcount = 1;
@@ -367,7 +368,7 @@ void __iomem __ref
 
 out:
 	mutex_unlock(&acpi_ioremap_lock);
-	return map->virt + (phys - map->phys);
+	return virt;
 }
 EXPORT_SYMBOL_GPL(acpi_os_map_iomem);
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI: ioremap: avoid redundant rounding to OS page size
  2020-08-17 12:04 [PATCH] ACPI: ioremap: avoid redundant rounding to OS page size Ard Biesheuvel
@ 2020-08-17 12:40 ` Christoph Hellwig
  2020-08-17 12:59   ` Ard Biesheuvel
  2020-08-17 16:30 ` kernel test robot
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-08-17 12:40 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: lorenzo.pieralisi, catalin.marinas, rjw, linux-acpi, will,
	linux-arm-kernel, lenb

On Mon, Aug 17, 2020 at 02:04:31PM +0200, Ard Biesheuvel wrote:
> The arm64 implementation of acpi_os_ioremap() was recently updated to
> tighten the checks around which parts of memory are permitted to be
> mapped by ACPI code, which generally only needs access to memory regions
> that are statically described by firmware, and any attempts to access
> memory that is in active use by the OS is generally a bug or a hacking
> attempt. This tightening is based on the EFI memory map, which describes
> all memory in the system.
> 
> The AArch64 architecture permits page sizes of 16k and 64k in addition
> to the EFI default, which is 4k, which means that the EFI memory map may
> describe regions that cannot be mapped seamlessly if the OS page size is
> greater than 4k. This is usually not a problem, given that the EFI spec
> does not permit memory regions requiring different memory attributes to
> share a 64k page frame, and so the usual rounding to page size performed
> by ioremap() is sufficient to deal with this. However, this rounding does
> complicate our EFI memory map permission check, due to the loss of
> information that occurs when several small regions share a single 64k
> page frame (where rounding each of them will result in the same 64k
> single page region).
> 
> However, due to the fact that the region check occurs *before* the call
> to ioremap() where the necessary rounding is performed, we can deal
> with this issue simply by removing the redundant rounding performed by
> acpi_os_map_iomem(), as it appears to be the only place where the
> arguments to a call to acpi_os_ioremap() are rounded up. So omit the
> rounding in the call, and instead, apply the necessary offset to the
> result of kmap().
> 
> Fixes: 1583052d111f ("arm64/acpi: disallow AML memory opregions to access kernel memory")
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

To me the whole acpi_map() / acpi_os_map_iomem() concept looks bogus,
especially as it mixes up iomem and RAM pages in a really bad way,
and then throws in staic fixmap-like mappings as well.

Also looking at the callers I see no point in keeping a list of the
memory mappings.  Does anyone have an idea where this craziness comes
from?  Which of the callers actually has to deal both with iomem and
RAM mappings at the same time?

It seems like we should be able to untangle the few callers and remove
this mess entirely.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI: ioremap: avoid redundant rounding to OS page size
  2020-08-17 12:40 ` Christoph Hellwig
@ 2020-08-17 12:59   ` Ard Biesheuvel
  0 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2020-08-17 12:59 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Lorenzo Pieralisi, Catalin Marinas, Rafael J. Wysocki,
	ACPI Devel Maling List, Will Deacon, Linux ARM, Len Brown

On Mon, 17 Aug 2020 at 14:40, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Aug 17, 2020 at 02:04:31PM +0200, Ard Biesheuvel wrote:
> > The arm64 implementation of acpi_os_ioremap() was recently updated to
> > tighten the checks around which parts of memory are permitted to be
> > mapped by ACPI code, which generally only needs access to memory regions
> > that are statically described by firmware, and any attempts to access
> > memory that is in active use by the OS is generally a bug or a hacking
> > attempt. This tightening is based on the EFI memory map, which describes
> > all memory in the system.
> >
> > The AArch64 architecture permits page sizes of 16k and 64k in addition
> > to the EFI default, which is 4k, which means that the EFI memory map may
> > describe regions that cannot be mapped seamlessly if the OS page size is
> > greater than 4k. This is usually not a problem, given that the EFI spec
> > does not permit memory regions requiring different memory attributes to
> > share a 64k page frame, and so the usual rounding to page size performed
> > by ioremap() is sufficient to deal with this. However, this rounding does
> > complicate our EFI memory map permission check, due to the loss of
> > information that occurs when several small regions share a single 64k
> > page frame (where rounding each of them will result in the same 64k
> > single page region).
> >
> > However, due to the fact that the region check occurs *before* the call
> > to ioremap() where the necessary rounding is performed, we can deal
> > with this issue simply by removing the redundant rounding performed by
> > acpi_os_map_iomem(), as it appears to be the only place where the
> > arguments to a call to acpi_os_ioremap() are rounded up. So omit the
> > rounding in the call, and instead, apply the necessary offset to the
> > result of kmap().
> >
> > Fixes: 1583052d111f ("arm64/acpi: disallow AML memory opregions to access kernel memory")
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> To me the whole acpi_map() / acpi_os_map_iomem() concept looks bogus,
> especially as it mixes up iomem and RAM pages in a really bad way,
> and then throws in staic fixmap-like mappings as well.
>
> Also looking at the callers I see no point in keeping a list of the
> memory mappings.  Does anyone have an idea where this craziness comes
> from?  Which of the callers actually has to deal both with iomem and
> RAM mappings at the same time?
>
> It seems like we should be able to untangle the few callers and remove
> this mess entirely.

That would be my preference as well. However, this code is used by
x86, ia64 and arm64, and I'd like to get this piece in as a fix, given
that the referenced patch was merged in v5.9-rc1, and causes issues on
64k kernels running on older arm64 systems with not-quite-compliant
EFI firmware.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI: ioremap: avoid redundant rounding to OS page size
  2020-08-17 12:04 [PATCH] ACPI: ioremap: avoid redundant rounding to OS page size Ard Biesheuvel
  2020-08-17 12:40 ` Christoph Hellwig
@ 2020-08-17 16:30 ` kernel test robot
  2020-08-17 17:06 ` kernel test robot
  2020-08-17 23:41 ` kernel test robot
  3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-08-17 16:30 UTC (permalink / raw)
  To: Ard Biesheuvel, linux-acpi
  Cc: lorenzo.pieralisi, kbuild-all, catalin.marinas, rjw,
	clang-built-linux, will, Ard Biesheuvel, linux-arm-kernel, lenb

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

Hi Ard,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pm/linux-next]
[also build test WARNING on arm64/for-next/core arm/for-next soc/for-next v5.9-rc1 next-20200817]
[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/Ard-Biesheuvel/ACPI-ioremap-avoid-redundant-rounding-to-OS-page-size/20200817-200603
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-a013-20200817 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project de71b46a519db014ce906a39f8a0e1b235ef1568)
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
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

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

All warnings (new ones prefixed by >>):

>> drivers/acpi/osl.c:341:6: warning: variable 'virt' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (map) {
               ^~~
   drivers/acpi/osl.c:371:9: note: uninitialized use occurs here
           return virt;
                  ^~~~
   drivers/acpi/osl.c:341:2: note: remove the 'if' if its condition is always false
           if (map) {
           ^~~~~~~~~~
   drivers/acpi/osl.c:326:20: note: initialize the variable 'virt' to silence this warning
           void __iomem *virt;
                             ^
                              = NULL
   1 warning generated.

# https://github.com/0day-ci/linux/commit/a34cc34917319aed90ebf9b0fbf4146666f5f75d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ard-Biesheuvel/ACPI-ioremap-avoid-redundant-rounding-to-OS-page-size/20200817-200603
git checkout a34cc34917319aed90ebf9b0fbf4146666f5f75d
vim +341 drivers/acpi/osl.c

ba242d5b1a84bc Myron Stowe         2012-01-20  308  
9d128ed17c672b Rafael J. Wysocki   2016-01-02  309  /**
9d128ed17c672b Rafael J. Wysocki   2016-01-02  310   * acpi_os_map_iomem - Get a virtual address for a given physical address range.
9d128ed17c672b Rafael J. Wysocki   2016-01-02  311   * @phys: Start of the physical address range to map.
9d128ed17c672b Rafael J. Wysocki   2016-01-02  312   * @size: Size of the physical address range to map.
9d128ed17c672b Rafael J. Wysocki   2016-01-02  313   *
9d128ed17c672b Rafael J. Wysocki   2016-01-02  314   * Look up the given physical address range in the list of existing ACPI memory
9d128ed17c672b Rafael J. Wysocki   2016-01-02  315   * mappings.  If found, get a reference to it and return a pointer to it (its
9d128ed17c672b Rafael J. Wysocki   2016-01-02  316   * virtual address).  If not found, map it, add it to that list and return a
9d128ed17c672b Rafael J. Wysocki   2016-01-02  317   * pointer to it.
9d128ed17c672b Rafael J. Wysocki   2016-01-02  318   *
8d3523fb3b7274 Lv Zheng            2016-12-14  319   * During early init (when acpi_permanent_mmap has not been set yet) this
9d128ed17c672b Rafael J. Wysocki   2016-01-02  320   * routine simply calls __acpi_map_table() to get the job done.
9d128ed17c672b Rafael J. Wysocki   2016-01-02  321   */
9fe51603d95341 Qian Cai            2019-06-03  322  void __iomem __ref
9fe51603d95341 Qian Cai            2019-06-03  323  *acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
^1da177e4c3f41 Linus Torvalds      2005-04-16  324  {
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  325  	struct acpi_ioremap *map;
620242ae8c3d9c Myron Stowe         2010-10-21  326  	void __iomem *virt;
2d6d9fd3a54a28 Rafael J. Wysocki   2011-01-19  327  	acpi_physical_address pg_off;
2d6d9fd3a54a28 Rafael J. Wysocki   2011-01-19  328  	acpi_size pg_sz;
620242ae8c3d9c Myron Stowe         2010-10-21  329  
^1da177e4c3f41 Linus Torvalds      2005-04-16  330  	if (phys > ULONG_MAX) {
^1da177e4c3f41 Linus Torvalds      2005-04-16  331  		printk(KERN_ERR PREFIX "Cannot map memory that high\n");
70c0846e430881 Randy Dunlap        2007-02-13  332  		return NULL;
^1da177e4c3f41 Linus Torvalds      2005-04-16  333  	}
620242ae8c3d9c Myron Stowe         2010-10-21  334  
8d3523fb3b7274 Lv Zheng            2016-12-14  335  	if (!acpi_permanent_mmap)
ad71860a17ba33 Alexey Starikovskiy 2007-02-02  336  		return __acpi_map_table((unsigned long)phys, size);
620242ae8c3d9c Myron Stowe         2010-10-21  337  
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  338  	mutex_lock(&acpi_ioremap_lock);
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  339  	/* Check if there's a suitable mapping already. */
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  340  	map = acpi_map_lookup(phys, size);
7ffd0443f25024 Rafael J. Wysocki   2011-02-08 @341  	if (map) {
1757659d022b73 Rafael J. Wysocki   2020-07-02  342  		map->track.refcount++;
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  343  		goto out;
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  344  	}
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  345  
620242ae8c3d9c Myron Stowe         2010-10-21  346  	map = kzalloc(sizeof(*map), GFP_KERNEL);
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  347  	if (!map) {
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  348  		mutex_unlock(&acpi_ioremap_lock);
620242ae8c3d9c Myron Stowe         2010-10-21  349  		return NULL;
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  350  	}
620242ae8c3d9c Myron Stowe         2010-10-21  351  
4a3cba5e72a523 Myron Stowe         2010-10-21  352  	pg_off = round_down(phys, PAGE_SIZE);
4a3cba5e72a523 Myron Stowe         2010-10-21  353  	pg_sz = round_up(phys + size, PAGE_SIZE) - pg_off;
a34cc34917319a Ard Biesheuvel      2020-08-17  354  	virt = acpi_map(phys, size);
620242ae8c3d9c Myron Stowe         2010-10-21  355  	if (!virt) {
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  356  		mutex_unlock(&acpi_ioremap_lock);
620242ae8c3d9c Myron Stowe         2010-10-21  357  		kfree(map);
620242ae8c3d9c Myron Stowe         2010-10-21  358  		return NULL;
620242ae8c3d9c Myron Stowe         2010-10-21  359  	}
620242ae8c3d9c Myron Stowe         2010-10-21  360  
620242ae8c3d9c Myron Stowe         2010-10-21  361  	INIT_LIST_HEAD(&map->list);
a34cc34917319a Ard Biesheuvel      2020-08-17  362  	map->virt = (void *)((unsigned long)virt & PAGE_MASK);
4a3cba5e72a523 Myron Stowe         2010-10-21  363  	map->phys = pg_off;
4a3cba5e72a523 Myron Stowe         2010-10-21  364  	map->size = pg_sz;
1757659d022b73 Rafael J. Wysocki   2020-07-02  365  	map->track.refcount = 1;
620242ae8c3d9c Myron Stowe         2010-10-21  366  
78cdb3ed405379 Myron Stowe         2010-10-21  367  	list_add_tail_rcu(&map->list, &acpi_ioremaps);
620242ae8c3d9c Myron Stowe         2010-10-21  368  
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  369  out:
7ffd0443f25024 Rafael J. Wysocki   2011-02-08  370  	mutex_unlock(&acpi_ioremap_lock);
a34cc34917319a Ard Biesheuvel      2020-08-17  371  	return virt;
^1da177e4c3f41 Linus Torvalds      2005-04-16  372  }
a238317ce81855 Lv Zheng            2014-05-20  373  EXPORT_SYMBOL_GPL(acpi_os_map_iomem);
a238317ce81855 Lv Zheng            2014-05-20  374  

---
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: 32997 bytes --]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI: ioremap: avoid redundant rounding to OS page size
  2020-08-17 12:04 [PATCH] ACPI: ioremap: avoid redundant rounding to OS page size Ard Biesheuvel
  2020-08-17 12:40 ` Christoph Hellwig
  2020-08-17 16:30 ` kernel test robot
@ 2020-08-17 17:06 ` kernel test robot
  2020-08-17 23:41 ` kernel test robot
  3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-08-17 17:06 UTC (permalink / raw)
  To: Ard Biesheuvel, linux-acpi
  Cc: lorenzo.pieralisi, kbuild-all, catalin.marinas, rjw, will,
	Ard Biesheuvel, linux-arm-kernel, lenb

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

Hi Ard,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pm/linux-next]
[also build test WARNING on arm64/for-next/core arm/for-next soc/for-next v5.9-rc1 next-20200817]
[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/Ard-Biesheuvel/ACPI-ioremap-avoid-redundant-rounding-to-OS-page-size/20200817-200603
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-s022-20200817 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-180-g49f7e13a-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

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


sparse warnings: (new ones prefixed by >>)

>> drivers/acpi/osl.c:362:19: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void [noderef] __iomem *virt @@     got void * @@
>> drivers/acpi/osl.c:362:19: sparse:     expected void [noderef] __iomem *virt
>> drivers/acpi/osl.c:362:19: sparse:     got void *
   drivers/acpi/osl.c:377:17: sparse: sparse: cast removes address space '__iomem' of expression
   drivers/acpi/osl.c:714:1: sparse: sparse: context imbalance in 'acpi_os_read_memory' - wrong count at exit
   drivers/acpi/osl.c:747:1: sparse: sparse: context imbalance in 'acpi_os_write_memory' - wrong count at exit

# https://github.com/0day-ci/linux/commit/a34cc34917319aed90ebf9b0fbf4146666f5f75d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ard-Biesheuvel/ACPI-ioremap-avoid-redundant-rounding-to-OS-page-size/20200817-200603
git checkout a34cc34917319aed90ebf9b0fbf4146666f5f75d
vim +362 drivers/acpi/osl.c

   308	
   309	/**
   310	 * acpi_os_map_iomem - Get a virtual address for a given physical address range.
   311	 * @phys: Start of the physical address range to map.
   312	 * @size: Size of the physical address range to map.
   313	 *
   314	 * Look up the given physical address range in the list of existing ACPI memory
   315	 * mappings.  If found, get a reference to it and return a pointer to it (its
   316	 * virtual address).  If not found, map it, add it to that list and return a
   317	 * pointer to it.
   318	 *
   319	 * During early init (when acpi_permanent_mmap has not been set yet) this
   320	 * routine simply calls __acpi_map_table() to get the job done.
   321	 */
   322	void __iomem __ref
   323	*acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
   324	{
   325		struct acpi_ioremap *map;
   326		void __iomem *virt;
   327		acpi_physical_address pg_off;
   328		acpi_size pg_sz;
   329	
   330		if (phys > ULONG_MAX) {
   331			printk(KERN_ERR PREFIX "Cannot map memory that high\n");
   332			return NULL;
   333		}
   334	
   335		if (!acpi_permanent_mmap)
   336			return __acpi_map_table((unsigned long)phys, size);
   337	
   338		mutex_lock(&acpi_ioremap_lock);
   339		/* Check if there's a suitable mapping already. */
   340		map = acpi_map_lookup(phys, size);
   341		if (map) {
   342			map->track.refcount++;
   343			goto out;
   344		}
   345	
   346		map = kzalloc(sizeof(*map), GFP_KERNEL);
   347		if (!map) {
   348			mutex_unlock(&acpi_ioremap_lock);
   349			return NULL;
   350		}
   351	
   352		pg_off = round_down(phys, PAGE_SIZE);
   353		pg_sz = round_up(phys + size, PAGE_SIZE) - pg_off;
   354		virt = acpi_map(phys, size);
   355		if (!virt) {
   356			mutex_unlock(&acpi_ioremap_lock);
   357			kfree(map);
   358			return NULL;
   359		}
   360	
   361		INIT_LIST_HEAD(&map->list);
 > 362		map->virt = (void *)((unsigned long)virt & PAGE_MASK);
   363		map->phys = pg_off;
   364		map->size = pg_sz;
   365		map->track.refcount = 1;
   366	
   367		list_add_tail_rcu(&map->list, &acpi_ioremaps);
   368	
   369	out:
   370		mutex_unlock(&acpi_ioremap_lock);
   371		return virt;
   372	}
   373	EXPORT_SYMBOL_GPL(acpi_os_map_iomem);
   374	

---
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: 41158 bytes --]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ACPI: ioremap: avoid redundant rounding to OS page size
  2020-08-17 12:04 [PATCH] ACPI: ioremap: avoid redundant rounding to OS page size Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2020-08-17 17:06 ` kernel test robot
@ 2020-08-17 23:41 ` kernel test robot
  3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-08-17 23:41 UTC (permalink / raw)
  To: Ard Biesheuvel, linux-acpi
  Cc: lorenzo.pieralisi, kbuild-all, catalin.marinas, rjw, will,
	Ard Biesheuvel, linux-arm-kernel, lenb

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

Hi Ard,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pm/linux-next]
[also build test WARNING on arm64/for-next/core arm/for-next soc/for-next v5.9-rc1 next-20200817]
[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/Ard-Biesheuvel/ACPI-ioremap-avoid-redundant-rounding-to-OS-page-size/20200817-200603
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-randconfig-m021-20200817 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

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

smatch warnings:
drivers/acpi/osl.c:371 acpi_os_map_iomem() error: uninitialized symbol 'virt'.

# https://github.com/0day-ci/linux/commit/a34cc34917319aed90ebf9b0fbf4146666f5f75d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ard-Biesheuvel/ACPI-ioremap-avoid-redundant-rounding-to-OS-page-size/20200817-200603
git checkout a34cc34917319aed90ebf9b0fbf4146666f5f75d
vim +/virt +371 drivers/acpi/osl.c

   308	
   309	/**
   310	 * acpi_os_map_iomem - Get a virtual address for a given physical address range.
   311	 * @phys: Start of the physical address range to map.
   312	 * @size: Size of the physical address range to map.
   313	 *
   314	 * Look up the given physical address range in the list of existing ACPI memory
   315	 * mappings.  If found, get a reference to it and return a pointer to it (its
   316	 * virtual address).  If not found, map it, add it to that list and return a
   317	 * pointer to it.
   318	 *
   319	 * During early init (when acpi_permanent_mmap has not been set yet) this
   320	 * routine simply calls __acpi_map_table() to get the job done.
   321	 */
   322	void __iomem __ref
   323	*acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
   324	{
   325		struct acpi_ioremap *map;
   326		void __iomem *virt;
   327		acpi_physical_address pg_off;
   328		acpi_size pg_sz;
   329	
   330		if (phys > ULONG_MAX) {
   331			printk(KERN_ERR PREFIX "Cannot map memory that high\n");
   332			return NULL;
   333		}
   334	
   335		if (!acpi_permanent_mmap)
   336			return __acpi_map_table((unsigned long)phys, size);
   337	
   338		mutex_lock(&acpi_ioremap_lock);
   339		/* Check if there's a suitable mapping already. */
   340		map = acpi_map_lookup(phys, size);
   341		if (map) {
   342			map->track.refcount++;
   343			goto out;
   344		}
   345	
   346		map = kzalloc(sizeof(*map), GFP_KERNEL);
   347		if (!map) {
   348			mutex_unlock(&acpi_ioremap_lock);
   349			return NULL;
   350		}
   351	
   352		pg_off = round_down(phys, PAGE_SIZE);
   353		pg_sz = round_up(phys + size, PAGE_SIZE) - pg_off;
   354		virt = acpi_map(phys, size);
   355		if (!virt) {
   356			mutex_unlock(&acpi_ioremap_lock);
   357			kfree(map);
   358			return NULL;
   359		}
   360	
   361		INIT_LIST_HEAD(&map->list);
   362		map->virt = (void *)((unsigned long)virt & PAGE_MASK);
   363		map->phys = pg_off;
   364		map->size = pg_sz;
   365		map->track.refcount = 1;
   366	
   367		list_add_tail_rcu(&map->list, &acpi_ioremaps);
   368	
   369	out:
   370		mutex_unlock(&acpi_ioremap_lock);
 > 371		return virt;
   372	}
   373	EXPORT_SYMBOL_GPL(acpi_os_map_iomem);
   374	

---
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: 35926 bytes --]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-08-17 23:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-17 12:04 [PATCH] ACPI: ioremap: avoid redundant rounding to OS page size Ard Biesheuvel
2020-08-17 12:40 ` Christoph Hellwig
2020-08-17 12:59   ` Ard Biesheuvel
2020-08-17 16:30 ` kernel test robot
2020-08-17 17:06 ` kernel test robot
2020-08-17 23:41 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).