On Tue, Feb 18, 2020 at 04:46:21PM -0500, Luben Tuikov wrote: > On 2020-02-17 10:08 a.m., Christian König wrote: > > Am 17.02.20 um 15:44 schrieb Alex Deucher: > >> On Fri, Feb 14, 2020 at 7:17 PM Luben Tuikov <luben.tuikov@xxxxxxx> wrote: > >>> Add a AMDGPU_GEM_CREATE_MASK and use it to check > >>> for valid/invalid GEM create flags coming in from > >>> userspace. > >>> > >>> Fix a bug in checking whether TMZ is supported at > >>> GEM create time. > >>> > >>> Signed-off-by: Luben Tuikov <luben.tuikov@xxxxxxx> > >>> --- > >>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 11 ++--------- > >>> include/uapi/drm/amdgpu_drm.h | 2 ++ > >>> 2 files changed, 4 insertions(+), 9 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > >>> index b51a060c637d..74bb79e64fa3 100644 > >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > >>> @@ -221,21 +221,14 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data, > >>> int r; > >>> > >>> /* reject invalid gem flags */ > >>> - if (flags & ~(AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED | > >>> - AMDGPU_GEM_CREATE_NO_CPU_ACCESS | > >>> - AMDGPU_GEM_CREATE_CPU_GTT_USWC | > >>> - AMDGPU_GEM_CREATE_VRAM_CLEARED | > >>> - AMDGPU_GEM_CREATE_VM_ALWAYS_VALID | > >>> - AMDGPU_GEM_CREATE_EXPLICIT_SYNC | > >>> - AMDGPU_GEM_CREATE_ENCRYPTED)) > >>> - > >> I'd rather keep the list explicit so no one ends up forgetting to > >> update the mask the next time new flags are added. > > > > Additional to that this patch is bogus. > > So the only "additional" thing you're contributing to the review of > this patch is calling it "bogus". Characterizing the patch with an adjective > as "bogus" is very disturbing. What's next? > > > > > We intentionally filter out the flags here which userspace is allowed to > > specify in the GEM interface, but after this patch we would allow all > > flags to be specified. > > I thought that that could be the case. But in your review why not > recommend we have a mask to check user-settable flags? So the > actual flags are collected and visible in one place and well > understood that that is what is being checked and well-defined > by a macro with an appropriate name? > > Why did NO ONE comment on the bug fix below? No one. > I missed the bugfix patch before, sorry. (There are many patches from amd-gfx one day, if you didn't cc me, the mail may be missed). So I found the issue after sync up with drm-next and then give the fix. Hope you can understand and don't take it to heart. Thanks, Ray _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
