| Message ID | 20210609043613.102962-31-jason@jlekstrand.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gem: ioctl clean-ups (v6) | expand |
Hi Jason,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-tip/drm-tip]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next drm/drm-next v5.13-rc5 next-20210608]
[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/Jason-Ekstrand/drm-i915-gem-ioctl-clean-ups-v6/20210609-123926
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-r011-20210608 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project d2012d965d60c3258b3a69d024491698f8aec386)
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
# https://github.com/0day-ci/linux/commit/48a06ef4f7f3fbb4b4768b25829796cf235de32c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jason-Ekstrand/drm-i915-gem-ioctl-clean-ups-v6/20210609-123926
git checkout 48a06ef4f7f3fbb4b4768b25829796cf235de32c
# 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 errors (new ones prefixed by >>):
drivers/gpu/drm/i915/gem/i915_gem_context.c:1910:1: error: no previous prototype for function 'finalize_create_context_locked' [-Werror,-Wmissing-prototypes]
finalize_create_context_locked(struct drm_i915_file_private *file_priv,
^
drivers/gpu/drm/i915/gem/i915_gem_context.c:1909:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
struct i915_gem_context *
^
static
>> drivers/gpu/drm/i915/gem/i915_gem_context.c:2007:45: error: variable 'id' is uninitialized when used here [-Werror,-Wuninitialized]
gem_context_register(ctx, ext_data.fpriv, id);
^~
drivers/gpu/drm/i915/gem/i915_gem_context.c:1964:8: note: initialize the variable 'id' to silence this warning
u32 id;
^
= 0
2 errors generated.
vim +/id +2007 drivers/gpu/drm/i915/gem/i915_gem_context.c
1908
1909 struct i915_gem_context *
> 1910 finalize_create_context_locked(struct drm_i915_file_private *file_priv,
1911 struct i915_gem_proto_context *pc, u32 id)
1912 {
1913 struct i915_gem_context *ctx;
1914 void *old;
1915
1916 lockdep_assert_held(&file_priv->proto_context_lock);
1917
1918 ctx = i915_gem_create_context(file_priv->dev_priv, pc);
1919 if (IS_ERR(ctx))
1920 return ctx;
1921
1922 gem_context_register(ctx, file_priv, id);
1923
1924 old = xa_erase(&file_priv->proto_context_xa, id);
1925 GEM_BUG_ON(old != pc);
1926 proto_context_close(pc);
1927
1928 /* One for the xarray and one for the caller */
1929 return i915_gem_context_get(ctx);
1930 }
1931
1932 struct i915_gem_context *
1933 i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
1934 {
1935 struct i915_gem_proto_context *pc;
1936 struct i915_gem_context *ctx;
1937
1938 ctx = __context_lookup(file_priv, id);
1939 if (ctx)
1940 return ctx;
1941
1942 mutex_lock(&file_priv->proto_context_lock);
1943 /* Try one more time under the lock */
1944 ctx = __context_lookup(file_priv, id);
1945 if (!ctx) {
1946 pc = xa_load(&file_priv->proto_context_xa, id);
1947 if (!pc)
1948 ctx = ERR_PTR(-ENOENT);
1949 else
1950 ctx = finalize_create_context_locked(file_priv, pc, id);
1951 }
1952 mutex_unlock(&file_priv->proto_context_lock);
1953
1954 return ctx;
1955 }
1956
1957 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
1958 struct drm_file *file)
1959 {
1960 struct drm_i915_private *i915 = to_i915(dev);
1961 struct drm_i915_gem_context_create_ext *args = data;
1962 struct create_ext ext_data;
1963 int ret;
1964 u32 id;
1965
1966 if (!DRIVER_CAPS(i915)->has_logical_contexts)
1967 return -ENODEV;
1968
1969 if (args->flags & I915_CONTEXT_CREATE_FLAGS_UNKNOWN)
1970 return -EINVAL;
1971
1972 ret = intel_gt_terminally_wedged(&i915->gt);
1973 if (ret)
1974 return ret;
1975
1976 ext_data.fpriv = file->driver_priv;
1977 if (client_is_banned(ext_data.fpriv)) {
1978 drm_dbg(&i915->drm,
1979 "client %s[%d] banned from creating ctx\n",
1980 current->comm, task_pid_nr(current));
1981 return -EIO;
1982 }
1983
1984 ext_data.pc = proto_context_create(i915, args->flags);
1985 if (IS_ERR(ext_data.pc))
1986 return PTR_ERR(ext_data.pc);
1987
1988 if (args->flags & I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS) {
1989 ret = i915_user_extensions(u64_to_user_ptr(args->extensions),
1990 create_extensions,
1991 ARRAY_SIZE(create_extensions),
1992 &ext_data);
1993 if (ret)
1994 goto err_pc;
1995 }
1996
1997 if (GRAPHICS_VER(i915) > 12) {
1998 struct i915_gem_context *ctx;
1999
2000 ctx = i915_gem_create_context(i915, ext_data.pc);
2001 if (IS_ERR(ctx)) {
2002 ret = PTR_ERR(ctx);
2003 goto err_pc;
2004 }
2005
2006 proto_context_close(ext_data.pc);
> 2007 gem_context_register(ctx, ext_data.fpriv, id);
2008 } else {
2009 ret = proto_context_register(ext_data.fpriv, ext_data.pc, &id);
2010 if (ret < 0)
2011 goto err_pc;
2012 }
2013
2014 args->ctx_id = id;
2015 drm_dbg(&i915->drm, "HW context %d created\n", args->ctx_id);
2016
2017 return 0;
2018
2019 err_pc:
2020 proto_context_close(ext_data.pc);
2021 return ret;
2022 }
2023
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Tue, Jun 08, 2021 at 11:36:12PM -0500, Jason Ekstrand wrote: > All the proto-context stuff for context creation exists to allow older > userspace drivers to set VMs and engine sets via SET_CONTEXT_PARAM. > Drivers need to update to use CONTEXT_CREATE_EXT_* for this going > forward. Force the issue by blocking the old mechanism on any future > hardware generations. > > Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> With that static added here (same ofc holds for the other one 0day spotted): Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> But also I think an ack from Jon Bloomfield here would be needed, plus Cc Carl Zhang and Micheal Mrozek to get their acks too pls. Also I'm assuming you've tested this with your igt changes (change the condition to GFX_VER > 11 in a trybot run) and it all works? -Daniel > --- > drivers/gpu/drm/i915/gem/i915_gem_context.c | 33 +++++++++++++++------ > 1 file changed, 24 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c > index d3c9c42dcae4d..5312142daa0c0 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c > @@ -1994,9 +1994,22 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, > goto err_pc; > } > > - ret = proto_context_register(ext_data.fpriv, ext_data.pc, &id); > - if (ret < 0) > - goto err_pc; > + if (GRAPHICS_VER(i915) > 12) { > + struct i915_gem_context *ctx; > + > + ctx = i915_gem_create_context(i915, ext_data.pc); > + if (IS_ERR(ctx)) { > + ret = PTR_ERR(ctx); > + goto err_pc; > + } > + > + proto_context_close(ext_data.pc); > + gem_context_register(ctx, ext_data.fpriv, id); > + } else { > + ret = proto_context_register(ext_data.fpriv, ext_data.pc, &id); > + if (ret < 0) > + goto err_pc; > + } > > args->ctx_id = id; > drm_dbg(&i915->drm, "HW context %d created\n", args->ctx_id); > @@ -2179,15 +2192,17 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data, > mutex_lock(&file_priv->proto_context_lock); > ctx = __context_lookup(file_priv, args->ctx_id); > if (!ctx) { > - /* FIXME: We should consider disallowing SET_CONTEXT_PARAM > - * for most things on future platforms. Clients should be > - * using CONTEXT_CREATE_EXT_PARAM instead. > - */ > pc = xa_load(&file_priv->proto_context_xa, args->ctx_id); > - if (pc) > + if (pc) { > + /* Contexts should be finalized inside > + * GEM_CONTEXT_CREATE starting with graphics > + * version 13. > + */ > + WARN_ON(GRAPHICS_VER(file_priv->dev_priv) > 12); > ret = set_proto_ctx_param(file_priv, pc, args); > - else > + } else { > ret = -ENOENT; > + } > } > mutex_unlock(&file_priv->proto_context_lock); > > -- > 2.31.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index d3c9c42dcae4d..5312142daa0c0 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -1994,9 +1994,22 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, goto err_pc; } - ret = proto_context_register(ext_data.fpriv, ext_data.pc, &id); - if (ret < 0) - goto err_pc; + if (GRAPHICS_VER(i915) > 12) { + struct i915_gem_context *ctx; + + ctx = i915_gem_create_context(i915, ext_data.pc); + if (IS_ERR(ctx)) { + ret = PTR_ERR(ctx); + goto err_pc; + } + + proto_context_close(ext_data.pc); + gem_context_register(ctx, ext_data.fpriv, id); + } else { + ret = proto_context_register(ext_data.fpriv, ext_data.pc, &id); + if (ret < 0) + goto err_pc; + } args->ctx_id = id; drm_dbg(&i915->drm, "HW context %d created\n", args->ctx_id); @@ -2179,15 +2192,17 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data, mutex_lock(&file_priv->proto_context_lock); ctx = __context_lookup(file_priv, args->ctx_id); if (!ctx) { - /* FIXME: We should consider disallowing SET_CONTEXT_PARAM - * for most things on future platforms. Clients should be - * using CONTEXT_CREATE_EXT_PARAM instead. - */ pc = xa_load(&file_priv->proto_context_xa, args->ctx_id); - if (pc) + if (pc) { + /* Contexts should be finalized inside + * GEM_CONTEXT_CREATE starting with graphics + * version 13. + */ + WARN_ON(GRAPHICS_VER(file_priv->dev_priv) > 12); ret = set_proto_ctx_param(file_priv, pc, args); - else + } else { ret = -ENOENT; + } } mutex_unlock(&file_priv->proto_context_lock);
All the proto-context stuff for context creation exists to allow older userspace drivers to set VMs and engine sets via SET_CONTEXT_PARAM. Drivers need to update to use CONTEXT_CREATE_EXT_* for this going forward. Force the issue by blocking the old mechanism on any future hardware generations. Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 33 +++++++++++++++------ 1 file changed, 24 insertions(+), 9 deletions(-)