| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 19 Oct 2022 08:49:03 +0200
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Gokul krishna Krishnakumar <quic_gokukris@...cinc.com>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...ainline.org>,
Mathieu Poirier <mathieu.poirier@...aro.org>
Cc: linux-arm-msm@...r.kernel.org, linux-remoteproc@...r.kernel.org,
linux-kernel@...r.kernel.org, Trilok Soni <quic_tsoni@...cinc.com>,
Satya Durga Srinivasu Prabhala <quic_satyap@...cinc.com>,
Rajendra Nayak <quic_rjendra@...cinc.com>,
Elliot Berman <quic_eberman@...cinc.com>,
Guru Das Srinagesh <quic_gurus@...cinc.com>
Subject: Re: [PATCH v1 1/1] drivers: remoteproc: Add bus scaling capability
during bootup
Hi Gokul,
On 18/10/2022 23:10, Gokul krishna Krishnakumar wrote:
> During bootup since remote processors cannot request for
> additional bus bandwidth from the interconect framework,
> platform driver should provide the proxy resources. Make
> a proxy vote for maximizing the bus bandwidth during bootup
> for a remote processor and remove it once processor is up.
A similar change has been upstreamed in
- [0] 65b7ebda5028 ("firmware: qcom_scm: Add bw voting support to the SCM interface")
- [1] 8d9be5c6bdcd ("remoteproc: qcom: q6v5: Add interconnect path proxy vote")
The main difference with this patch are:
- [0] only votes for each scm calls, not across multiple calls, is that a problem ?
- [0] & [1] votes (0, UINT_MAX) but you change votes (UINT_MAX, UINT_MAX), does this make a difference ?
[0] https://github.com/torvalds/linux/commit/65b7ebda5028
[1] https://github.com/torvalds/linux/commit/8d9be5c6bdcd
>
> Change-Id: I798b3b000aef4761a7ff8cb6390b7ecd98f925b7
> Signed-off-by: Gokul krishna Krishnakumar <quic_gokukris@...cinc.com>
> ---
> drivers/remoteproc/qcom_q6v5_pas.c | 106 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 100 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 6afd094..b02a1dc 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -21,6 +21,7 @@
> #include <linux/qcom_scm.h>
> #include <linux/regulator/consumer.h>
> #include <linux/remoteproc.h>
> +#include <linux/interconnect.h>
> #include <linux/soc/qcom/mdt_loader.h>
> #include <linux/soc/qcom/smem.h>
> #include <linux/soc/qcom/smem_state.h>
> @@ -30,8 +31,14 @@
> #include "qcom_q6v5.h"
> #include "remoteproc_internal.h"
>
> +#define PIL_TZ_AVG_BW UINT_MAX
> +#define PIL_TZ_PEAK_BW UINT_MAX
> #define ADSP_DECRYPT_SHUTDOWN_DELAY_MS 100
>
> +static struct icc_path *scm_perf_client;
> +static int scm_pas_bw_count;
> +static DEFINE_MUTEX(scm_pas_bw_mutex);
> +
> struct adsp_data {
> int crash_reason_smem;
> const char *firmware_name;
> @@ -67,6 +74,7 @@ struct qcom_adsp {
>
> int pas_id;
> unsigned int minidump_id;
> + struct icc_path *bus_client;
> int crash_reason_smem;
> bool has_aggre2_clk;
> bool decrypt_shutdown;
> @@ -136,6 +144,44 @@ static void adsp_pds_disable(struct qcom_adsp *adsp, struct device **pds,
> }
> }
>
> +static int scm_pas_enable_bw(void)
> +{
> + int ret = 0;
> +
> + if (IS_ERR(scm_perf_client))
> + return -EINVAL;
> +
> + mutex_lock(&scm_pas_bw_mutex);
> + if (!scm_pas_bw_count) {
> + ret = icc_set_bw(scm_perf_client, PIL_TZ_AVG_BW,
> + PIL_TZ_PEAK_BW);
> + if (ret)
> + goto err_bus;
> + }
> +
> + scm_pas_bw_count++;
> + mutex_unlock(&scm_pas_bw_mutex);
> + return ret;
> +
> +err_bus:
> + pr_err("scm-pas: Bandwidth request failed (%d)\n", ret);
> + icc_set_bw(scm_perf_client, 0, 0);
> +
> + mutex_unlock(&scm_pas_bw_mutex);
> + return ret;
> +}
> +
> +static void scm_pas_disable_bw(void)
> +{
> + if (IS_ERR(scm_perf_client))
> + return;
> +
> + mutex_lock(&scm_pas_bw_mutex);
> + if (scm_pas_bw_count-- == 1)
> + icc_set_bw(scm_perf_client, 0, 0);
> + mutex_unlock(&scm_pas_bw_mutex);
> +}
> +
> static int adsp_shutdown_poll_decrypt(struct qcom_adsp *adsp)
> {
> unsigned int retry_num = 50;
> @@ -174,15 +220,35 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
> if (ret)
> return ret;
>
> + scm_pas_enable_bw();
> ret = qcom_mdt_load_no_init(adsp->dev, fw, rproc->firmware, adsp->pas_id,
> adsp->mem_region, adsp->mem_phys, adsp->mem_size,
> &adsp->mem_reloc);
> if (ret)
> - return ret;
> + goto exit;
>
> qcom_pil_info_store(adsp->info_name, adsp->mem_phys, adsp->mem_size);
> +exit:
> + scm_pas_disable_bw();
> + return ret;
> +}
>
> - return 0;
> +static int do_bus_scaling(struct qcom_adsp *adsp, bool enable)
> +{
> + int rc = 0;
> + u32 avg_bw = enable ? PIL_TZ_AVG_BW : 0;
> + u32 peak_bw = enable ? PIL_TZ_PEAK_BW : 0;
> +
> + if (IS_ERR(adsp->bus_client))
> + dev_err(adsp->dev, "Bus scaling not setup for %s\n",
> + adsp->rproc->name);
> + else
> + rc = icc_set_bw(adsp->bus_client, avg_bw, peak_bw);
> +
> + if (rc)
> + dev_err(adsp->dev, "bandwidth request failed(rc:%d)\n", rc);
> +
> + return rc;
> }
>
> static int adsp_start(struct rproc *rproc)
> @@ -194,10 +260,14 @@ static int adsp_start(struct rproc *rproc)
> if (ret)
> return ret;
>
> - ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
> + ret = do_bus_scaling(adsp, true);
> if (ret < 0)
> goto disable_irqs;
>
> + ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
> + if (ret < 0)
> + goto unscale_bus;
> +
> ret = clk_prepare_enable(adsp->xo);
> if (ret)
> goto disable_proxy_pds;
> @@ -218,6 +288,8 @@ static int adsp_start(struct rproc *rproc)
> goto disable_cx_supply;
> }
>
> + scm_pas_enable_bw();
> +
> ret = qcom_scm_pas_auth_and_reset(adsp->pas_id);
> if (ret) {
> dev_err(adsp->dev,
> @@ -234,9 +306,10 @@ static int adsp_start(struct rproc *rproc)
>
> qcom_scm_pas_metadata_release(&adsp->pas_metadata);
>
> - return 0;
> -
> disable_px_supply:
> + scm_pas_disable_bw();
> + if(!ret)
> + goto exit;
> if (adsp->px_supply)
> regulator_disable(adsp->px_supply);
> disable_cx_supply:
> @@ -248,9 +321,11 @@ static int adsp_start(struct rproc *rproc)
> clk_disable_unprepare(adsp->xo);
> disable_proxy_pds:
> adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
> +unscale_bus:
> + do_bus_scaling(adsp, false);
> disable_irqs:
> qcom_q6v5_unprepare(&adsp->q6v5);
> -
> +exit:
> return ret;
> }
>
> @@ -265,6 +340,7 @@ static void qcom_pas_handover(struct qcom_q6v5 *q6v5)
> clk_disable_unprepare(adsp->aggre2_clk);
> clk_disable_unprepare(adsp->xo);
> adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
> + do_bus_scaling(adsp, false);
> }
>
> static int adsp_stop(struct rproc *rproc)
> @@ -383,6 +459,22 @@ static int adsp_init_regulator(struct qcom_adsp *adsp)
> return 0;
> }
>
> +static void adsp_init_bus_scaling(struct qcom_adsp *adsp)
> +{
> + if (scm_perf_client)
> + goto get_rproc_client;
> +
> + scm_perf_client = of_icc_get(adsp->dev, "crypto_ddr");
> + if (IS_ERR(scm_perf_client))
> + dev_warn(adsp->dev, "Crypto scaling not setup\n");
> +
> +get_rproc_client:
> + adsp->bus_client = of_icc_get(adsp->dev, "rproc_ddr");
> + if (IS_ERR(adsp->bus_client))
> + dev_warn(adsp->dev, "%s: No bus client\n", __func__);
> +
> +}
> +
> static int adsp_pds_attach(struct device *dev, struct device **devs,
> char **pd_names)
> {
> @@ -525,6 +617,8 @@ static int adsp_probe(struct platform_device *pdev)
> if (ret)
> goto free_rproc;
>
> + adsp_init_bus_scaling(adsp);
> +
> ret = adsp_pds_attach(&pdev->dev, adsp->proxy_pds,
> desc->proxy_pd_names);
> if (ret < 0)
Powered by blists - more mailing lists