| Message ID | 20220915141941.3408991-1-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/etnaviv: don't truncate physical page address | expand |
Hi Lucas, On Do, 2022-09-15 at 16:19 +0200, Lucas Stach wrote: > While the interface for the MMU mapping takes phys_addr_t to hold a > full 64bit address when necessary and MMUv2 is able to map physical > addresses with up to 40bit, etnaviv_iommu_map() truncates the address > to 32bits. Fix this by using the correct type. > > Fixes: 931e97f3afd8 ("drm/etnaviv: mmuv2: support 40 bit phys address") > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > index dc1aa738c4f1..2ff80d5ccf07 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > @@ -80,7 +80,7 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_context *context, u32 iova, > return -EINVAL; > > > for_each_sgtable_dma_sg(sgt, sg, i) { > - u32 pa = sg_dma_address(sg) - sg->offset; > + phys_addr_t pa = sg_dma_address(sg) - sg->offset; > size_t bytes = sg_dma_len(sg) + sg->offset; > > > VERB("map[%d]: %08x %08x(%zx)", i, iova, pa, bytes); ^^^^ ^^ Use %pap, &pa here? regards Philipp
Hi Lucas,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.0-rc5 next-20220915]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Lucas-Stach/drm-etnaviv-don-t-truncate-physical-page-address/20220915-222156
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
config: arm64-randconfig-r001-20220915 (https://download.01.org/0day-ci/archive/20220916/202209160334.FfC9eCgv-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
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 arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/941356fb766e7f49216d44f0df7614c2e4610a11
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Lucas-Stach/drm-etnaviv-don-t-truncate-physical-page-address/20220915-222156
git checkout 941356fb766e7f49216d44f0df7614c2e4610a11
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/gpu/drm/etnaviv/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/etnaviv/etnaviv_mmu.c:86:44: warning: format specifies type 'unsigned int' but the argument has type 'phys_addr_t' (aka 'unsigned long long') [-Wformat]
VERB("map[%d]: %08x %08x(%zx)", i, iova, pa, bytes);
~~~~ ^~
%08llx
drivers/gpu/drm/etnaviv/etnaviv_drv.h:85:52: note: expanded from macro 'VERB'
#define VERB(fmt, ...) if (0) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/drm/drm_print.h:526:32: note: expanded from macro 'DRM_DEBUG'
__drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
1 warning generated.
vim +86 drivers/gpu/drm/etnaviv/etnaviv_mmu.c
50073cf98d1635 Lucas Stach 2017-09-07 71
27b67278e007b5 Lucas Stach 2019-07-05 72 static int etnaviv_iommu_map(struct etnaviv_iommu_context *context, u32 iova,
a8c21a5451d831 The etnaviv authors 2015-12-03 73 struct sg_table *sgt, unsigned len, int prot)
27b67278e007b5 Lucas Stach 2019-07-05 74 { struct scatterlist *sg;
a8c21a5451d831 The etnaviv authors 2015-12-03 75 unsigned int da = iova;
182354a526a054 Marek Szyprowski 2020-04-28 76 unsigned int i;
a8c21a5451d831 The etnaviv authors 2015-12-03 77 int ret;
a8c21a5451d831 The etnaviv authors 2015-12-03 78
27b67278e007b5 Lucas Stach 2019-07-05 79 if (!context || !sgt)
a8c21a5451d831 The etnaviv authors 2015-12-03 80 return -EINVAL;
a8c21a5451d831 The etnaviv authors 2015-12-03 81
182354a526a054 Marek Szyprowski 2020-04-28 82 for_each_sgtable_dma_sg(sgt, sg, i) {
941356fb766e7f Lucas Stach 2022-09-15 83 phys_addr_t pa = sg_dma_address(sg) - sg->offset;
a8c21a5451d831 The etnaviv authors 2015-12-03 84 size_t bytes = sg_dma_len(sg) + sg->offset;
a8c21a5451d831 The etnaviv authors 2015-12-03 85
a8c21a5451d831 The etnaviv authors 2015-12-03 @86 VERB("map[%d]: %08x %08x(%zx)", i, iova, pa, bytes);
a8c21a5451d831 The etnaviv authors 2015-12-03 87
27b67278e007b5 Lucas Stach 2019-07-05 88 ret = etnaviv_context_map(context, da, pa, bytes, prot);
a8c21a5451d831 The etnaviv authors 2015-12-03 89 if (ret)
a8c21a5451d831 The etnaviv authors 2015-12-03 90 goto fail;
a8c21a5451d831 The etnaviv authors 2015-12-03 91
a8c21a5451d831 The etnaviv authors 2015-12-03 92 da += bytes;
a8c21a5451d831 The etnaviv authors 2015-12-03 93 }
a8c21a5451d831 The etnaviv authors 2015-12-03 94
9247fcca3982a2 Lucas Stach 2022-03-23 95 context->flush_seq++;
9247fcca3982a2 Lucas Stach 2022-03-23 96
a8c21a5451d831 The etnaviv authors 2015-12-03 97 return 0;
a8c21a5451d831 The etnaviv authors 2015-12-03 98
a8c21a5451d831 The etnaviv authors 2015-12-03 99 fail:
182354a526a054 Marek Szyprowski 2020-04-28 100 etnaviv_context_unmap(context, iova, da - iova);
a8c21a5451d831 The etnaviv authors 2015-12-03 101 return ret;
a8c21a5451d831 The etnaviv authors 2015-12-03 102 }
a8c21a5451d831 The etnaviv authors 2015-12-03 103
Hi Lucas,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.0-rc5 next-20220915]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Lucas-Stach/drm-etnaviv-don-t-truncate-physical-page-address/20220915-222156
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
config: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20220916/202209160813.srwkRhUH-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
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/intel-lab-lkp/linux/commit/941356fb766e7f49216d44f0df7614c2e4610a11
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Lucas-Stach/drm-etnaviv-don-t-truncate-physical-page-address/20220915-222156
git checkout 941356fb766e7f49216d44f0df7614c2e4610a11
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/gpu/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/drm/drm_mm.h:51,
from include/drm/drm_vma_manager.h:26,
from include/drm/drm_gem.h:40,
from drivers/gpu/drm/etnaviv/etnaviv_drv.h:16,
from drivers/gpu/drm/etnaviv/etnaviv_mmu.c:11:
drivers/gpu/drm/etnaviv/etnaviv_mmu.c: In function 'etnaviv_iommu_map':
>> drivers/gpu/drm/etnaviv/etnaviv_mmu.c:86:22: warning: format '%x' expects argument of type 'unsigned int', but argument 5 has type 'phys_addr_t' {aka 'long long unsigned int'} [-Wformat=]
86 | VERB("map[%d]: %08x %08x(%zx)", i, iova, pa, bytes);
| ^~~~~~~~~~~~~~~~~~~~~~~~~ ~~
| |
| phys_addr_t {aka long long unsigned int}
include/drm/drm_print.h:526:32: note: in definition of macro 'DRM_DEBUG'
526 | __drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
| ^~~
drivers/gpu/drm/etnaviv/etnaviv_mmu.c:86:17: note: in expansion of macro 'VERB'
86 | VERB("map[%d]: %08x %08x(%zx)", i, iova, pa, bytes);
| ^~~~
drivers/gpu/drm/etnaviv/etnaviv_mmu.c:86:40: note: format string is defined here
86 | VERB("map[%d]: %08x %08x(%zx)", i, iova, pa, bytes);
| ~~~^
| |
| unsigned int
| %08llx
{standard input}: Assembler messages:
{standard input}:2347: Error: Register number out of range 0..3
{standard input}:2347: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 40
{standard input}:2347: Warning: Only the first path encountering the conflict is reported
{standard input}:2342: Warning: This is the location of the conflicting usage
{standard input}:2348: Error: Register number out of range 0..3
{standard input}:2348: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 40
{standard input}:2348: Warning: Only the first path encountering the conflict is reported
{standard input}:2342: Warning: This is the location of the conflicting usage
{standard input}:2348: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 40
{standard input}:2348: Warning: Only the first path encountering the conflict is reported
{standard input}:2347: Warning: This is the location of the conflicting usage
{standard input}:2351: Error: Register number out of range 0..3
{standard input}:2352: Error: Register number out of range 0..3
{standard input}:2352: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 40
{standard input}:2352: Warning: Only the first path encountering the conflict is reported
{standard input}:2351: Warning: This is the location of the conflicting usage
{standard input}:2532: Error: Register number out of range 0..2
{standard input}:2532: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
{standard input}:2532: Warning: Only the first path encountering the conflict is reported
{standard input}:2528: Warning: This is the location of the conflicting usage
{standard input}:2533: Error: Register number out of range 0..2
{standard input}:2533: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
{standard input}:2533: Warning: Only the first path encountering the conflict is reported
{standard input}:2528: Warning: This is the location of the conflicting usage
{standard input}:2533: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
{standard input}:2533: Warning: Only the first path encountering the conflict is reported
{standard input}:2532: Warning: This is the location of the conflicting usage
{standard input}:2534: Error: Register number out of range 0..2
{standard input}:2534: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
{standard input}:2534: Warning: Only the first path encountering the conflict is reported
{standard input}:2528: Warning: This is the location of the conflicting usage
{standard input}:2534: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
{standard input}:2534: Warning: Only the first path encountering the conflict is reported
{standard input}:2532: Warning: This is the location of the conflicting usage
{standard input}:2534: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
{standard input}:2534: Warning: Only the first path encountering the conflict is reported
{standard input}:2533: Warning: This is the location of the conflicting usage
{standard input}:2537: Error: Register number out of range 0..2
{standard input}:2538: Error: Register number out of range 0..2
{standard input}:2538: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 37
{standard input}:2538: Warning: Only the first path encountering the conflict is reported
{standard input}:2537: Warning: This is the location of the conflicting usage
{standard input}:3788: Error: Register number out of range 0..4
{standard input}:3788: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 38
{standard input}:3788: Warning: Only the first path encountering the conflict is reported
{standard input}:3782: Warning: This is the location of the conflicting usage
{standard input}:3791: Error: Register number out of range 0..4
{standard input}:3792: Error: Register number out of range 0..4
{standard input}:3792: Warning: Use of 'mov' violates WAW dependency 'GR%, % in 1 - 127' (impliedf), specific resource number is 38
{standard input}:3792: Warning: Only the first path encountering the conflict is reported
{standard input}:3791: Warning: This is the location of the conflicting usage
vim +86 drivers/gpu/drm/etnaviv/etnaviv_mmu.c
50073cf98d1635 Lucas Stach 2017-09-07 71
27b67278e007b5 Lucas Stach 2019-07-05 72 static int etnaviv_iommu_map(struct etnaviv_iommu_context *context, u32 iova,
a8c21a5451d831 The etnaviv authors 2015-12-03 73 struct sg_table *sgt, unsigned len, int prot)
27b67278e007b5 Lucas Stach 2019-07-05 74 { struct scatterlist *sg;
a8c21a5451d831 The etnaviv authors 2015-12-03 75 unsigned int da = iova;
182354a526a054 Marek Szyprowski 2020-04-28 76 unsigned int i;
a8c21a5451d831 The etnaviv authors 2015-12-03 77 int ret;
a8c21a5451d831 The etnaviv authors 2015-12-03 78
27b67278e007b5 Lucas Stach 2019-07-05 79 if (!context || !sgt)
a8c21a5451d831 The etnaviv authors 2015-12-03 80 return -EINVAL;
a8c21a5451d831 The etnaviv authors 2015-12-03 81
182354a526a054 Marek Szyprowski 2020-04-28 82 for_each_sgtable_dma_sg(sgt, sg, i) {
941356fb766e7f Lucas Stach 2022-09-15 83 phys_addr_t pa = sg_dma_address(sg) - sg->offset;
a8c21a5451d831 The etnaviv authors 2015-12-03 84 size_t bytes = sg_dma_len(sg) + sg->offset;
a8c21a5451d831 The etnaviv authors 2015-12-03 85
a8c21a5451d831 The etnaviv authors 2015-12-03 @86 VERB("map[%d]: %08x %08x(%zx)", i, iova, pa, bytes);
a8c21a5451d831 The etnaviv authors 2015-12-03 87
27b67278e007b5 Lucas Stach 2019-07-05 88 ret = etnaviv_context_map(context, da, pa, bytes, prot);
a8c21a5451d831 The etnaviv authors 2015-12-03 89 if (ret)
a8c21a5451d831 The etnaviv authors 2015-12-03 90 goto fail;
a8c21a5451d831 The etnaviv authors 2015-12-03 91
a8c21a5451d831 The etnaviv authors 2015-12-03 92 da += bytes;
a8c21a5451d831 The etnaviv authors 2015-12-03 93 }
a8c21a5451d831 The etnaviv authors 2015-12-03 94
9247fcca3982a2 Lucas Stach 2022-03-23 95 context->flush_seq++;
9247fcca3982a2 Lucas Stach 2022-03-23 96
a8c21a5451d831 The etnaviv authors 2015-12-03 97 return 0;
a8c21a5451d831 The etnaviv authors 2015-12-03 98
a8c21a5451d831 The etnaviv authors 2015-12-03 99 fail:
182354a526a054 Marek Szyprowski 2020-04-28 100 etnaviv_context_unmap(context, iova, da - iova);
a8c21a5451d831 The etnaviv authors 2015-12-03 101 return ret;
a8c21a5451d831 The etnaviv authors 2015-12-03 102 }
a8c21a5451d831 The etnaviv authors 2015-12-03 103
Hi Philipp, Am Donnerstag, dem 15.09.2022 um 16:40 +0200 schrieb Philipp Zabel: > Hi Lucas, > > On Do, 2022-09-15 at 16:19 +0200, Lucas Stach wrote: > > While the interface for the MMU mapping takes phys_addr_t to hold a > > full 64bit address when necessary and MMUv2 is able to map physical > > addresses with up to 40bit, etnaviv_iommu_map() truncates the address > > to 32bits. Fix this by using the correct type. > > > > Fixes: 931e97f3afd8 ("drm/etnaviv: mmuv2: support 40 bit phys address") > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > --- > > drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > > index dc1aa738c4f1..2ff80d5ccf07 100644 > > --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > > @@ -80,7 +80,7 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_context *context, u32 iova, > > return -EINVAL; > > > > > > for_each_sgtable_dma_sg(sgt, sg, i) { > > - u32 pa = sg_dma_address(sg) - sg->offset; > > + phys_addr_t pa = sg_dma_address(sg) - sg->offset; > > size_t bytes = sg_dma_len(sg) + sg->offset; > > > > > > VERB("map[%d]: %08x %08x(%zx)", i, iova, pa, bytes); > ^^^^ ^^ > Use %pap, &pa here? > Yep, I actually thought about this when writing the patch, but then got distracted and forgot to add this change. :/ Regards, Lucas
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c index dc1aa738c4f1..2ff80d5ccf07 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c @@ -80,7 +80,7 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_context *context, u32 iova, return -EINVAL; for_each_sgtable_dma_sg(sgt, sg, i) { - u32 pa = sg_dma_address(sg) - sg->offset; + phys_addr_t pa = sg_dma_address(sg) - sg->offset; size_t bytes = sg_dma_len(sg) + sg->offset; VERB("map[%d]: %08x %08x(%zx)", i, iova, pa, bytes);
While the interface for the MMU mapping takes phys_addr_t to hold a full 64bit address when necessary and MMUv2 is able to map physical addresses with up to 40bit, etnaviv_iommu_map() truncates the address to 32bits. Fix this by using the correct type. Fixes: 931e97f3afd8 ("drm/etnaviv: mmuv2: support 40 bit phys address") Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)