RE: [PATCH v2] drm/amdgpu: Fix possible NULL dereference in amdgpu_ras_query_error_status_helper()
[AMD Official Use Only - General] Reviewed-by: Hawking Zhang Regards, Hawking -Original Message- From: SHANMUGAM, SRINIVASAN Sent: Wednesday, December 27, 2023 12:37 To: Deucher, Alexander ; Koenig, Christian Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN ; Zhang, Hawking ; Zhou1, Tao Subject: [PATCH v2] drm/amdgpu: Fix possible NULL dereference in amdgpu_ras_query_error_status_helper() Return invalid error code -EINVAL for invalid block id. Fixes the below: drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1183 amdgpu_ras_query_error_status_helper() error: we previously assumed 'info' could be null (see line 1176) Suggested-by: Hawking Zhang Cc: Tao Zhou Cc: Hawking Zhang Cc: Christian König Cc: Alex Deucher Signed-off-by: Srinivasan Shanmugam --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index bad62141f708..9c4db031e5ee 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1176,6 +1176,9 @@ static int amdgpu_ras_query_error_status_helper(struct amdgpu_device *adev, enum amdgpu_ras_block blk = info ? info->head.block : AMDGPU_RAS_BLOCK_COUNT; struct amdgpu_ras_block_object *block_obj = NULL; + if (blk == AMDGPU_RAS_BLOCK_COUNT) + return -EINVAL; + if (error_query_mode == AMDGPU_RAS_INVALID_ERROR_QUERY) return -EINVAL; -- 2.34.1
[PATCH v2] drm/amdgpu: Fix possible NULL dereference in amdgpu_ras_query_error_status_helper()
Return invalid error code -EINVAL for invalid block id. Fixes the below: drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1183 amdgpu_ras_query_error_status_helper() error: we previously assumed 'info' could be null (see line 1176) Suggested-by: Hawking Zhang Cc: Tao Zhou Cc: Hawking Zhang Cc: Christian König Cc: Alex Deucher Signed-off-by: Srinivasan Shanmugam --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index bad62141f708..9c4db031e5ee 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1176,6 +1176,9 @@ static int amdgpu_ras_query_error_status_helper(struct amdgpu_device *adev, enum amdgpu_ras_block blk = info ? info->head.block : AMDGPU_RAS_BLOCK_COUNT; struct amdgpu_ras_block_object *block_obj = NULL; + if (blk == AMDGPU_RAS_BLOCK_COUNT) + return -EINVAL; + if (error_query_mode == AMDGPU_RAS_INVALID_ERROR_QUERY) return -EINVAL; -- 2.34.1
Re: [PATCH] Removing duplicate copyright text
Hello Bagas, I'm sorry for that. I will send another mail of this patch with your recommendations. Em ter., 26 de dez. de 2023 23:30, Bagas Sanjaya escreveu: > On Tue, Dec 26, 2023 at 08:57:41PM -0300, Marcelo Mendes Spessoto Junior > wrote: > > Signed-off-by: Marcelo Mendes Spessoto Junior < > marcelomspess...@gmail.com> > > > > The file display/modules/inc/mod_freesync.h has two identical AMD > > Copyright texts. This simple patch aims to remove the duplicate one. > > Hi Marcelo, > > The patch subject should have a subsystem prefix (e.g. the full subject > should have been "[PATCH] drm/amdgpu: mod_freesync: Remove duplicate > copyright boilerplate"). > > For patch description, I'd like to write it as "mod_freesync header file > has duplicated copyright boilerplate. Drop the duplicate". And make > sure that your Signed-off-by: trailer is on the bottom of description, > before triple dashes (`git commit -s` does it for you). > > > > > --- > > .../amd/display/modules/inc/mod_freesync.h| 28 --- > > 1 file changed, 28 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h > b/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h > > index afe1f6cce..cc3dc9b58 100644 > > --- a/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h > > +++ b/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h > > @@ -1,31 +1,3 @@ > > -/* > > - * Copyright 2016 Advanced Micro Devices, Inc. > > - * > > - * Permission is hereby granted, free of charge, to any person > obtaining a > > - * copy of this software and associated documentation files (the > "Software"), > > - * to deal in the Software without restriction, including without > limitation > > - * the rights to use, copy, modify, merge, publish, distribute, > sublicense, > > - * and/or sell copies of the Software, and to permit persons to whom the > > - * Software is furnished to do so, subject to the following conditions: > > - * > > - * The above copyright notice and this permission notice shall be > included in > > - * all copies or substantial portions of the Software. > > - * > > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > EXPRESS OR > > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > MERCHANTABILITY, > > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT > SHALL > > - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, > DAMAGES OR > > - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > > - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > > - * OTHER DEALINGS IN THE SOFTWARE. > > - * > > - * Authors: AMD > > - * > > - */ > > - > > - > > - > > - > > /* > > * Copyright 2016 Advanced Micro Devices, Inc. > > * > > The diff itself LGTM. > > Thanks. > > -- > An old man doll... just what I always wanted! - Clara >
Re: [PATCH v2 0/1] Implementation of resource_query_layout
On Thu, Dec 21, 2023 at 2:01 AM Julia Zhang wrote: > Hi all, > > Sorry to late reply. This is v2 of the implementation of > resource_query_layout. This adds a new ioctl to let guest query information > of host resource, which is originally from Daniel Stone. We add some > changes to support query the correct stride of host resource before it's > created, which is to support to blit data from dGPU to virtio iGPU for dGPU > prime feature. > > Changes from v1 to v2: > -Squash two patches to a single patch. > -A small modification of VIRTIO_GPU_F_RESOURCE_QUERY_LAYOUT > > > Below is description of v1: > This add implementation of resource_query_layout to get the information of > how the host has actually allocated the buffer. This function is now used > to query the stride for guest linear resource for dGPU prime on guest VMs. > You can use a context specific protocol or even the virgl capabilities [for a linear strided resource]. For example, Sommelier does the following: https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/vm_tools/sommelier/virtualization/virtgpu_channel.cc#549 i.e, you should be able to avoid extra ioctl + hypercall. > > v1 of kernel side: > https: > // > lore.kernel.org/xen-devel/20231110074027.24862-1-julia.zh...@amd.com/T/#t > > v1 of qemu side: > https: > // > lore.kernel.org/qemu-devel/20231110074027.24862-1-julia.zh...@amd.com/T/#t > > Daniel Stone (1): > drm/virtio: Implement RESOURCE_GET_LAYOUT ioctl > > drivers/gpu/drm/virtio/virtgpu_drv.c | 1 + > drivers/gpu/drm/virtio/virtgpu_drv.h | 22 - > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 66 ++ > drivers/gpu/drm/virtio/virtgpu_kms.c | 8 +++- > drivers/gpu/drm/virtio/virtgpu_vq.c| 63 > include/uapi/drm/virtgpu_drm.h | 21 > include/uapi/linux/virtio_gpu.h| 30 > 7 files changed, 208 insertions(+), 3 deletions(-) > > -- > 2.34.1 > >
RE: [PATCH] drm/amdgpu: Fix possible NULL dereference in amdgpu_ras_query_error_status_helper()
[AMD Official Use Only - General] Invalid block id should return invalid error code. Regards, Hawking -Original Message- From: SHANMUGAM, SRINIVASAN Sent: Tuesday, December 26, 2023 18:04 To: Deucher, Alexander ; Koenig, Christian Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN ; Zhou1, Tao ; Zhang, Hawking Subject: [PATCH] drm/amdgpu: Fix possible NULL dereference in amdgpu_ras_query_error_status_helper() Return 0, if blk is >= AMDGPU_RAS_BLOCK_COUNT Fixes the below: drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1183 amdgpu_ras_query_error_status_helper() error: we previously assumed 'info' could be null (see line 1176) Cc: Tao Zhou Cc: Hawking Zhang Cc: Christian König Cc: Alex Deucher Signed-off-by: Srinivasan Shanmugam --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index bad62141f708..327415a15b05 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1176,6 +1176,9 @@ static int amdgpu_ras_query_error_status_helper(struct amdgpu_device *adev, enum amdgpu_ras_block blk = info ? info->head.block : AMDGPU_RAS_BLOCK_COUNT; struct amdgpu_ras_block_object *block_obj = NULL; + if (blk >= AMDGPU_RAS_BLOCK_COUNT) + return 0; + if (error_query_mode == AMDGPU_RAS_INVALID_ERROR_QUERY) return -EINVAL; -- 2.34.1
[PATCH] Removing duplicate copyright text
Signed-off-by: Marcelo Mendes Spessoto Junior The file display/modules/inc/mod_freesync.h has two identical AMD Copyright texts. This simple patch aims to remove the duplicate one. --- .../amd/display/modules/inc/mod_freesync.h| 28 --- 1 file changed, 28 deletions(-) diff --git a/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h b/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h index afe1f6cce..cc3dc9b58 100644 --- a/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h +++ b/drivers/gpu/drm/amd/display/modules/inc/mod_freesync.h @@ -1,31 +1,3 @@ -/* - * Copyright 2016 Advanced Micro Devices, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR - * OTHER DEALINGS IN THE SOFTWARE. - * - * Authors: AMD - * - */ - - - - /* * Copyright 2016 Advanced Micro Devices, Inc. * -- 2.39.2
[PATCH] drm/amdgpu: Fix possible NULL dereference in amdgpu_ras_query_error_status_helper()
Return 0, if blk is >= AMDGPU_RAS_BLOCK_COUNT Fixes the below: drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1183 amdgpu_ras_query_error_status_helper() error: we previously assumed 'info' could be null (see line 1176) Cc: Tao Zhou Cc: Hawking Zhang Cc: Christian König Cc: Alex Deucher Signed-off-by: Srinivasan Shanmugam --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index bad62141f708..327415a15b05 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1176,6 +1176,9 @@ static int amdgpu_ras_query_error_status_helper(struct amdgpu_device *adev, enum amdgpu_ras_block blk = info ? info->head.block : AMDGPU_RAS_BLOCK_COUNT; struct amdgpu_ras_block_object *block_obj = NULL; + if (blk >= AMDGPU_RAS_BLOCK_COUNT) + return 0; + if (error_query_mode == AMDGPU_RAS_INVALID_ERROR_QUERY) return -EINVAL; -- 2.34.1
[PATCH v3] drm/amdgpu: Release 'adev->pm.fw' before return in 'amdgpu_device_need_post()'
In function 'amdgpu_device_need_post(struct amdgpu_device *adev)' - 'adev->pm.fw' may not be released before return. Using the function release_firmware() to release adev->pm.fw. Thus fixing the below: drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:1571 amdgpu_device_need_post() warn: 'adev->pm.fw' from request_firmware() not released on lines: 1554. Suggested-by: Lijo Lazar Cc: Monk Liu Cc: Christian König Cc: Alex Deucher Signed-off-by: Srinivasan Shanmugam --- v2: release fw needs to be done only FIJI ASIC (Lijo) drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 4b694696930e..33f37efaf373 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1544,6 +1544,7 @@ bool amdgpu_device_need_post(struct amdgpu_device *adev) return true; fw_ver = *((uint32_t *)adev->pm.fw->data + 69); + release_firmware(adev->pm.fw); if (fw_ver < 0x00160e00) return true; } -- 2.34.1
