Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 29 Jun 2023 08:19:40 +0530
From:   Pavan Kondeti <quic_pkondeti@quicinc.com>
To:     Mukesh Ojha <quic_mojha@quicinc.com>
CC:     <corbet@lwn.net>, <agross@kernel.org>, <andersson@kernel.org>,
        <konrad.dybcio@linaro.org>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
        <keescook@chromium.org>, <tony.luck@intel.com>,
        <gpiccoli@igalia.com>, <mathieu.poirier@linaro.org>,
        <catalin.marinas@arm.com>, <will@kernel.org>,
        <linus.walleij@linaro.org>, <andy.shevchenko@gmail.com>,
        <linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-hardening@vger.kernel.org>,
        <linux-remoteproc@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-gpio@vger.kernel.org>
Subject: Re: [PATCH v4 07/21] soc: qcom: minidump: Add update region support
Message-ID: <b099458b-8e37-43cb-831e-fe7812d11a65@quicinc.com>
References: <1687955688-20809-1-git-send-email-quic_mojha@quicinc.com>
 <1687955688-20809-8-git-send-email-quic_mojha@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <1687955688-20809-8-git-send-email-quic_mojha@quicinc.com>
Precedence: bulk

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