From 4cacfb709caf09961268c386ba8b9141a2b9e1f2 Mon Sep 17 00:00:00 2001 From: Vedashree Vidwans Date: Wed, 7 Sep 2022 14:41:34 -0700 Subject: [PATCH] tegra: hwpm: fix bugs and rearrange functions Fix bugs for accessing uninitialized mem_mgmt and alist_map structures. - Not initializing memory management functionality in HWPM is a valid situation. Currently, tegra_hwpm_clear_mem_pipeline dereferences the mem_mgmt pointer without validation, which can lead to failures. Check if mem_mgmt struct pointer is valid before dereferencing. - Alist_map structure is only required if user requests allowlist size or list of allowlist registers. Currently, tegra_hwpm_sw_setup dereferences alist_map pointer to set full_alist_size to 0. Add alist_map allocation function and set full_alist_size to 0. - Add PMA and CMD_ROUTER as valid cases for tegra_hwpm_translate_soc_hwpm_resource() translation. - Move logic to record IP function pointers to separate function. This way changes to IP ops registration will be in single location. Jira THWPM-41 Change-Id: I2c221bb13b6875b76a6fabee4c224d77ac72a6fc Signed-off-by: Vedashree Vidwans Reviewed-on: https://git-master.nvidia.com/r/c/linux-nvidia/+/2770418 Reviewed-by: svc-mobile-coverity Reviewed-by: Vasuki Shankar Reviewed-by: Seema Khowala GVS: Gerrit_Virtual_Submit --- common/allowlist.c | 15 +- common/init.c | 1 - include/tegra_hwpm_common.h | 1 + os/linux/ioctl.c | 12 +- os/linux/ip_utils.c | 339 ++++++++++++++++++------------------ os/linux/mem_mgmt_utils.c | 34 ++-- 6 files changed, 212 insertions(+), 190 deletions(-) diff --git a/common/allowlist.c b/common/allowlist.c index ce6cdf5..6a36ec5 100644 --- a/common/allowlist.c +++ b/common/allowlist.c @@ -18,10 +18,8 @@ #include #include -int tegra_hwpm_get_allowlist_size(struct tegra_soc_hwpm *hwpm) +int tegra_hwpm_alloc_alist_map(struct tegra_soc_hwpm *hwpm) { - int ret = 0; - tegra_hwpm_fn(hwpm, " "); if (hwpm->alist_map == NULL) { @@ -29,13 +27,20 @@ int tegra_hwpm_get_allowlist_size(struct tegra_soc_hwpm *hwpm) hwpm->alist_map = tegra_hwpm_kzalloc(hwpm, sizeof(struct tegra_hwpm_allowlist_map)); if (!hwpm->alist_map) { - tegra_hwpm_err(NULL, - "Couldn't allocate allowlist map structure"); return -ENOMEM; } hwpm->alist_map->full_alist_size = 0ULL; } + return 0; +} + +int tegra_hwpm_get_allowlist_size(struct tegra_soc_hwpm *hwpm) +{ + int ret = 0; + + tegra_hwpm_fn(hwpm, " "); + if (hwpm->alist_map->full_alist_size == 0) { /* Full alist size is not computed yet */ ret = tegra_hwpm_func_all_ip(hwpm, NULL, diff --git a/common/init.c b/common/init.c index a383681..98b214d 100644 --- a/common/init.c +++ b/common/init.c @@ -112,7 +112,6 @@ int tegra_hwpm_setup_sw(struct tegra_soc_hwpm *hwpm) /* Initialize SW state */ hwpm->bind_completed = false; - hwpm->alist_map->full_alist_size = 0; return 0; } diff --git a/include/tegra_hwpm_common.h b/include/tegra_hwpm_common.h index 6797a9e..0905dfe 100644 --- a/include/tegra_hwpm_common.h +++ b/include/tegra_hwpm_common.h @@ -60,6 +60,7 @@ int tegra_hwpm_finalize_chip_info(struct tegra_soc_hwpm *hwpm); int tegra_hwpm_ip_handle_power_mgmt(struct tegra_soc_hwpm *hwpm, struct hwpm_ip_inst *ip_inst, bool disable); +int tegra_hwpm_alloc_alist_map(struct tegra_soc_hwpm *hwpm); int tegra_hwpm_get_allowlist_size(struct tegra_soc_hwpm *hwpm); int tegra_hwpm_combine_alist(struct tegra_soc_hwpm *hwpm, u64 *alist); diff --git a/os/linux/ioctl.c b/os/linux/ioctl.c index 0418b0d..2c4a2d4 100644 --- a/os/linux/ioctl.c +++ b/os/linux/ioctl.c @@ -186,12 +186,20 @@ static int tegra_hwpm_query_allowlist_ioctl(struct tegra_soc_hwpm *hwpm, return -EPERM; } + if (hwpm->alist_map == NULL) { + ret = tegra_hwpm_alloc_alist_map(hwpm); + if (ret != 0) { + tegra_hwpm_err(hwpm, + "Couldn't allocate allowlist map structure"); + return ret; + } + } + if (query_allowlist->allowlist == NULL) { /* Userspace is querying allowlist size only */ ret = tegra_hwpm_get_allowlist_size(hwpm); if (ret != 0) { - tegra_hwpm_err(hwpm, - "failed to get alist_size"); + tegra_hwpm_err(hwpm, "failed to get alist_size"); return ret; } query_allowlist->allowlist_size = diff --git a/os/linux/ip_utils.c b/os/linux/ip_utils.c index 019e0c8..6dbb85e 100644 --- a/os/linux/ip_utils.c +++ b/os/linux/ip_utils.c @@ -27,171 +27,6 @@ struct hwpm_ip_register_list *ip_register_list_head; #define REGISTER_IP true #define UNREGISTER_IP false -static int tegra_hwpm_alloc_ip_register_list_node( - struct tegra_soc_hwpm_ip_ops *hwpm_ip_ops, - struct hwpm_ip_register_list **node_ptr) -{ - struct hwpm_ip_register_list *new_node = NULL; - - new_node = tegra_hwpm_kzalloc(NULL, - sizeof(struct hwpm_ip_register_list)); - if (new_node == NULL) { - tegra_hwpm_err(NULL, - "struct hwpm_ip_register_list node allocation failed"); - return -ENOMEM; - } - new_node->next = NULL; - - /* Copy given ip register details to node */ - memcpy(&new_node->ip_ops, hwpm_ip_ops, - sizeof(struct tegra_soc_hwpm_ip_ops)); - (*node_ptr) = new_node; - - return 0; -} - -static int tegra_hwpm_note_ip_register( - struct tegra_soc_hwpm_ip_ops *hwpm_ip_ops) -{ - int err = 0; - struct hwpm_ip_register_list *node; - - if (ip_register_list_head == NULL) { - err = tegra_hwpm_alloc_ip_register_list_node(hwpm_ip_ops, - &ip_register_list_head); - if (err != 0) { - tegra_hwpm_err(NULL, - "failed to note ip registration"); - return err; - } - } else { - node = ip_register_list_head; - while (node->next != NULL) { - node = node->next; - } - - err = tegra_hwpm_alloc_ip_register_list_node(hwpm_ip_ops, - &node->next); - if (err != 0) { - tegra_hwpm_err(NULL, - "failed to note ip registration"); - return err; - } - } - - return err; -} - -void tegra_soc_hwpm_ip_register(struct tegra_soc_hwpm_ip_ops *hwpm_ip_ops) -{ - struct tegra_soc_hwpm *hwpm = NULL; - struct tegra_hwpm_ip_ops ip_ops; - int ret = 0; - - if (hwpm_ip_ops == NULL) { - tegra_hwpm_err(NULL, "IP details missing"); - return; - } - - if (tegra_soc_hwpm_pdev == NULL) { - tegra_hwpm_dbg(hwpm, hwpm_info | hwpm_dbg_ip_register, - "Noting IP 0x%llx register request", - hwpm_ip_ops->ip_base_address); - ret = tegra_hwpm_note_ip_register(hwpm_ip_ops); - if (ret != 0) { - tegra_hwpm_err(NULL, - "Couldn't save IP register details"); - return; - } - } else { - if (hwpm_ip_ops->ip_dev == NULL) { - tegra_hwpm_err(hwpm, "IP dev to register is NULL"); - return; - } - hwpm = platform_get_drvdata(tegra_soc_hwpm_pdev); - - tegra_hwpm_dbg(hwpm, hwpm_info | hwpm_dbg_ip_register, - "Register IP 0x%llx", hwpm_ip_ops->ip_base_address); - - ip_ops.ip_dev = hwpm_ip_ops->ip_dev; - ip_ops.hwpm_ip_pm = hwpm_ip_ops->hwpm_ip_pm; - ip_ops.hwpm_ip_reg_op = hwpm_ip_ops->hwpm_ip_reg_op; - - ret = hwpm->active_chip->extract_ip_ops(hwpm, - hwpm_ip_ops->resource_enum, - hwpm_ip_ops->ip_base_address, &ip_ops, REGISTER_IP); - if (ret < 0) { - tegra_hwpm_err(hwpm, "Failed to set IP ops for IP %d", - hwpm_ip_ops->resource_enum); - } - } -} - -void tegra_soc_hwpm_ip_unregister(struct tegra_soc_hwpm_ip_ops *hwpm_ip_ops) -{ - struct tegra_soc_hwpm *hwpm = NULL; - struct tegra_hwpm_ip_ops ip_ops; - int ret = 0; - - if (hwpm_ip_ops == NULL) { - tegra_hwpm_err(NULL, "IP details missing"); - return; - } - - if (tegra_soc_hwpm_pdev == NULL) { - tegra_hwpm_dbg(hwpm, hwpm_info | hwpm_dbg_ip_register, - "HWPM device not available"); - } else { - if (hwpm_ip_ops->ip_dev == NULL) { - tegra_hwpm_err(hwpm, "IP dev to unregister is NULL"); - return; - } - hwpm = platform_get_drvdata(tegra_soc_hwpm_pdev); - - tegra_hwpm_dbg(hwpm, hwpm_info | hwpm_dbg_ip_register, - "Unregister IP 0x%llx", hwpm_ip_ops->ip_base_address); - - ip_ops.ip_dev = hwpm_ip_ops->ip_dev; - ip_ops.hwpm_ip_pm = hwpm_ip_ops->hwpm_ip_pm; - ip_ops.hwpm_ip_reg_op = hwpm_ip_ops->hwpm_ip_reg_op; - - ret = hwpm->active_chip->extract_ip_ops(hwpm, - hwpm_ip_ops->resource_enum, - hwpm_ip_ops->ip_base_address, &ip_ops, UNREGISTER_IP); - if (ret < 0) { - tegra_hwpm_err(hwpm, "Failed to reset IP ops for IP %d", - hwpm_ip_ops->resource_enum); - } - } -} - -int tegra_hwpm_complete_ip_register_impl(struct tegra_soc_hwpm *hwpm) -{ - int ret = 0; - struct hwpm_ip_register_list *node = ip_register_list_head; - struct tegra_hwpm_ip_ops ip_ops; - - tegra_hwpm_fn(hwpm, " "); - - while (node != NULL) { - ip_ops.ip_dev = node->ip_ops.ip_dev; - ip_ops.hwpm_ip_pm = node->ip_ops.hwpm_ip_pm; - ip_ops.hwpm_ip_reg_op = node->ip_ops.hwpm_ip_reg_op; - - ret = hwpm->active_chip->extract_ip_ops(hwpm, - node->ip_ops.resource_enum, - node->ip_ops.ip_base_address, &ip_ops, true); - if (ret != 0) { - tegra_hwpm_err(hwpm, - "Resource enum %d extract IP ops failed", - node->ip_ops.resource_enum); - return ret; - } - node = node->next; - } - return ret; -} - static u32 tegra_hwpm_translate_soc_hwpm_ip(struct tegra_soc_hwpm *hwpm, enum tegra_soc_hwpm_ip ip_enum) { @@ -342,6 +177,12 @@ u32 tegra_hwpm_translate_soc_hwpm_resource(struct tegra_soc_hwpm *hwpm, case TEGRA_SOC_HWPM_RESOURCE_MSS_MCF: res_enum_idx = TEGRA_HWPM_RESOURCE_MSS_MCF; break; + case TEGRA_SOC_HWPM_RESOURCE_PMA: + res_enum_idx = TEGRA_HWPM_RESOURCE_PMA; + break; + case TEGRA_SOC_HWPM_RESOURCE_CMD_SLICE_RTR: + res_enum_idx = TEGRA_HWPM_RESOURCE_CMD_SLICE_RTR; + break; case TEGRA_SOC_HWPM_RESOURCE_APE: res_enum_idx = TEGRA_HWPM_RESOURCE_APE; break; @@ -381,6 +222,174 @@ int tegra_hwpm_get_resource_info(struct tegra_soc_hwpm *hwpm, return ret; } +static int tegra_hwpm_record_ip_ops(struct tegra_soc_hwpm *hwpm, + struct tegra_soc_hwpm_ip_ops *soc_ip_ops) +{ + struct tegra_hwpm_ip_ops ip_ops; + + tegra_hwpm_fn(hwpm, " "); + + ip_ops.ip_dev = soc_ip_ops->ip_dev; + ip_ops.hwpm_ip_pm = soc_ip_ops->hwpm_ip_pm; + ip_ops.hwpm_ip_reg_op = soc_ip_ops->hwpm_ip_reg_op; + + if (soc_ip_ops->resource_enum >= TERGA_SOC_HWPM_NUM_RESOURCES) { + tegra_hwpm_err(hwpm, "resource enum %d out of scope", + soc_ip_ops->resource_enum); + return -EINVAL; + } + + return hwpm->active_chip->extract_ip_ops(hwpm, + tegra_hwpm_translate_soc_hwpm_resource(hwpm, + (enum tegra_soc_hwpm_resource)soc_ip_ops->resource_enum), + soc_ip_ops->ip_base_address, + &ip_ops, true); +} + +int tegra_hwpm_complete_ip_register_impl(struct tegra_soc_hwpm *hwpm) +{ + int ret = 0; + struct hwpm_ip_register_list *node = ip_register_list_head; + + tegra_hwpm_fn(hwpm, " "); + + while (node != NULL) { + ret = tegra_hwpm_record_ip_ops(hwpm, &node->ip_ops); + if (ret != 0) { + tegra_hwpm_err(hwpm, + "Resource enum %d extract IP ops failed", + node->ip_ops.resource_enum); + return ret; + } + node = node->next; + } + return ret; +} + +static int tegra_hwpm_alloc_ip_register_list_node( + struct tegra_soc_hwpm_ip_ops *hwpm_ip_ops, + struct hwpm_ip_register_list **node_ptr) +{ + struct hwpm_ip_register_list *new_node = NULL; + + new_node = tegra_hwpm_kzalloc(NULL, + sizeof(struct hwpm_ip_register_list)); + if (new_node == NULL) { + tegra_hwpm_err(NULL, + "struct hwpm_ip_register_list node allocation failed"); + return -ENOMEM; + } + new_node->next = NULL; + + /* Copy given ip register details to node */ + memcpy(&new_node->ip_ops, hwpm_ip_ops, + sizeof(struct tegra_soc_hwpm_ip_ops)); + (*node_ptr) = new_node; + + return 0; +} + +static int tegra_hwpm_note_ip_register( + struct tegra_soc_hwpm_ip_ops *hwpm_ip_ops) +{ + int err = 0; + struct hwpm_ip_register_list *node; + + if (ip_register_list_head == NULL) { + err = tegra_hwpm_alloc_ip_register_list_node(hwpm_ip_ops, + &ip_register_list_head); + if (err != 0) { + tegra_hwpm_err(NULL, + "failed to note ip registration"); + return err; + } + } else { + node = ip_register_list_head; + while (node->next != NULL) { + node = node->next; + } + + err = tegra_hwpm_alloc_ip_register_list_node(hwpm_ip_ops, + &node->next); + if (err != 0) { + tegra_hwpm_err(NULL, + "failed to note ip registration"); + return err; + } + } + + return err; +} + +void tegra_soc_hwpm_ip_register(struct tegra_soc_hwpm_ip_ops *hwpm_ip_ops) +{ + struct tegra_soc_hwpm *hwpm = NULL; + int ret = 0; + + if (hwpm_ip_ops == NULL) { + tegra_hwpm_err(NULL, "IP details missing"); + return; + } + + if (tegra_soc_hwpm_pdev == NULL) { + tegra_hwpm_dbg(hwpm, hwpm_info | hwpm_dbg_ip_register, + "Noting IP 0x%llx register request", + hwpm_ip_ops->ip_base_address); + ret = tegra_hwpm_note_ip_register(hwpm_ip_ops); + if (ret != 0) { + tegra_hwpm_err(NULL, + "Couldn't save IP register details"); + return; + } + } else { + if (hwpm_ip_ops->ip_dev == NULL) { + tegra_hwpm_err(hwpm, "IP dev to register is NULL"); + return; + } + hwpm = platform_get_drvdata(tegra_soc_hwpm_pdev); + + tegra_hwpm_dbg(hwpm, hwpm_info | hwpm_dbg_ip_register, + "Register IP 0x%llx", hwpm_ip_ops->ip_base_address); + + ret = tegra_hwpm_record_ip_ops(hwpm, hwpm_ip_ops); + if (ret < 0) { + tegra_hwpm_err(hwpm, "Failed to set IP ops for IP %d", + hwpm_ip_ops->resource_enum); + } + } +} + +void tegra_soc_hwpm_ip_unregister(struct tegra_soc_hwpm_ip_ops *hwpm_ip_ops) +{ + struct tegra_soc_hwpm *hwpm = NULL; + int ret = 0; + + if (hwpm_ip_ops == NULL) { + tegra_hwpm_err(NULL, "IP details missing"); + return; + } + + if (tegra_soc_hwpm_pdev == NULL) { + tegra_hwpm_dbg(hwpm, hwpm_info | hwpm_dbg_ip_register, + "HWPM device not available"); + } else { + if (hwpm_ip_ops->ip_dev == NULL) { + tegra_hwpm_err(hwpm, "IP dev to unregister is NULL"); + return; + } + hwpm = platform_get_drvdata(tegra_soc_hwpm_pdev); + + tegra_hwpm_dbg(hwpm, hwpm_info | hwpm_dbg_ip_register, + "Unregister IP 0x%llx", hwpm_ip_ops->ip_base_address); + + ret = tegra_hwpm_record_ip_ops(hwpm, hwpm_ip_ops); + if (ret < 0) { + tegra_hwpm_err(hwpm, "Failed to reset IP ops for IP %d", + hwpm_ip_ops->resource_enum); + } + } +} + void tegra_hwpm_release_ip_register_node(struct tegra_soc_hwpm *hwpm) { struct hwpm_ip_register_list *node = ip_register_list_head; diff --git a/os/linux/mem_mgmt_utils.c b/os/linux/mem_mgmt_utils.c index 2798865..42e725b 100644 --- a/os/linux/mem_mgmt_utils.c +++ b/os/linux/mem_mgmt_utils.c @@ -278,6 +278,13 @@ int tegra_hwpm_clear_mem_pipeline(struct tegra_soc_hwpm *hwpm) tegra_hwpm_fn(hwpm, " "); + if (hwpm->mem_mgmt == NULL) { + /* Memory buffer was not initialized */ + tegra_hwpm_dbg(hwpm, hwpm_dbg_alloc_pma_stream, + "mem_mgmt struct was uninitialized in this sesion"); + return 0; + } + /* Stream MEM_BYTES to clear pipeline */ if (hwpm->mem_mgmt->mem_bytes_kernel) { u32 *mem_bytes_kernel_u32 = @@ -392,17 +399,6 @@ int tegra_hwpm_map_update_allowlist(struct tegra_soc_hwpm *hwpm, tegra_hwpm_fn(hwpm, " "); - if (hwpm->alist_map == NULL) { - /* Allocate tegra_hwpm_allowlist_map */ - hwpm->alist_map = tegra_hwpm_kzalloc(hwpm, - sizeof(struct tegra_hwpm_allowlist_map)); - if (!hwpm->alist_map) { - tegra_hwpm_err(NULL, - "Couldn't allocate allowlist map structure"); - return -ENOMEM; - } - } - if (hwpm->alist_map->full_alist_size == 0ULL) { tegra_hwpm_err(hwpm, "Allowlist size should be computed before"); @@ -474,12 +470,13 @@ void tegra_hwpm_release_alist_map(struct tegra_soc_hwpm *hwpm) vunmap(hwpm->alist_map->full_alist_map); } - for (idx = 0ULL; idx < hwpm->alist_map->num_pages; idx++) { - set_page_dirty(hwpm->alist_map->pages[idx]); - put_page(hwpm->alist_map->pages[idx]); - } - if (hwpm->alist_map->pages) { + for (idx = 0ULL; idx < hwpm->alist_map->num_pages; + idx++) { + set_page_dirty(hwpm->alist_map->pages[idx]); + put_page(hwpm->alist_map->pages[idx]); + } + tegra_hwpm_kfree(hwpm, hwpm->alist_map->pages); } @@ -489,5 +486,8 @@ void tegra_hwpm_release_alist_map(struct tegra_soc_hwpm *hwpm) void tegra_hwpm_release_mem_mgmt(struct tegra_soc_hwpm *hwpm) { - tegra_hwpm_kfree(hwpm, hwpm->mem_mgmt); + if (hwpm->mem_mgmt != NULL) { + tegra_hwpm_kfree(hwpm, hwpm->mem_mgmt); + hwpm->mem_mgmt = NULL; + } }