Re: [PATCH v8 6/7] drm/msm: Set the global virtual address range from the IOMMU domain

[kernel test robot <lkp@intel.com>]

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

Hi Jordan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on next-20200611]
[cannot apply to iommu/next robh/for-next arm/for-next keystone/next rockchip/for-next arm64/for-next/core shawnguo/for-next soc/for-next v5.7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Jordan-Crouse/iommu-arm-smmu-Enable-split-pagetable-support/20200612-094718
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git b961f8dc8976c091180839f4483d67b7c2ca2578
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.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
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

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 >>, old ones prefixed by <<):

In file included from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from include/linux/ascii85.h:11,
from drivers/gpu/drm/msm/adreno/adreno_gpu.c:9:
drivers/gpu/drm/msm/adreno/adreno_gpu.c: In function 'adreno_iommu_create_address_space':
include/linux/bits.h:37:11: warning: right shift count is negative [-Wshift-count-negative]
37 |   (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
|           ^~
include/linux/bits.h:39:31: note: in expansion of macro '__GENMASK'
39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|                               ^~~~~~~~~
>> drivers/gpu/drm/msm/adreno/adreno_gpu.c:206:11: note: in expansion of macro 'GENMASK'
206 |   start & GENMASK(48, 0), size);
|           ^~~~~~~
--
In file included from include/linux/bits.h:6,
from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from drivers/gpu/drm/msm/msm_drv.h:11,
from drivers/gpu/drm/msm/msm_iommu.c:7:
drivers/gpu/drm/msm/msm_iommu.c: In function 'msm_iommu_map':
>> include/vdso/bits.h:7:26: warning: left shift count >= width of type [-Wshift-count-overflow]
7 | #define BIT(nr)   (UL(1) << (nr))
|                          ^~
>> drivers/gpu/drm/msm/msm_iommu.c:40:13: note: in expansion of macro 'BIT'
40 |  if (iova & BIT(48))
|             ^~~
In file included from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from drivers/gpu/drm/msm/msm_drv.h:11,
from drivers/gpu/drm/msm/msm_iommu.c:7:
>> include/linux/bits.h:36:22: warning: left shift count >= width of type [-Wshift-count-overflow]
36 |  (((~UL(0)) - (UL(1) << (l)) + 1) &          |                      ^~
include/linux/bits.h:39:31: note: in expansion of macro '__GENMASK'
39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|                               ^~~~~~~~~
>> drivers/gpu/drm/msm/msm_iommu.c:41:11: note: in expansion of macro 'GENMASK'
41 |   iova |= GENMASK(63, 49);
|           ^~~~~~~
include/linux/bits.h:37:11: warning: right shift count is negative [-Wshift-count-negative]
37 |   (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
|           ^~
include/linux/bits.h:39:31: note: in expansion of macro '__GENMASK'
39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|                               ^~~~~~~~~
>> drivers/gpu/drm/msm/msm_iommu.c:41:11: note: in expansion of macro 'GENMASK'
41 |   iova |= GENMASK(63, 49);
|           ^~~~~~~
In file included from include/linux/bits.h:6,
from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from drivers/gpu/drm/msm/msm_drv.h:11,
from drivers/gpu/drm/msm/msm_iommu.c:7:
drivers/gpu/drm/msm/msm_iommu.c: In function 'msm_iommu_unmap':
>> include/vdso/bits.h:7:26: warning: left shift count >= width of type [-Wshift-count-overflow]
7 | #define BIT(nr)   (UL(1) << (nr))
|                          ^~
drivers/gpu/drm/msm/msm_iommu.c:53:13: note: in expansion of macro 'BIT'
53 |  if (iova & BIT(48))
|             ^~~
In file included from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from drivers/gpu/drm/msm/msm_drv.h:11,
from drivers/gpu/drm/msm/msm_iommu.c:7:
>> include/linux/bits.h:36:22: warning: left shift count >= width of type [-Wshift-count-overflow]
36 |  (((~UL(0)) - (UL(1) << (l)) + 1) &          |                      ^~
include/linux/bits.h:39:31: note: in expansion of macro '__GENMASK'
39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|                               ^~~~~~~~~
drivers/gpu/drm/msm/msm_iommu.c:54:11: note: in expansion of macro 'GENMASK'
54 |   iova |= GENMASK(63, 49);
|           ^~~~~~~
include/linux/bits.h:37:11: warning: right shift count is negative [-Wshift-count-negative]
37 |   (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
|           ^~
include/linux/bits.h:39:31: note: in expansion of macro '__GENMASK'
39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
|                               ^~~~~~~~~
drivers/gpu/drm/msm/msm_iommu.c:54:11: note: in expansion of macro 'GENMASK'
54 |   iova |= GENMASK(63, 49);
|           ^~~~~~~

vim +/GENMASK +206 drivers/gpu/drm/msm/adreno/adreno_gpu.c

   > 9	#include <linux/ascii85.h>
    10	#include <linux/interconnect.h>
    11	#include <linux/qcom_scm.h>
    12	#include <linux/kernel.h>
    13	#include <linux/of_address.h>
    14	#include <linux/pm_opp.h>
    15	#include <linux/slab.h>
    16	#include <linux/soc/qcom/mdt_loader.h>
    17	#include <soc/qcom/ocmem.h>
    18	#include "adreno_gpu.h"
    19	#include "msm_gem.h"
    20	#include "msm_mmu.h"
    21	
    22	static bool zap_available = true;
    23	
    24	static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
    25			u32 pasid)
    26	{
    27		struct device *dev = &gpu->pdev->dev;
    28		const struct firmware *fw;
    29		const char *signed_fwname = NULL;
    30		struct device_node *np, *mem_np;
    31		struct resource r;
    32		phys_addr_t mem_phys;
    33		ssize_t mem_size;
    34		void *mem_region = NULL;
    35		int ret;
    36	
    37		if (!IS_ENABLED(CONFIG_ARCH_QCOM)) {
    38			zap_available = false;
    39			return -EINVAL;
    40		}
    41	
    42		np = of_get_child_by_name(dev->of_node, "zap-shader");
    43		if (!np) {
    44			zap_available = false;
    45			return -ENODEV;
    46		}
    47	
    48		mem_np = of_parse_phandle(np, "memory-region", 0);
    49		of_node_put(np);
    50		if (!mem_np) {
    51			zap_available = false;
    52			return -EINVAL;
    53		}
    54	
    55		ret = of_address_to_resource(mem_np, 0, &r);
    56		of_node_put(mem_np);
    57		if (ret)
    58			return ret;
    59	
    60		mem_phys = r.start;
    61	
    62		/*
    63		 * Check for a firmware-name property.  This is the new scheme
    64		 * to handle firmware that may be signed with device specific
    65		 * keys, allowing us to have a different zap fw path for different
    66		 * devices.
    67		 *
    68		 * If the firmware-name property is found, we bypass the
    69		 * adreno_request_fw() mechanism, because we don't need to handle
    70		 * the /lib/firmware/qcom/... vs /lib/firmware/... case.
    71		 *
    72		 * If the firmware-name property is not found, for backwards
    73		 * compatibility we fall back to the fwname from the gpulist
    74		 * table.
    75		 */
    76		of_property_read_string_index(np, "firmware-name", 0, &signed_fwname);
    77		if (signed_fwname) {
    78			fwname = signed_fwname;
    79			ret = request_firmware_direct(&fw, fwname, gpu->dev->dev);
    80			if (ret)
    81				fw = ERR_PTR(ret);
    82		} else if (fwname) {
    83			/* Request the MDT file from the default location: */
    84			fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
    85		} else {
    86			/*
    87			 * For new targets, we require the firmware-name property,
    88			 * if a zap-shader is required, rather than falling back
    89			 * to a firmware name specified in gpulist.
    90			 *
    91			 * Because the firmware is signed with a (potentially)
    92			 * device specific key, having the name come from gpulist
    93			 * was a bad idea, and is only provided for backwards
    94			 * compatibility for older targets.
    95			 */
    96			return -ENODEV;
    97		}
    98	
    99		if (IS_ERR(fw)) {
   100			DRM_DEV_ERROR(dev, "Unable to load %s\n", fwname);
   101			return PTR_ERR(fw);
   102		}
   103	
   104		/* Figure out how much memory we need */
   105		mem_size = qcom_mdt_get_size(fw);
   106		if (mem_size < 0) {
   107			ret = mem_size;
   108			goto out;
   109		}
   110	
   111		if (mem_size > resource_size(&r)) {
   112			DRM_DEV_ERROR(dev,
   113				"memory region is too small to load the MDT\n");
   114			ret = -E2BIG;
   115			goto out;
   116		}
   117	
   118		/* Allocate memory for the firmware image */
   119		mem_region = memremap(mem_phys, mem_size,  MEMREMAP_WC);
   120		if (!mem_region) {
   121			ret = -ENOMEM;
   122			goto out;
   123		}
   124	
   125		/*
   126		 * Load the rest of the MDT
   127		 *
   128		 * Note that we could be dealing with two different paths, since
   129		 * with upstream linux-firmware it would be in a qcom/ subdir..
   130		 * adreno_request_fw() handles this, but qcom_mdt_load() does
   131		 * not.  But since we've already gotten through adreno_request_fw()
   132		 * we know which of the two cases it is:
   133		 */
   134		if (signed_fwname || (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY)) {
   135			ret = qcom_mdt_load(dev, fw, fwname, pasid,
   136					mem_region, mem_phys, mem_size, NULL);
   137		} else {
   138			char *newname;
   139	
   140			newname = kasprintf(GFP_KERNEL, "qcom/%s", fwname);
   141	
   142			ret = qcom_mdt_load(dev, fw, newname, pasid,
   143					mem_region, mem_phys, mem_size, NULL);
   144			kfree(newname);
   145		}
   146		if (ret)
   147			goto out;
   148	
   149		/* Send the image to the secure world */
   150		ret = qcom_scm_pas_auth_and_reset(pasid);
   151	
   152		/*
   153		 * If the scm call returns -EOPNOTSUPP we assume that this target
   154		 * doesn't need/support the zap shader so quietly fail
   155		 */
   156		if (ret == -EOPNOTSUPP)
   157			zap_available = false;
   158		else if (ret)
   159			DRM_DEV_ERROR(dev, "Unable to authorize the image\n");
   160	
   161	out:
   162		if (mem_region)
   163			memunmap(mem_region);
   164	
   165		release_firmware(fw);
   166	
   167		return ret;
   168	}
   169	
   170	int adreno_zap_shader_load(struct msm_gpu *gpu, u32 pasid)
   171	{
   172		struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
   173		struct platform_device *pdev = gpu->pdev;
   174	
   175		/* Short cut if we determine the zap shader isn't available/needed */
   176		if (!zap_available)
   177			return -ENODEV;
   178	
   179		/* We need SCM to be able to load the firmware */
   180		if (!qcom_scm_is_available()) {
   181			DRM_DEV_ERROR(&pdev->dev, "SCM is not available\n");
   182			return -EPROBE_DEFER;
   183		}
   184	
   185		return zap_shader_load_mdt(gpu, adreno_gpu->info->zapfw, pasid);
   186	}
   187	
   188	struct msm_gem_address_space *
   189	adreno_iommu_create_address_space(struct msm_gpu *gpu,
   190			struct platform_device *pdev)
   191	{
   192		struct iommu_domain *iommu = iommu_domain_alloc(&platform_bus_type);
   193		struct msm_mmu *mmu = msm_iommu_new(&pdev->dev, iommu);
   194		struct msm_gem_address_space *aspace;
   195		u64 start, size;
   196	
   197		/*
   198		 * Use the aperture start or SZ_16M, whichever is greater. This will
   199		 * ensure that we align with the allocated pagetable range while still
   200		 * allowing room in the lower 32 bits for GMEM and whatnot
   201		 */
   202		start = max_t(u64, SZ_16M, iommu->geometry.aperture_start);
   203		size = iommu->geometry.aperture_end - start + 1;
   204	
   205		aspace = msm_gem_address_space_create(mmu, "gpu",
 > 206			start & GENMASK(48, 0), size);
   207	
   208		if (IS_ERR(aspace) && !IS_ERR(mmu))
   209			mmu->funcs->destroy(mmu);
   210	
   211		return aspace;
   212	}
   213	

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