Re: [PATCH v4 07/21] soc: qcom: minidump: Add update region support

Pavan Kondeti
2023-06-29T02:50:35Z

On Wed, Jun 28, 2023 at 06:04:34PM +0530, Mukesh Ojha wrote:
> Add support to update client's region physical/virtual addresses,
> which is useful for dynamic loadable modules, dynamic address
> changing clients like if we want to collect current stack
> information for each core and the current stack is changing on
> each sched_switch event, So here virtual/physical address of
> the current stack is changing. So, to cover such use cases
> add the update region support in minidump driver and the
> corresponding smem backend support.
>
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> ---
> drivers/soc/qcom/qcom_minidump.c | 55 +++++++++++++++++++++++++++++++
> drivers/soc/qcom/qcom_minidump_internal.h | 3 ++
> drivers/soc/qcom/qcom_minidump_smem.c | 21 ++++++++++++
> include/soc/qcom/qcom_minidump.h | 5 +++
> 4 files changed, 84 insertions(+)
>
> diff --git a/drivers/soc/qcom/qcom_minidump.c b/drivers/soc/qcom/qcom_minidump.c
> index cfdb63cc29d6..37d6ceb4d85c 100644
> --- a/drivers/soc/qcom/qcom_minidump.c
> +++ b/drivers/soc/qcom/qcom_minidump.c
> @@ -318,6 +318,61 @@ int qcom_minidump_region_unregister(const struct qcom_minidump_region *region)
> }
> EXPORT_SYMBOL_GPL(qcom_minidump_region_unregister);
>
> +/**
> + * qcom_minidump_update_region() - Update region information with new physical
> + * address and virtual address for already registered region e.g, current task
> + * stack for a core keeps on changing on each context switch, there it needs to
> + * change registered region with new updated addresses.
> + *
> + * @region: Should be already registered minidump region.
> + *
> + * Return: On success, it returns 0 and negative error value on failure.
> + */
> +int qcom_minidump_update_region(const struct qcom_minidump_region *region)
> +{
> + struct minidump_pregion *md_pregion;
> + struct qcom_minidump_region *tmp;
> + struct elfhdr *ehdr;
> + struct elf_shdr *shdr;
> + struct elf_phdr *phdr;
> + int idx;
> + int ret = 0;
> +
> + if (!qcom_minidump_valid_region(region))
> + return -EINVAL;
> +
> + mutex_lock(&md_lock);
> + if (!md) {
> + md_pregion = check_if_pending_region_exist(region);
> + if (!md_pregion) {
> + ret = -ENOENT;
> + goto unlock;
> + }
> + tmp = &md_pregion->region;
> + tmp->phys_addr = region->phys_addr;
> + tmp->virt_addr = region->virt_addr;
> + goto unlock;
> + }
> +
> + ret = md->ops->md_update_region(md, &idx, region);
> + if (ret)
> + goto unlock;
> +
> + /* Skip predefined shdr/phdr header entry at the start */
> + ehdr = md->elf.ehdr;
> + shdr = elf_shdr_entry_addr(ehdr, idx + 4);
> + phdr = elf_phdr_entry_addr(ehdr, idx + 1);
> +
> + shdr->sh_addr = (elf_addr_t)region->virt_addr;
> + phdr->p_vaddr = (elf_addr_t)region->virt_addr;
> + phdr->p_paddr = region->phys_addr;
> +
> +unlock:
> + mutex_unlock(&md_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_minidump_update_region);
> +

I don't see any use of this API in the series. Do you plan to add one in
the next version?

Thanks,
Pavan