All of lore.kernel.org
 help / color / mirror / Atom feed
From: Konrad Dybcio <konradybcio@gmail.com>
To: Paul Hollinsky <phollinsky@holtechnik.com>,
	sumit.garg@oss.qualcomm.com, andersson@kernel.org
Cc: mathieu.poirier@linaro.org, robin.clark@oss.qualcomm.com,
	lumag@kernel.org, abhinav.kumar@linux.dev,
	akhilpo@oss.qualcomm.com, konradybcio@kernel.org,
	mukesh.ojha@oss.qualcomm.com,
	cros-qcom-dts-watchers@chromium.org,
	linux-arm-msm@vger.kernel.org, linux-remoteproc@vger.kernel.org,
	dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, regressions@lists.linux.dev
Subject: Re: [REGRESSION] qcom PAS TZ API migration breaks GPU and modem on TrustZone without PAS (sc7180 trogdor)
Date: Mon, 17 Aug 2026 17:41:11 +0200	[thread overview]
Message-ID: <b63e93e4-2f4c-4cad-b726-e1b0565379d7@gmail.com> (raw)
In-Reply-To: <20260808034716.58888-1-phollinsky@holtechnik.com>

On 8/8/26 5:47 AM, Paul Hollinsky wrote:
> Hi,
> 
> Two commits from the "firmware: qcom: Add OP-TEE PAS service support"
> series break the GPU and the modem on SC7180 Chromebooks (trogdor):
> 
>   f3b1357673dd ("remoteproc: qcom_q6v5_mss: Switch to generic PAS TZ APIs")
>   0be72be03ca7 ("drm/msm: Switch to generic PAS TZ APIs")
> 
> Both are in linux-next as of next-20260805 and neither is in a released
> kernel yet, so there is still time to fix this before v7.3.

First of all, thank you for the amazingly thorough write-up. I've been on
holiday so I couldn't look into it quicker.

[...]

> Suggested fix, drm/msm
> ======================
> 
> Given the above, I think the right fix is to move the availability check
> behind the DT discovery rather than restore the SCM check. Boards with no
> zap-shader node then never consult PAS at all, and boards that do have one
> keep the qcom_pas_is_available() gate the original patch was going for:
> 
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -52,6 +52,12 @@
>  		return -ENODEV;
>  	}
>  
> +	/* We need PAS to be able to load the firmware */
> +	if (!qcom_pas_is_available()) {
> +		DRM_DEV_ERROR(dev, "PAS is not available\n");
> +		return -EPROBE_DEFER;
> +	}
> +
>  	ret = of_reserved_mem_region_to_resource(np, 0, &r);
>  	if (ret) {
>  		zap_available = false;
> @@ -170,18 +176,11 @@
>  int adreno_zap_shader_load(struct msm_gpu *gpu, u32 pasid)
>  {
>  	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> -	struct platform_device *pdev = gpu->pdev;
>  
>  	/* Short cut if we determine the zap shader isn't available/needed */
>  	if (!zap_available)
>  		return -ENODEV;
>  
> -	/* We need PAS to be able to load the firmware */
> -	if (!qcom_pas_is_available()) {
> -		DRM_DEV_ERROR(&pdev->dev, "PAS is not available\n");
> -		return -EPROBE_DEFER;
> -	}
> -
>  	return zap_shader_load_mdt(gpu, adreno_gpu->info->zapfw, pasid);
>  }

I believe this is the right fix

> I have this booting here: the GPU initialises and logs "Zap shader not
> enabled - using SECVID_TRUST_CNTL instead", as it did before the
> regression.
> 
> 
> Suggested fix, remoteproc
> =========================
> 
> Keep an SCM gate for need_mem_protection and add a separate PAS gate for
> need_pas_mem_setup. This is what I did to get it working:
> 
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -2079,7 +2079,16 @@ static int q6v5_probe(struct platform_device *pdev)
>  	if (!desc)
>  		return -EINVAL;
>  
> -	if (desc->need_mem_protection && !qcom_pas_is_available())
> +	/*
> +	 * Memory protection is done through qcom_scm_assign_mem(), which needs
> +	 * SCM but not PAS. Only the memory setup path issues PAS calls, so
> +	 * requiring PAS for every need_mem_protection platform prevents the
> +	 * modem from probing at all on TZ firmware that offers no PAS.
> +	 */
> +	if (desc->need_mem_protection && !qcom_scm_is_available())
> +		return -EPROBE_DEFER;
> +
> +	if (desc->need_pas_mem_setup && !qcom_pas_is_available())
>  		return -EPROBE_DEFER;

And likewise this looks like the correct fix too, feel free to submit
both as patches

[...]

> On the API contract
> ===================
> 
> For what it is worth, the kernel-doc for qcom_pas_is_available()
> (qcom_pas.c:256) reads:
> 
>   Note that it is mandatory for any PAS client to invoke this API.
>   If it returns true then only any other PAS API can be invoked.
> 
> That is a guard to call before invoking a PAS API, not a statement that a
> driver touching TZ at all should refuse to probe without PAS.
> ipa_main.c:764 consults it only on the loader path that genuinely needs
> PAS rather than as a blanket probe precondition, which is the shape both
> fixes above are aiming for.

This is a sloppiness that got through the transition patchset.. previously
as you mentioned, qcom_scm_is_available() was a blanket "can we talk to TZ
yet"?, and as the conversion happened, it was largely just find-and-replaced..

Konrad

  parent reply	other threads:[~2026-08-17 15:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-08  3:47 [REGRESSION] qcom PAS TZ API migration breaks GPU and modem on TrustZone without PAS (sc7180 trogdor) Paul Hollinsky
2026-08-08  3:55 ` sashiko-bot
2026-08-17 15:41 ` Konrad Dybcio [this message]
2026-08-21  6:33   ` Thorsten Leemhuis
2026-08-21  8:27     ` Paul Hollinsky
2026-08-21  8:37       ` Thorsten Leemhuis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b63e93e4-2f4c-4cad-b726-e1b0565379d7@gmail.com \
    --to=konradybcio@gmail.com \
    --cc=abhinav.kumar@linux.dev \
    --cc=akhilpo@oss.qualcomm.com \
    --cc=andersson@kernel.org \
    --cc=cros-qcom-dts-watchers@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=konradybcio@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=lumag@kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=mukesh.ojha@oss.qualcomm.com \
    --cc=phollinsky@holtechnik.com \
    --cc=regressions@lists.linux.dev \
    --cc=robin.clark@oss.qualcomm.com \
    --cc=sumit.garg@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.