[Public]
Asking for some advice here.
Coverity throws uninitialized errors (covered below), but at least the first
two (commented below) are explicitly set in the various function calls. Should
we be initializing them anyways, or should we only be doing that for the
variables where there's some possible flow where they can be accessed
uninitialized? Thanks!
Kent
> -----Original Message-----
> From: Martin, Andrew <andrew.mar...@amd.com>
> Sent: Wednesday, January 8, 2025 1:15 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Russell, Kent <kent.russ...@amd.com>; Martin, Andrew
> <andrew.mar...@amd.com>
> Subject: [PATCH v3] drm/amdkfd: Uninitialized and Unused variables
>
> This patch initialized key variables and removed unused ones.
>
> Signed-off-by: Andrew Martin <andrew.mar...@amd.com>
> ---
> .../gpu/drm/amd/amdkfd/cik_event_interrupt.c | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 24 +++++------
> drivers/gpu/drm/amd/amdkfd/kfd_device.c | 2 +-
> .../drm/amd/amdkfd/kfd_device_queue_manager.c | 42 ++++++++-----------
> .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c | 8 ++--
> .../drm/amd/amdkfd/kfd_packet_manager_vi.c | 1 -
> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 14 +++----
> .../amd/amdkfd/kfd_process_queue_manager.c | 6 +--
> 8 files changed, 43 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
> b/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
> index 795382b55e0a..9767f6483871 100644
> --- a/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
> +++ b/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
> @@ -35,7 +35,7 @@ static bool cik_event_interrupt_isr(struct kfd_node *dev,
> (const struct cik_ih_ring_entry *)ih_ring_entry;
> const struct kfd2kgd_calls *f2g = dev->kfd2kgd;
> unsigned int vmid;
> - uint16_t pasid;
> + uint16_t pasid = 0;
This one isn't needed. It's initialized in one of the
get_atc_vmid_pasid_mapping_info calls, or below.
> bool ret;
>
> /* This workaround is due to HW/FW limitation on Hawaii that
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 065d87841459..306ea5a8a747 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -923,7 +923,7 @@ static int kfd_ioctl_get_tile_config(struct file *filep,
> {
> struct kfd_ioctl_get_tile_config_args *args = data;
> struct kfd_process_device *pdd;
> - struct tile_config config;
> + struct tile_config config = {0};
Same with this one, it's set in amdgpu_amdkfd_get_tile_config
> int err = 0;
>
> mutex_lock(&p->mutex);
> @@ -1042,7 +1042,7 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file
> *filep,
> {
> struct kfd_ioctl_alloc_memory_of_gpu_args *args = data;
> struct kfd_process_device *pdd;
> - void *mem;
> + void *mem = NULL;
> struct kfd_node *dev;
> int idr_handle;
> long err;
> @@ -1496,12 +1496,12 @@ static int kfd_ioctl_get_dmabuf_info(struct file
> *filep,
> {
> struct kfd_ioctl_get_dmabuf_info_args *args = data;
> struct kfd_node *dev = NULL;
> - struct amdgpu_device *dmabuf_adev;
> + struct amdgpu_device *dmabuf_adev = NULL;
> void *metadata_buffer = NULL;
> - uint32_t flags;
> - int8_t xcp_id;
> + uint32_t flags = 0;
> + int8_t xcp_id = 0;
> unsigned int i;
> - int r;
> + int r = 0;
>
> /* Find a KFD GPU device that supports the get_dmabuf_info query */
> for (i = 0; kfd_topology_enum_kfd_devices(i, &dev) == 0; i++)
> @@ -1551,7 +1551,7 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
> struct kfd_process_device *pdd;
> int idr_handle;
> uint64_t size;
> - void *mem;
> + void *mem = NULL;
> int r;
>
> mutex_lock(&p->mutex);
> @@ -1837,10 +1837,8 @@ static int criu_get_prime_handle(struct kgd_mem *mem,
> int flags, u32 *shared_fd,
> struct file **file)
> {
> - struct dma_buf *dmabuf;
> - int ret;
> -
> - ret = amdgpu_amdkfd_gpuvm_export_dmabuf(mem, &dmabuf);
> + struct dma_buf *dmabuf = NULL;
> + int ret = amdgpu_amdkfd_gpuvm_export_dmabuf(mem, &dmabuf);
> if (ret) {
> pr_err("dmabuf export failed for the BO\n");
> return ret;
> @@ -2291,7 +2289,7 @@ static int criu_restore_memory_of_gpu(struct
> kfd_process_device *pdd,
> int idr_handle;
> int ret;
> const bool criu_resume = true;
> - u64 offset;
> + u64 offset = 0;
>
> if (bo_bucket->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_DOORBELL)
> {
> if (bo_bucket->size !=
> @@ -2358,7 +2356,7 @@ static int criu_restore_bo(struct kfd_process *p,
> struct file **file)
> {
> struct kfd_process_device *pdd;
> - struct kgd_mem *kgd_mem;
> + struct kgd_mem *kgd_mem = NULL;
> int ret;
> int j;
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index a29374c86405..5af5b9baa894 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -1038,7 +1038,7 @@ void kgd2kfd_suspend(struct kfd_dev *kfd, bool run_pm)
>
> int kgd2kfd_resume(struct kfd_dev *kfd, bool run_pm)
> {
> - int ret, i;
> + int ret = 0, i;
>
> if (!kfd->init_complete)
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 1405e8affd48..d4a346ac1167 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -199,7 +199,7 @@ static int add_queue_mes(struct device_queue_manager
> *dqm, struct queue *q,
> struct amdgpu_device *adev = (struct amdgpu_device *)dqm->dev->adev;
> struct kfd_process_device *pdd = qpd_to_pdd(qpd);
> struct mes_add_queue_input queue_input;
> - int r, queue_type;
> + int r = 0, queue_type;
> uint64_t wptr_addr_off;
>
> if (!dqm->sched_running || dqm->sched_halt)
> @@ -284,7 +284,7 @@ static int remove_queue_mes(struct
> device_queue_manager *dqm, struct queue *q,
> struct qcm_process_device *qpd)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)dqm->dev->adev;
> - int r;
> + int r = 0;
> struct mes_remove_queue_input queue_input;
>
> if (!dqm->sched_running || dqm->sched_halt)
> @@ -634,7 +634,7 @@ static int create_queue_nocpsch(struct
> device_queue_manager *dqm,
> const void *restore_mqd, const void
> *restore_ctl_stack)
> {
> struct mqd_manager *mqd_mgr;
> - int retval;
> + int retval = 0;
>
> dqm_lock(dqm);
>
> @@ -795,7 +795,7 @@ static int dbgdev_wave_reset_wavefronts(struct kfd_node
> *dev, struct kfd_process
> {
> int status = 0;
> unsigned int vmid;
> - uint16_t queried_pasid;
> + uint16_t queried_pasid = 0;
> union SQ_CMD_BITS reg_sq_cmd;
> union GRBM_GFX_INDEX_BITS reg_gfx_index;
> struct kfd_process_device *pdd;
> @@ -862,11 +862,9 @@ static int destroy_queue_nocpsch_locked(struct
> device_queue_manager *dqm,
> struct qcm_process_device *qpd,
> struct queue *q)
> {
> - int retval;
> - struct mqd_manager *mqd_mgr;
> -
> - mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> - q->properties.type)];
> + int retval = 0;
> + struct mqd_manager *mqd_mgr =
> + dqm->mqd_mgrs[get_mqd_type_from_queue_type(q->properties.type)];
>
> if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE)
> deallocate_hqd(dqm, q);
> @@ -921,7 +919,7 @@ static int destroy_queue_nocpsch(struct
> device_queue_manager *dqm,
> struct qcm_process_device *qpd,
> struct queue *q)
> {
> - int retval;
> + int retval = 0;
> uint64_t sdma_val = 0;
> struct device *dev = dqm->dev->adev->dev;
> struct kfd_process_device *pdd = qpd_to_pdd(qpd);
> @@ -1150,7 +1148,7 @@ static int evict_process_queues_nocpsch(struct
> device_queue_manager *dqm,
> struct queue *q;
> struct mqd_manager *mqd_mgr;
> struct kfd_process_device *pdd;
> - int retval, ret = 0;
> + int retval = 0, ret = 0;
>
> dqm_lock(dqm);
> if (qpd->evicted++ > 0) /* already evicted, do nothing */
> @@ -1260,7 +1258,7 @@ static int restore_process_queues_nocpsch(struct
> device_queue_manager *dqm,
> struct kfd_process_device *pdd;
> uint64_t pd_base;
> uint64_t eviction_duration;
> - int retval, ret = 0;
> + int retval = 0, ret = 0;
>
> pdd = qpd_to_pdd(qpd);
> /* Retrieve PD base */
> @@ -1437,13 +1435,12 @@ static int register_process(struct
> device_queue_manager *dqm,
> static int unregister_process(struct device_queue_manager *dqm,
> struct qcm_process_device *qpd)
> {
> - int retval;
> + int retval = 0;
> struct device_process_node *cur, *next;
>
> pr_debug("qpd->queues_list is %s\n",
> list_empty(&qpd->queues_list) ? "empty" : "not empty");
>
> - retval = 0;
> dqm_lock(dqm);
>
> list_for_each_entry_safe(cur, next, &dqm->queues, list) {
> @@ -1473,7 +1470,7 @@ set_pasid_vmid_mapping(struct device_queue_manager
> *dqm, u32 pasid,
> unsigned int vmid)
> {
> uint32_t xcc_mask = dqm->dev->xcc_mask;
> - int xcc_id, ret;
> + int xcc_id, ret = 0;
>
> for_each_inst(xcc_id, xcc_mask) {
> ret = dqm->dev->kfd2kgd->set_pasid_vmid_mapping(
> @@ -1827,8 +1824,6 @@ static int start_cpsch(struct device_queue_manager
> *dqm)
> struct device *dev = dqm->dev->adev->dev;
> int retval, num_hw_queue_slots;
>
> - retval = 0;
> -
> dqm_lock(dqm);
>
> if (!dqm->dev->kfd->shared_resources.enable_mes) {
> @@ -1988,7 +1983,7 @@ static int create_queue_cpsch(struct
> device_queue_manager *dqm, struct queue *q,
> const struct kfd_criu_queue_priv_data *qd,
> const void *restore_mqd, const void *restore_ctl_stack)
> {
> - int retval;
> + int retval = 0;
> struct mqd_manager *mqd_mgr;
>
> if (dqm->total_queue_count >= max_num_of_queues_per_device) {
> @@ -2282,8 +2277,8 @@ static int unmap_queues_cpsch(struct
> device_queue_manager *dqm,
> bool reset)
> {
> struct device *dev = dqm->dev->adev->dev;
> - struct mqd_manager *mqd_mgr;
> - int retval;
> + struct mqd_manager *mqd_mgr = NULL;
> + int retval = 0;
>
> if (!dqm->sched_running)
> return 0;
> @@ -2705,7 +2700,7 @@ static int checkpoint_mqd(struct device_queue_manager
> *dqm,
> static int process_termination_cpsch(struct device_queue_manager *dqm,
> struct qcm_process_device *qpd)
> {
> - int retval;
> + int retval = 0;
> struct queue *q;
> struct device *dev = dqm->dev->adev->dev;
> struct kernel_queue *kq, *kq_next;
> @@ -2715,8 +2710,6 @@ static int process_termination_cpsch(struct
> device_queue_manager *dqm,
> KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES;
> bool found = false;
>
> - retval = 0;
> -
> dqm_lock(dqm);
>
> /* Clean all kernel queues */
> @@ -3439,7 +3432,6 @@ int suspend_queues(struct kfd_process *p,
> else
> per_device_suspended++;
> } else if (err != -EBUSY) {
> - r = err;
> queue_ids[q_idx] |=
> KFD_DBG_QUEUE_ERROR_MASK;
> break;
> }
> @@ -3647,7 +3639,7 @@ int dqm_debugfs_hqds(struct seq_file *m, void *data)
> {
> struct device_queue_manager *dqm = data;
> uint32_t xcc_mask = dqm->dev->xcc_mask;
> - uint32_t (*dump)[2], n_regs;
> + uint32_t (*dump)[2] = {}, n_regs = 0;
> int pipe, queue;
> int r = 0, xcc_id;
> uint32_t sdma_engine_start;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> index ff417d5361c4..55fc98358999 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> @@ -571,7 +571,7 @@ static int hiq_load_mqd_kiq_v9_4_3(struct mqd_manager
> *mm, void *mqd,
> struct queue_properties *p, struct mm_struct *mms)
> {
> uint32_t xcc_mask = mm->dev->xcc_mask;
> - int xcc_id, err, inst = 0;
> + int xcc_id, err = 0, inst = 0;
> void *xcc_mqd;
> uint64_t hiq_mqd_size = kfd_hiq_mqd_stride(mm->dev);
>
> @@ -595,7 +595,7 @@ static int destroy_hiq_mqd_v9_4_3(struct mqd_manager
> *mm, void *mqd,
> uint32_t pipe_id, uint32_t queue_id)
> {
> uint32_t xcc_mask = mm->dev->xcc_mask;
> - int xcc_id, err, inst = 0;
> + int xcc_id, err = 0, inst = 0;
> uint64_t hiq_mqd_size = kfd_hiq_mqd_stride(mm->dev);
> struct v9_mqd *m;
> u32 doorbell_off;
> @@ -751,7 +751,7 @@ static int destroy_mqd_v9_4_3(struct mqd_manager *mm,
> void *mqd,
> uint32_t pipe_id, uint32_t queue_id)
> {
> uint32_t xcc_mask = mm->dev->xcc_mask;
> - int xcc_id, err, inst = 0;
> + int xcc_id, err = 0, inst = 0;
> void *xcc_mqd;
> struct v9_mqd *m;
> uint64_t mqd_offset;
> @@ -781,7 +781,7 @@ static int load_mqd_v9_4_3(struct mqd_manager *mm, void
> *mqd,
> /* AQL write pointer counts in 64B packets, PM4/CP counts in dwords. */
> uint32_t wptr_shift = (p->format == KFD_QUEUE_FORMAT_AQL ? 4 : 0);
> uint32_t xcc_mask = mm->dev->xcc_mask;
> - int xcc_id, err, inst = 0;
> + int xcc_id, err = 0, inst = 0;
> void *xcc_mqd;
> uint64_t mqd_stride_size = mm->mqd_stride(mm, p);
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
> index c1199d06d131..cf803aed5069 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
> @@ -173,7 +173,6 @@ static int pm_map_queues_vi(struct packet_manager *pm,
> uint32_t *buffer,
> case KFD_QUEUE_TYPE_SDMA_XGMI:
> packet->bitfields2.engine_sel = q->properties.sdma_engine_id +
> engine_sel__mes_map_queues__sdma0_vi;
> - use_static = false; /* no static queues under SDMA */
> break;
> default:
> WARN(1, "queue type %d", q->properties.type);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 083f83c94531..a6188c06c7a1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -272,7 +272,7 @@ static int kfd_get_cu_occupancy(struct attribute *attr,
> char
> *buffer)
> struct kfd_process_device *pdd = NULL;
> int i;
> struct kfd_cu_occupancy *cu_occupancy;
> - u32 queue_format;
> + u32 queue_format = 0;
>
> pdd = container_of(attr, struct kfd_process_device, attr_cu_occupancy);
> dev = pdd->dev;
> @@ -743,9 +743,7 @@ static int kfd_process_alloc_gpuvm(struct
> kfd_process_device *pdd,
> uint32_t flags, struct kgd_mem **mem, void
> **kptr)
> {
> struct kfd_node *kdev = pdd->dev;
> - int err;
> -
> - err = amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(kdev->adev, gpu_va,
> size,
> + int err = amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(kdev->adev,
> gpu_va, size,
> pdd->drm_priv, mem, NULL,
> flags, false);
> if (err)
> @@ -798,8 +796,8 @@ static int kfd_process_device_reserve_ib_mem(struct
> kfd_process_device *pdd)
> KFD_IOC_ALLOC_MEM_FLAGS_NO_SUBSTITUTE |
> KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE |
> KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
> - struct kgd_mem *mem;
> - void *kaddr;
> + struct kgd_mem *mem = NULL;
> + void *kaddr = NULL;
> int ret;
>
> if (qpd->ib_kaddr || !qpd->ib_base)
> @@ -1369,8 +1367,8 @@ static int kfd_process_device_init_cwsr_dgpu(struct
> kfd_process_device *pdd)
> uint32_t flags = KFD_IOC_ALLOC_MEM_FLAGS_GTT
> | KFD_IOC_ALLOC_MEM_FLAGS_NO_SUBSTITUTE
> | KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
> - struct kgd_mem *mem;
> - void *kaddr;
> + struct kgd_mem *mem = NULL;
> + void *kaddr = NULL;
> int ret;
>
> if (!dev->kfd->cwsr_enabled || qpd->cwsr_kaddr || !qpd->cwsr_base)
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index 9df56f8e09f9..06fbb107f303 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -769,7 +769,7 @@ int kfd_process_get_queue_info(struct kfd_process *p,
> if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE ||
> q->properties.type == KFD_QUEUE_TYPE_SDMA ||
> q->properties.type ==
> KFD_QUEUE_TYPE_SDMA_XGMI) {
> - uint32_t mqd_size, ctl_stack_size;
> + uint32_t mqd_size = 0, ctl_stack_size = 0;
>
> *num_queues = *num_queues + 1;
>
> @@ -872,8 +872,8 @@ static int criu_checkpoint_queues_device(struct
> kfd_process_device *pdd,
> list_for_each_entry(q, &pdd->qpd.queues_list, list) {
> struct kfd_criu_queue_priv_data *q_data;
> uint64_t q_data_size;
> - uint32_t mqd_size;
> - uint32_t ctl_stack_size;
> + uint32_t mqd_size = 0;
> + uint32_t ctl_stack_size = 0;
>
> if (q->properties.type != KFD_QUEUE_TYPE_COMPUTE &&
> q->properties.type != KFD_QUEUE_TYPE_SDMA &&
> --
> 2.43.0
