* Re: [PATCH v2] iommu: arm-smmu-qcom: Ensure smmu is powered up in set_ttbr0_cfg
[not found] <20260325-qcom_smmu_pmfix-v2-1-ba769a6ad0be@gmail.com>
@ 2026-05-07 8:21 ` Xilin Wu
2026-05-07 13:27 ` Anna Maniscalco
2026-05-07 14:12 ` Rob Clark
2026-05-07 14:47 ` Robin Murphy
2 siblings, 1 reply; 5+ messages in thread
From: Xilin Wu @ 2026-05-07 8:21 UTC (permalink / raw)
To: Anna Maniscalco, Rob Clark, Will Deacon, Robin Murphy,
Joerg Roedel
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel
On 3/26/2026 5:11 AM, Anna Maniscalco wrote:
> Previously the device was being accessed while potentially in a
> suspended state.
>
> Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
> ---
> Changes in v2:
> - Simplify patch by acquiring device just around the call that needs it
> - Link to v1: https://lore.kernel.org/r/20260210-qcom_smmu_pmfix-v1-1-78b7143ac053@gmail.com
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 573085349df3..cab7d110aaf5 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -231,6 +231,7 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const void *cookie,
> struct io_pgtable *pgtable = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
> struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
> struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx];
> + int ret;
>
> /* The domain must have split pagetables already enabled */
> if (cb->tcr[0] & ARM_SMMU_TCR_EPD1)
> @@ -260,8 +261,16 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const void *cookie,
> cb->ttbr[0] |= FIELD_PREP(ARM_SMMU_TTBRn_ASID, cb->cfg->asid);
> }
>
> + ret = pm_runtime_resume_and_get(smmu_domain->smmu->dev);
> + if (ret < 0) {
> + dev_err(smmu_domain->smmu->dev, "failed to get runtime PM: %d\n", ret);
> + return -ENODEV;
> + }
> +
> arm_smmu_write_context_bank(smmu_domain->smmu, cb->cfg->cbndx);
>
> + pm_runtime_put_autosuspend(smmu_domain->smmu->dev);
> +
> return 0;
> }
>
>
> ---
> base-commit: 50c4a49f7292b33b454ea1a16c4f77d6965405dc
> change-id: 20260210-qcom_smmu_pmfix-2aead2ba4e20
>
> Best regards,
May I ask what is the status of this patch? Without this patch, I can
trigger a crash easily on sc8280xp by running fastfetch multiple times.
Tested-by: Xilin Wu <sophon@radxa.com> # sc8280xp-radxa-dragon-q8b
--
Best regards,
Xilin Wu <sophon@radxa.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] iommu: arm-smmu-qcom: Ensure smmu is powered up in set_ttbr0_cfg
2026-05-07 8:21 ` [PATCH v2] iommu: arm-smmu-qcom: Ensure smmu is powered up in set_ttbr0_cfg Xilin Wu
@ 2026-05-07 13:27 ` Anna Maniscalco
0 siblings, 0 replies; 5+ messages in thread
From: Anna Maniscalco @ 2026-05-07 13:27 UTC (permalink / raw)
To: Xilin Wu, Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel
On 5/7/26 10:21 AM, Xilin Wu wrote:
> On 3/26/2026 5:11 AM, Anna Maniscalco wrote:
>> Previously the device was being accessed while potentially in a
>> suspended state.
>>
>> Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
>> ---
>> Changes in v2:
>> - Simplify patch by acquiring device just around the call that needs it
>> - Link to v1:
>> https://lore.kernel.org/r/20260210-qcom_smmu_pmfix-v1-1-78b7143ac053@gmail.com
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 573085349df3..cab7d110aaf5 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -231,6 +231,7 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const
>> void *cookie,
>> struct io_pgtable *pgtable =
>> io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
>> struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
>> struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx];
>> + int ret;
>> /* The domain must have split pagetables already enabled */
>> if (cb->tcr[0] & ARM_SMMU_TCR_EPD1)
>> @@ -260,8 +261,16 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const
>> void *cookie,
>> cb->ttbr[0] |= FIELD_PREP(ARM_SMMU_TTBRn_ASID, cb->cfg->asid);
>> }
>> + ret = pm_runtime_resume_and_get(smmu_domain->smmu->dev);
>> + if (ret < 0) {
>> + dev_err(smmu_domain->smmu->dev, "failed to get runtime PM:
>> %d\n", ret);
>> + return -ENODEV;
>> + }
>> +
>> arm_smmu_write_context_bank(smmu_domain->smmu, cb->cfg->cbndx);
>> + pm_runtime_put_autosuspend(smmu_domain->smmu->dev);
>> +
>> return 0;
>> }
>>
>> ---
>> base-commit: 50c4a49f7292b33b454ea1a16c4f77d6965405dc
>> change-id: 20260210-qcom_smmu_pmfix-2aead2ba4e20
>>
>> Best regards,
>
> May I ask what is the status of this patch? Without this patch, I can
> trigger a crash easily on sc8280xp by running fastfetch multiple times.
It's ready on my side so if someone could review I think it could be
merged.
>
> Tested-by: Xilin Wu <sophon@radxa.com> # sc8280xp-radxa-dragon-q8b
>
Best regards,
--
Anna Maniscalco <anna.maniscalco2000@gmail.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] iommu: arm-smmu-qcom: Ensure smmu is powered up in set_ttbr0_cfg
[not found] <20260325-qcom_smmu_pmfix-v2-1-ba769a6ad0be@gmail.com>
2026-05-07 8:21 ` [PATCH v2] iommu: arm-smmu-qcom: Ensure smmu is powered up in set_ttbr0_cfg Xilin Wu
@ 2026-05-07 14:12 ` Rob Clark
2026-05-07 14:47 ` Robin Murphy
2 siblings, 0 replies; 5+ messages in thread
From: Rob Clark @ 2026-05-07 14:12 UTC (permalink / raw)
To: Anna Maniscalco
Cc: Will Deacon, Robin Murphy, Joerg Roedel, iommu, linux-arm-msm,
linux-arm-kernel, linux-kernel
On Wed, Mar 25, 2026 at 2:11 PM Anna Maniscalco
<anna.maniscalco2000@gmail.com> wrote:
>
> Previously the device was being accessed while potentially in a
> suspended state.
>
> Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
Reviewed-by: Rob Clark <rob.clark@oss.qualcomm.com>
> ---
> Changes in v2:
> - Simplify patch by acquiring device just around the call that needs it
> - Link to v1: https://lore.kernel.org/r/20260210-qcom_smmu_pmfix-v1-1-78b7143ac053@gmail.com
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 573085349df3..cab7d110aaf5 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -231,6 +231,7 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const void *cookie,
> struct io_pgtable *pgtable = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
> struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
> struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx];
> + int ret;
>
> /* The domain must have split pagetables already enabled */
> if (cb->tcr[0] & ARM_SMMU_TCR_EPD1)
> @@ -260,8 +261,16 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const void *cookie,
> cb->ttbr[0] |= FIELD_PREP(ARM_SMMU_TTBRn_ASID, cb->cfg->asid);
> }
>
> + ret = pm_runtime_resume_and_get(smmu_domain->smmu->dev);
> + if (ret < 0) {
> + dev_err(smmu_domain->smmu->dev, "failed to get runtime PM: %d\n", ret);
> + return -ENODEV;
> + }
> +
> arm_smmu_write_context_bank(smmu_domain->smmu, cb->cfg->cbndx);
>
> + pm_runtime_put_autosuspend(smmu_domain->smmu->dev);
> +
> return 0;
> }
>
>
> ---
> base-commit: 50c4a49f7292b33b454ea1a16c4f77d6965405dc
> change-id: 20260210-qcom_smmu_pmfix-2aead2ba4e20
>
> Best regards,
> --
> Anna Maniscalco <anna.maniscalco2000@gmail.com>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] iommu: arm-smmu-qcom: Ensure smmu is powered up in set_ttbr0_cfg
[not found] <20260325-qcom_smmu_pmfix-v2-1-ba769a6ad0be@gmail.com>
2026-05-07 8:21 ` [PATCH v2] iommu: arm-smmu-qcom: Ensure smmu is powered up in set_ttbr0_cfg Xilin Wu
2026-05-07 14:12 ` Rob Clark
@ 2026-05-07 14:47 ` Robin Murphy
2026-05-07 15:44 ` Anna Maniscalco
2 siblings, 1 reply; 5+ messages in thread
From: Robin Murphy @ 2026-05-07 14:47 UTC (permalink / raw)
To: Anna Maniscalco, Rob Clark, Will Deacon, Joerg Roedel
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel
On 25/03/2026 9:11 pm, Anna Maniscalco wrote:
> Previously the device was being accessed while potentially in a
> suspended state.
The SMMU driver's own write_context_bank() calls all look to be safely
within arm_smmu_rpm_get() scopes that are holding an RPM count when
relevant (or the one at probe time before RPM is activated at all), so
indeed this seems like the appropriate solution for here where we're
free to assume that Adreno platforms must always have a power domain.
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Just a minor nit: it's probably OK in this case (unless Will disagrees),
but for future patches, it's preferable for the commit message to be
self-contained, so as to give a sufficiently clear description of the
problem, and summary of the solution, without having to be read in the
context of the title and the whole diff to make sense. For example, if I
were writing this I'd put it something like:
"
iommu/arm-smmu-qcom: Ensure SMMU is powered up in set_ttbr0_cfg
arm_smmu_write_context_bank() assumes it is being called with RPM
active, but it turns out that is not guaranteed in the path from
qcom_adreno_smmu_set_ttbr0_cfg(), so it's possible for the the
register accesses to lead to an [external abort/hang/whatever] when
[doing whatever it is] while the GPU is idle. Add the RPM calls here
to make sure the SMMU is active before we touch it.
"
Thanks,
Robin.
> Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
> ---
> Changes in v2:
> - Simplify patch by acquiring device just around the call that needs it
> - Link to v1: https://lore.kernel.org/r/20260210-qcom_smmu_pmfix-v1-1-78b7143ac053@gmail.com
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 573085349df3..cab7d110aaf5 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -231,6 +231,7 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const void *cookie,
> struct io_pgtable *pgtable = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
> struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
> struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx];
> + int ret;
>
> /* The domain must have split pagetables already enabled */
> if (cb->tcr[0] & ARM_SMMU_TCR_EPD1)
> @@ -260,8 +261,16 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const void *cookie,
> cb->ttbr[0] |= FIELD_PREP(ARM_SMMU_TTBRn_ASID, cb->cfg->asid);
> }
>
> + ret = pm_runtime_resume_and_get(smmu_domain->smmu->dev);
> + if (ret < 0) {
> + dev_err(smmu_domain->smmu->dev, "failed to get runtime PM: %d\n", ret);
> + return -ENODEV;
> + }
> +
> arm_smmu_write_context_bank(smmu_domain->smmu, cb->cfg->cbndx);
>
> + pm_runtime_put_autosuspend(smmu_domain->smmu->dev);
> +
> return 0;
> }
>
>
> ---
> base-commit: 50c4a49f7292b33b454ea1a16c4f77d6965405dc
> change-id: 20260210-qcom_smmu_pmfix-2aead2ba4e20
>
> Best regards,
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] iommu: arm-smmu-qcom: Ensure smmu is powered up in set_ttbr0_cfg
2026-05-07 14:47 ` Robin Murphy
@ 2026-05-07 15:44 ` Anna Maniscalco
0 siblings, 0 replies; 5+ messages in thread
From: Anna Maniscalco @ 2026-05-07 15:44 UTC (permalink / raw)
To: Robin Murphy, Rob Clark, Will Deacon, Joerg Roedel
Cc: iommu, linux-arm-msm, linux-arm-kernel, linux-kernel
On 5/7/26 4:47 PM, Robin Murphy wrote:
> On 25/03/2026 9:11 pm, Anna Maniscalco wrote:
>> Previously the device was being accessed while potentially in a
>> suspended state.
>
> The SMMU driver's own write_context_bank() calls all look to be safely
> within arm_smmu_rpm_get() scopes that are holding an RPM count when
> relevant (or the one at probe time before RPM is activated at all), so
> indeed this seems like the appropriate solution for here where we're
> free to assume that Adreno platforms must always have a power domain.
>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
>
> Just a minor nit: it's probably OK in this case (unless Will
> disagrees), but for future patches, it's preferable for the commit
> message to be self-contained, so as to give a sufficiently clear
> description of the problem, and summary of the solution, without
> having to be read in the context of the title and the whole diff to
> make sense. For example, if I were writing this I'd put it something
> like:
>
> "
> iommu/arm-smmu-qcom: Ensure SMMU is powered up in set_ttbr0_cfg
>
> arm_smmu_write_context_bank() assumes it is being called with RPM
> active, but it turns out that is not guaranteed in the path from
> qcom_adreno_smmu_set_ttbr0_cfg(), so it's possible for the the
> register accesses to lead to an [external abort/hang/whatever] when
> [doing whatever it is] while the GPU is idle. Add the RPM calls here
> to make sure the SMMU is active before we touch it.
> "
>
> Thanks,
> Robin.
Thanks for the feedback, I've sent a v3 with an improved commit message.
>
>> Signed-off-by: Anna Maniscalco <anna.maniscalco2000@gmail.com>
>> ---
>> Changes in v2:
>> - Simplify patch by acquiring device just around the call that needs it
>> - Link to v1:
>> https://lore.kernel.org/r/20260210-qcom_smmu_pmfix-v1-1-78b7143ac053@gmail.com
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 573085349df3..cab7d110aaf5 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -231,6 +231,7 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const
>> void *cookie,
>> struct io_pgtable *pgtable =
>> io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
>> struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
>> struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx];
>> + int ret;
>> /* The domain must have split pagetables already enabled */
>> if (cb->tcr[0] & ARM_SMMU_TCR_EPD1)
>> @@ -260,8 +261,16 @@ static int qcom_adreno_smmu_set_ttbr0_cfg(const
>> void *cookie,
>> cb->ttbr[0] |= FIELD_PREP(ARM_SMMU_TTBRn_ASID, cb->cfg->asid);
>> }
>> + ret = pm_runtime_resume_and_get(smmu_domain->smmu->dev);
>> + if (ret < 0) {
>> + dev_err(smmu_domain->smmu->dev, "failed to get runtime PM:
>> %d\n", ret);
>> + return -ENODEV;
>> + }
>> +
>> arm_smmu_write_context_bank(smmu_domain->smmu, cb->cfg->cbndx);
>> + pm_runtime_put_autosuspend(smmu_domain->smmu->dev);
>> +
>> return 0;
>> }
>>
>> ---
>> base-commit: 50c4a49f7292b33b454ea1a16c4f77d6965405dc
>> change-id: 20260210-qcom_smmu_pmfix-2aead2ba4e20
>>
>> Best regards,
>
Best regards,
--
Anna Maniscalco <anna.maniscalco2000@gmail.com>
^ permalink raw reply [flat|nested] 5+ messages in thread