From cb8003d9e0be4e82eca8672ebf95382c99f30c5b Mon Sep 17 00:00:00 2001 From: tkudav Date: Fri, 27 Dec 2019 17:23:15 +0530 Subject: [PATCH] gpu: nvgpu: Modify common.top for better coverage Following changes are done to common.top code and UT: 1.Change return type for device_info_parse_enum to void as it can never return non-zero value. - This is a private HAL and is only called by get_device_info HAL. - It gets called only for table entry with entry type = enum. - So there is no error path left. This helps remove unnecessary branches and get better branch coverage 2. Check if the data parsing function pointers are not NULL before parsing the device tree. Return error if there are no functions to interpret the device_info table registers. Add checks for same in unit test test_get_device_info(). JIRA NVGPU-2204 Change-Id: I8833da7aa58b070d19b50ee17f64362f301bd792 Signed-off-by: tkudav Reviewed-on: https://git-master.nvidia.com/r/2269603 Reviewed-by: mobile promotions Tested-by: mobile promotions --- drivers/gpu/nvgpu/hal/top/top_gm20b.c | 8 +----- drivers/gpu/nvgpu/hal/top/top_gm20b.h | 2 +- drivers/gpu/nvgpu/hal/top/top_gm20b_fusa.c | 3 +-- drivers/gpu/nvgpu/hal/top/top_gp10b_fusa.c | 29 +++++++++------------- drivers/gpu/nvgpu/include/nvgpu/gops_top.h | 2 +- userspace/units/top/nvgpu-top.c | 25 +++++++++---------- userspace/units/top/nvgpu-top.h | 4 +++ 7 files changed, 32 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/nvgpu/hal/top/top_gm20b.c b/drivers/gpu/nvgpu/hal/top/top_gm20b.c index 660229396..ab9d46c0f 100644 --- a/drivers/gpu/nvgpu/hal/top/top_gm20b.c +++ b/drivers/gpu/nvgpu/hal/top/top_gm20b.c @@ -110,18 +110,12 @@ int gm20b_get_device_info(struct gk20a *g, struct nvgpu_device_info *dev_info, if (top_device_info_type_enum_v(entry_engine) == engine_type) { dev_info->engine_type = engine_type; if (g->ops.top.device_info_parse_enum != NULL) { - ret = g->ops.top.device_info_parse_enum(g, + g->ops.top.device_info_parse_enum(g, entry_enum, &dev_info->engine_id, &dev_info->runlist_id, &dev_info->intr_id, &dev_info->reset_id); - if (ret != 0) { - nvgpu_err(g, - "Error parsing Enum Entry 0x%x", - entry_enum); - return ret; - } } if (g->ops.top.device_info_parse_data != NULL) { ret = g->ops.top.device_info_parse_data(g, diff --git a/drivers/gpu/nvgpu/hal/top/top_gm20b.h b/drivers/gpu/nvgpu/hal/top/top_gm20b.h index 7816c07ee..ccaeb0530 100644 --- a/drivers/gpu/nvgpu/hal/top/top_gm20b.h +++ b/drivers/gpu/nvgpu/hal/top/top_gm20b.h @@ -30,7 +30,7 @@ struct gk20a; struct nvgpu_device_info; -int gm20b_device_info_parse_enum(struct gk20a *g, u32 table_entry, +void gm20b_device_info_parse_enum(struct gk20a *g, u32 table_entry, u32 *engine_id, u32 *runlist_id, u32 *intr_id, u32 *reset_id); #ifdef CONFIG_NVGPU_HAL_NON_FUSA diff --git a/drivers/gpu/nvgpu/hal/top/top_gm20b_fusa.c b/drivers/gpu/nvgpu/hal/top/top_gm20b_fusa.c index 40d26652f..d5dc26daf 100644 --- a/drivers/gpu/nvgpu/hal/top/top_gm20b_fusa.c +++ b/drivers/gpu/nvgpu/hal/top/top_gm20b_fusa.c @@ -28,7 +28,7 @@ #include -int gm20b_device_info_parse_enum(struct gk20a *g, u32 table_entry, +void gm20b_device_info_parse_enum(struct gk20a *g, u32 table_entry, u32 *engine_id, u32 *runlist_id, u32 *intr_id, u32 *reset_id) { @@ -66,7 +66,6 @@ int gm20b_device_info_parse_enum(struct gk20a *g, u32 table_entry, } nvgpu_log_info(g, "Reset_id: %u", *reset_id); - return 0; } bool gm20b_is_engine_gr(struct gk20a *g, u32 engine_type) diff --git a/drivers/gpu/nvgpu/hal/top/top_gp10b_fusa.c b/drivers/gpu/nvgpu/hal/top/top_gp10b_fusa.c index f7b0d253d..84790338f 100644 --- a/drivers/gpu/nvgpu/hal/top/top_gp10b_fusa.c +++ b/drivers/gpu/nvgpu/hal/top/top_gp10b_fusa.c @@ -65,32 +65,21 @@ static int gp10b_check_device_match(struct gk20a *g, && (top_device_info_data_inst_id_v(entry_data) == inst_id)) { dev_info->engine_type = engine_type; - if (g->ops.top.device_info_parse_enum != NULL) { - ret = g->ops.top.device_info_parse_enum(g, + g->ops.top.device_info_parse_enum(g, entry_enum, &dev_info->engine_id, &dev_info->runlist_id, &dev_info->intr_id, &dev_info->reset_id); - if (ret != 0) { - nvgpu_err(g, - "Error parsing Enum Entry 0x%x", - entry_data); - return ret; - } - } - if (g->ops.top.device_info_parse_data != NULL) { - ret = g->ops.top.device_info_parse_data(g, + ret = g->ops.top.device_info_parse_data(g, entry_data, &dev_info->inst_id, &dev_info->pri_base, &dev_info->fault_id); - if (ret != 0) { - nvgpu_err(g, - "Error parsing Data Entry 0x%x", - entry_data); - return ret; - } + if (ret != 0) { + nvgpu_err(g, "Error parsing Data Entry 0x%x", + entry_data); + return ret; } } @@ -114,6 +103,12 @@ int gp10b_get_device_info(struct gk20a *g, struct nvgpu_device_info *dev_info, return -EINVAL; } + if ((g->ops.top.device_info_parse_enum == NULL) || + (g->ops.top.device_info_parse_data == NULL)) { + nvgpu_err(g, "Dev_info parsing functions ptrs not set."); + return -EINVAL; + } + for (i = 0; i < max_info_entries; i++) { table_entry = nvgpu_readl(g, top_device_info_r(i)); entry = top_device_info_entry_v(table_entry); diff --git a/drivers/gpu/nvgpu/include/nvgpu/gops_top.h b/drivers/gpu/nvgpu/include/nvgpu/gops_top.h index 0aabd6b8c..0562f712d 100644 --- a/drivers/gpu/nvgpu/include/nvgpu/gops_top.h +++ b/drivers/gpu/nvgpu/include/nvgpu/gops_top.h @@ -286,7 +286,7 @@ struct gops_top { /** * HALs used within "Top" unit. Private HALs. */ - int (*device_info_parse_enum)(struct gk20a *g, + void (*device_info_parse_enum)(struct gk20a *g, u32 table_entry, u32 *engine_id, u32 *runlist_id, u32 *intr_id, u32 *reset_id); diff --git a/userspace/units/top/nvgpu-top.c b/userspace/units/top/nvgpu-top.c index 5df35abed..f0266b4cd 100644 --- a/userspace/units/top/nvgpu-top.c +++ b/userspace/units/top/nvgpu-top.c @@ -131,7 +131,6 @@ int test_device_info_parse_enum(struct unit_module *m, struct gk20a *g, void *args) { int ret = UNIT_SUCCESS; - int val = 0; u32 engine_id = 0U; u32 runlist_id = 0U; u32 intr_id = 0U; @@ -149,15 +148,10 @@ int test_device_info_parse_enum(struct unit_module *m, struct gk20a *g, table_entry = 0x10228C3E; /* Call top.device_info_parse_enum to parse the above table entry */ - val = g->ops.top.device_info_parse_enum(g, table_entry, &engine_id, + g->ops.top.device_info_parse_enum(g, table_entry, &engine_id, &runlist_id, &intr_id, &reset_id); - if (val != 0) { - unit_err(m, "Call to top.device_info_parse_enum() failed.\n"); - ret = UNIT_FAIL; - } - /* Verify if the parsed data is as expected */ if (engine_id != 4U) { unit_err(m, @@ -190,15 +184,10 @@ int test_device_info_parse_enum(struct unit_module *m, struct gk20a *g, table_entry = 0x10228C02; /* Call top.device_info_parse_enum to parse the above table entry */ - val = g->ops.top.device_info_parse_enum(g, table_entry, &engine_id, + g->ops.top.device_info_parse_enum(g, table_entry, &engine_id, &runlist_id, &intr_id, &reset_id); - if (val != 0) { - unit_err(m, "Call to top.device_info_parse_enum() failed.\n"); - ret = UNIT_FAIL; - } - /* Verify if the parsed data is as expected */ if (engine_id != U32_MAX) { unit_err(m, @@ -543,6 +532,16 @@ int test_get_device_info(struct unit_module *m, struct gk20a *g, void *args) ret = UNIT_FAIL; } + /* Call top.get_device_info with NULL function pointers */ + g->ops.top.device_info_parse_enum = NULL; + g->ops.top.device_info_parse_data = NULL; + val = g->ops.top.get_device_info(g, &dev_info_1, engine_type, inst_id); + if (val != -EINVAL) { + unit_err(m, + "get_device_info() failed to handle NULL function pointers.\n"); + ret = UNIT_FAIL; + } + return ret; } diff --git a/userspace/units/top/nvgpu-top.h b/userspace/units/top/nvgpu-top.h index a14e478a9..0920bb881 100644 --- a/userspace/units/top/nvgpu-top.h +++ b/userspace/units/top/nvgpu-top.h @@ -353,6 +353,10 @@ int test_get_num_engine_type_entries(struct unit_module *m, struct gk20a *g, * Steps: * - The device_info table is setup during test_top_setup(). * - The device_info table is initialized to have one copy engine entry. + * - Set device_info_parse_enum() and device_info_parse_data() HAL to NULL. + * Call get_device_info HAL and check if it returns -EINVAL when the data + * parsing function pointers are not initialized. + * - Initialize the device_info_parse_enum and device_info_parse_data HAL. * - Call get_device_info HAL to parse copy engine related data. * - Here, we just make sure the call returns success; we do not check the * parsed values as we have separate tests for verifying enum and data