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

[Pavan Kondeti]

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@xxxxxxxxxxx>
> ---
>  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