* [PATCH v3 01/13] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-10 6:59 ` Krzysztof Kozlowski
2026-09-09 14:57 ` [PATCH v3 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes Vikash Garodia
` (11 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Daniel J Blueman
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. The non-pixel stream
cannot address the low 600MB of IOVA space, while the pixel stream can
address the full range:
+-----------------------------------------------------------+
| non-pixel stream addressable range (600 MB - 3.5 GB) |
| 0x25800000 - 0xe0000000 |
+-----------------------------------------------------------+
| pixel stream addressable range (0 - 3.5 GB) |
| 0x00000000 - 0xe0000000 |
+-----------------------------------------------------------+
A single "iommus" property on the video-codec node puts every stream in
one IOMMU domain sharing one IOVA allocator, so nothing restricts a
non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
that boundary the hardware faults, which shows up as unhandled SMMU page
faults and spontaneous reboots:
https://gitlab.freedesktop.org/drm/msm/-/work_items/100
Given that the address range restriction is for specific VPU stream, it
should be ideally be moved to that stream. To achieve the same, a subset
of streams is now represented as subnodes, so that each can be
associated with its respective addressable range. The design was
discussed and agreed by mainatiners here
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
In the past, this limitation was addressed with an iommu-map approach,
with the iris driver dynamically creating the devices. That approach was
later concluded to be a hack to avoid having subnodes, and was NAKed by
the iommu maintainers. It was discussed in detail here:
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
"iommu-ranges" on the subnode describes the *allowed* IOVA range that
stream is allowed to use, so the IOVA is allocated from the specified
range only. Define all the possible subnodes so as to describe all the
VPU hardware iommu interfaces, both secure as well as non secure.
address-cells, size-cells and dma-ranges declares the 1:1 DMA
translation into the parent.
The parent "iommus" is kept as an alternative via "oneOf", so platforms
that have not been converted to subnodes still validate. New platforms
should use the subnode form.
This patch depends-on:
https://github.com/devicetree-org/dt-schema/pull/207
Tested-by: Daniel J Blueman <daniel@quora.org>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
.../bindings/media/qcom,sc7180-venus.yaml | 15 ----
.../bindings/media/qcom,venus-common.yaml | 97 ++++++++++++++++++++++
2 files changed, 97 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
index b21bed314848480b82153e49602f0b19e08e7335..bfd8b1ad473128c974bce84639cb0aff59d8c2cc 100644
--- a/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
@@ -91,21 +91,6 @@ properties:
deprecated: true
additionalProperties: false
- video-firmware:
- type: object
- additionalProperties: false
-
- description: |
- Firmware subnode is needed when the platform does not
- have TrustZone.
-
- properties:
- iommus:
- maxItems: 1
-
- required:
- - iommus
-
required:
- compatible
- power-domain-names
diff --git a/Documentation/devicetree/bindings/media/qcom,venus-common.yaml b/Documentation/devicetree/bindings/media/qcom,venus-common.yaml
index 59a3fde846d2196ab1e4588eb396012ba6860712..68d5e592a028c5eecde04ce72cd5ae6815ba5c82 100644
--- a/Documentation/devicetree/bindings/media/qcom,venus-common.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,venus-common.yaml
@@ -47,6 +47,93 @@ properties:
minItems: 1
maxItems: 4
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 1
+
+ dma-ranges:
+ maxItems: 1
+
+ non-pixel:
+ type: object
+ description:
+ Context bank for VPU non-pixel buffers, including compressed and internal buffers.
+ additionalProperties: false
+
+ properties:
+ iommus:
+ maxItems: 1
+ iommu-ranges:
+ maxItems: 1
+ required:
+ - iommus
+ - iommu-ranges
+
+ pixel:
+ type: object
+ description:
+ Context bank for VPU pixel buffers containing uncompressed video data.
+ additionalProperties: false
+
+ properties:
+ iommus:
+ maxItems: 1
+ required:
+ - iommus
+
+ video-firmware:
+ type: object
+ description:
+ Context bank for the VPU firmware processing domain.
+ additionalProperties: false
+
+ properties:
+ iommus:
+ maxItems: 1
+ required:
+ - iommus
+
+ secure-non-pixel:
+ type: object
+ description:
+ Context bank for VPU secure non-pixel buffers.
+ additionalProperties: false
+
+ properties:
+ iommus:
+ maxItems: 1
+ iommu-ranges:
+ maxItems: 1
+ required:
+ - iommus
+ - iommu-ranges
+
+ secure-pixel:
+ type: object
+ description:
+ Context bank for VPU secure pixel buffers containing uncompressed video data.
+ additionalProperties: false
+
+ properties:
+ iommus:
+ maxItems: 1
+ required:
+ - iommus
+
+ secure-bitstream:
+ type: object
+ description:
+ Context bank for VPU secure bitstream buffers containing compressed video data.
+ additionalProperties: false
+
+ properties:
+ iommus:
+ maxItems: 1
+ required:
+ - iommus
+
required:
- reg
- clocks
@@ -55,4 +142,14 @@ required:
- memory-region
- power-domains
+oneOf:
+ - required:
+ - iommus
+ - required:
+ - '#address-cells'
+ - '#size-cells'
+ - dma-ranges
+ - non-pixel
+ - pixel
+
additionalProperties: true
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 01/13] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema
2026-09-09 14:57 ` [PATCH v3 01/13] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema Vikash Garodia
@ 2026-09-10 6:59 ` Krzysztof Kozlowski
2026-09-11 17:43 ` Vikash Garodia
0 siblings, 1 reply; 31+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-10 6:59 UTC (permalink / raw)
To: Vikash Garodia
Cc: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy,
linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, iommu, Daniel J Blueman
On Wed, Sep 09, 2026 at 08:27:08PM +0530, Vikash Garodia wrote:
> The VPU issues DMA through several SMMU streams, and the hardware does
> not give every stream the same addressable range. The non-pixel stream
> cannot address the low 600MB of IOVA space, while the pixel stream can
> address the full range:
> +-----------------------------------------------------------+
> | non-pixel stream addressable range (600 MB - 3.5 GB) |
> | 0x25800000 - 0xe0000000 |
> +-----------------------------------------------------------+
> | pixel stream addressable range (0 - 3.5 GB) |
> | 0x00000000 - 0xe0000000 |
> +-----------------------------------------------------------+
> A single "iommus" property on the video-codec node puts every stream in
> one IOMMU domain sharing one IOVA allocator, so nothing restricts a
> non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
> that boundary the hardware faults, which shows up as unhandled SMMU page
> faults and spontaneous reboots:
> https://gitlab.freedesktop.org/drm/msm/-/work_items/100
>
> Given that the address range restriction is for specific VPU stream, it
> should be ideally be moved to that stream. To achieve the same, a subset
> of streams is now represented as subnodes, so that each can be
> associated with its respective addressable range. The design was
> discussed and agreed by mainatiners here
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
>
> In the past, this limitation was addressed with an iommu-map approach,
> with the iris driver dynamically creating the devices. That approach was
> later concluded to be a hack to avoid having subnodes, and was NAKed by
> the iommu maintainers. It was discussed in detail here:
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
>
> "iommu-ranges" on the subnode describes the *allowed* IOVA range that
> stream is allowed to use, so the IOVA is allocated from the specified
> range only. Define all the possible subnodes so as to describe all the
> VPU hardware iommu interfaces, both secure as well as non secure.
>
> address-cells, size-cells and dma-ranges declares the 1:1 DMA
> translation into the parent.
>
> The parent "iommus" is kept as an alternative via "oneOf", so platforms
> that have not been converted to subnodes still validate. New platforms
> should use the subnode form.
>
> This patch depends-on:
> https://github.com/devicetree-org/dt-schema/pull/207
Please drop, not suitable for commit, keep it in the changelog
>
> Tested-by: Daniel J Blueman <daniel@quora.org>
Plese drop, not true here
> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
> ---
> .../bindings/media/qcom,sc7180-venus.yaml | 15 ----
> .../bindings/media/qcom,venus-common.yaml | 97 ++++++++++++++++++++++
> 2 files changed, 97 insertions(+), 15 deletions(-)
>
I see this causes DT checks failures but I don't see the report, so
probably it was because of schema dependency.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 01/13] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema
2026-09-10 6:59 ` Krzysztof Kozlowski
@ 2026-09-11 17:43 ` Vikash Garodia
0 siblings, 0 replies; 31+ messages in thread
From: Vikash Garodia @ 2026-09-11 17:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy,
linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, iommu, Daniel J Blueman
On 9/10/2026 12:29 PM, Krzysztof Kozlowski wrote:
> On Wed, Sep 09, 2026 at 08:27:08PM +0530, Vikash Garodia wrote:
>> The VPU issues DMA through several SMMU streams, and the hardware does
>> not give every stream the same addressable range. The non-pixel stream
>> cannot address the low 600MB of IOVA space, while the pixel stream can
>> address the full range:
>> +-----------------------------------------------------------+
>> | non-pixel stream addressable range (600 MB - 3.5 GB) |
>> | 0x25800000 - 0xe0000000 |
>> +-----------------------------------------------------------+
>> | pixel stream addressable range (0 - 3.5 GB) |
>> | 0x00000000 - 0xe0000000 |
>> +-----------------------------------------------------------+
>> A single "iommus" property on the video-codec node puts every stream in
>> one IOMMU domain sharing one IOVA allocator, so nothing restricts a
>> non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
>> that boundary the hardware faults, which shows up as unhandled SMMU page
>> faults and spontaneous reboots:
>> https://gitlab.freedesktop.org/drm/msm/-/work_items/100
>>
>> Given that the address range restriction is for specific VPU stream, it
>> should be ideally be moved to that stream. To achieve the same, a subset
>> of streams is now represented as subnodes, so that each can be
>> associated with its respective addressable range. The design was
>> discussed and agreed by mainatiners here
>> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
>>
>> In the past, this limitation was addressed with an iommu-map approach,
>> with the iris driver dynamically creating the devices. That approach was
>> later concluded to be a hack to avoid having subnodes, and was NAKed by
>> the iommu maintainers. It was discussed in detail here:
>> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
>>
>> "iommu-ranges" on the subnode describes the *allowed* IOVA range that
>> stream is allowed to use, so the IOVA is allocated from the specified
>> range only. Define all the possible subnodes so as to describe all the
>> VPU hardware iommu interfaces, both secure as well as non secure.
>>
>> address-cells, size-cells and dma-ranges declares the 1:1 DMA
>> translation into the parent.
>>
>> The parent "iommus" is kept as an alternative via "oneOf", so platforms
>> that have not been converted to subnodes still validate. New platforms
>> should use the subnode form.
>>
>> This patch depends-on:
>> https://github.com/devicetree-org/dt-schema/pull/207
>
> Please drop, not suitable for commit, keep it in the changelog
>
Sure, will add this to changelog.
>>
>> Tested-by: Daniel J Blueman <daniel@quora.org>
>
> Plese drop, not true here
>
ok, got picked in b4 trailers, will drop.
>
>> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
>> ---
>> .../bindings/media/qcom,sc7180-venus.yaml | 15 ----
>> .../bindings/media/qcom,venus-common.yaml | 97 ++++++++++++++++++++++
>> 2 files changed, 97 insertions(+), 15 deletions(-)
>>
>
> I see this causes DT checks failures but I don't see the report, so
> probably it was because of schema dependency.
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
2026-09-09 14:57 ` [PATCH v3 01/13] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 15:09 ` sashiko-bot
` (2 more replies)
2026-09-09 14:57 ` [PATCH v3 03/13] dt-bindings: media: qcom,sm8750-iris: " Vikash Garodia
` (10 subsequent siblings)
12 siblings, 3 replies; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Daniel J Blueman, Vishnu Reddy
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. The non-pixel stream
cannot address the low 600MB of IOVA space, while the pixel stream can
address the full range:
+-----------------------------------------------------------+
| non-pixel stream addressable range (600 MB - 3.5 GB) |
| 0x25800000 - 0xe0000000 |
+-----------------------------------------------------------+
| pixel stream addressable range (0 - 3.5 GB) |
| 0x00000000 - 0xe0000000 |
+-----------------------------------------------------------+
A single "iommus" property on the video-codec node puts every stream in
one IOMMU domain sharing one IOVA allocator, so nothing restricts a
non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
that boundary the hardware faults, which shows up as unhandled SMMU page
faults and spontaneous reboots:
https://gitlab.freedesktop.org/drm/msm/-/work_items/100
Given that the address range restriction is for specific VPU stream, it
should be ideally be moved to that stream. To achieve the same, a subset
of streams is now represented as subnodes, so that each can be
associated with its respective addressable range. The design was
discussed and agreed by mainatiners here
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
In the past, this limitation was addressed with an iommu-map approach,
with the iris driver dynamically creating the devices. That approach was
later concluded to be a hack to avoid having subnodes, and was NAKed by
the iommu maintainers. It was discussed in detail here:
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
Update example to the subnode form. Doing so, it picks up the supporting
properties needed on the video-codec node, address-cells, size-cells and
dma-ranges to declare 1:1 DMA translation into the parent. Given a soc
with address-cells and size-cells as 2, update the video-codec "reg"
accordingly.
This patch depends-on:
https://github.com/devicetree-org/dt-schema/pull/207
Tested-by: Daniel J Blueman <daniel@quora.org>
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
.../bindings/media/qcom,sm8550-iris.yaml | 118 ++++++++++++---------
1 file changed, 67 insertions(+), 51 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
index 0400ca1bff05dcef6b742c3fbf77e38adca9f280..dd80526838f1193f4c9105793ae4bf4fe0755ec2 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
@@ -98,10 +98,10 @@ required:
- interconnect-names
- resets
- reset-names
- - iommus
- dma-coherent
allOf:
+ - $ref: qcom,venus-common.yaml#
- if:
properties:
compatible:
@@ -150,70 +150,86 @@ examples:
#include <dt-bindings/power/qcom-rpmpd.h>
#include <dt-bindings/power/qcom,rpmhpd.h>
- video-codec@aa00000 {
- compatible = "qcom,sm8550-iris";
- reg = <0x0aa00000 0xf0000>;
- interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
- power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
- <&videocc VIDEO_CC_MVS0_GDSC>,
- <&rpmhpd RPMHPD_MXC>,
- <&rpmhpd RPMHPD_MMCX>;
- power-domain-names = "venus", "vcodec0", "mxc", "mmcx";
+ video-codec@aa00000 {
+ compatible = "qcom,sm8550-iris";
+ reg = <0x0 0x0aa00000 0x0 0xf0000>;
+ interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
- <&videocc VIDEO_CC_MVS0C_CLK>,
- <&videocc VIDEO_CC_MVS0_CLK>;
- clock-names = "iface", "core", "vcodec0_core";
+ power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
+ <&videocc VIDEO_CC_MVS0_GDSC>,
+ <&rpmhpd RPMHPD_MXC>,
+ <&rpmhpd RPMHPD_MMCX>;
+ power-domain-names = "venus", "vcodec0", "mxc", "mmcx";
- interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
- &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
- <&mmss_noc MASTER_VIDEO QCOM_ICC_TAG_ALWAYS
- &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
- interconnect-names = "cpu-cfg", "video-mem";
+ clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
+ <&videocc VIDEO_CC_MVS0C_CLK>,
+ <&videocc VIDEO_CC_MVS0_CLK>;
+ clock-names = "iface", "core", "vcodec0_core";
- memory-region = <&video_mem>;
+ interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
+ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>,
+ <&mmss_noc MASTER_VIDEO QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+ interconnect-names = "cpu-cfg", "video-mem";
- resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
- reset-names = "bus";
+ memory-region = <&video_mem>;
- iommus = <&apps_smmu 0x1940 0x0000>,
- <&apps_smmu 0x1947 0x0000>;
- dma-coherent;
+ resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
+ reset-names = "bus";
- operating-points-v2 = <&iris_opp_table>;
+ dma-coherent;
- iris_opp_table: opp-table {
- compatible = "operating-points-v2";
+ operating-points-v2 = <&iris_opp_table>;
- opp-240000000 {
- opp-hz = /bits/ 64 <240000000>;
- required-opps = <&rpmhpd_opp_svs>,
- <&rpmhpd_opp_low_svs>;
- };
-
- opp-338000000 {
- opp-hz = /bits/ 64 <338000000>;
- required-opps = <&rpmhpd_opp_svs>,
- <&rpmhpd_opp_svs>;
- };
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges = <0x0 0x0 0x0 0xe0000000>;
- opp-366000000 {
- opp-hz = /bits/ 64 <366000000>;
- required-opps = <&rpmhpd_opp_svs_l1>,
- <&rpmhpd_opp_svs_l1>;
+ non-pixel {
+ iommus = <&apps_smmu 0x1940 0x0>;
+ iommu-ranges = <0x25800000 0xba800000>;
};
- opp-444000000 {
- opp-hz = /bits/ 64 <444000000>;
- required-opps = <&rpmhpd_opp_turbo>,
- <&rpmhpd_opp_turbo>;
+ pixel {
+ iommus = <&apps_smmu 0x1947 0x0>;
};
- opp-533333334 {
- opp-hz = /bits/ 64 <533333334>;
- required-opps = <&rpmhpd_opp_turbo_l1>,
- <&rpmhpd_opp_turbo_l1>;
+ iris_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-240000000 {
+ opp-hz = /bits/ 64 <240000000>;
+ required-opps = <&rpmhpd_opp_svs>,
+ <&rpmhpd_opp_low_svs>;
+ };
+
+ opp-338000000 {
+ opp-hz = /bits/ 64 <338000000>;
+ required-opps = <&rpmhpd_opp_svs>,
+ <&rpmhpd_opp_svs>;
+ };
+
+ opp-366000000 {
+ opp-hz = /bits/ 64 <366000000>;
+ required-opps = <&rpmhpd_opp_svs_l1>,
+ <&rpmhpd_opp_svs_l1>;
+ };
+
+ opp-444000000 {
+ opp-hz = /bits/ 64 <444000000>;
+ required-opps = <&rpmhpd_opp_turbo>,
+ <&rpmhpd_opp_turbo>;
+ };
+
+ opp-533333334 {
+ opp-hz = /bits/ 64 <533333334>;
+ required-opps = <&rpmhpd_opp_turbo_l1>,
+ <&rpmhpd_opp_turbo_l1>;
+ };
};
};
};
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes
2026-09-09 14:57 ` [PATCH v3 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes Vikash Garodia
@ 2026-09-09 15:09 ` sashiko-bot
2026-09-09 20:57 ` Dmitry Baryshkov
2026-09-10 7:00 ` Krzysztof Kozlowski
2 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-09-09 15:09 UTC (permalink / raw)
To: Vikash Garodia; +Cc: conor+dt, devicetree, robh, media-ci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] Missing 'ranges' property in the newly added 'soc' wrapper node in the example.
Pre-existing issues:
- [High] Missing matching updates for related Iris family device binding (qcom,milos-iris.yaml) to address the same hardware SMMU limitation.
--
commit b9e07ce021dac6c49fe1c64b1aac26288fe5cc3f
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
Subject: dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes
This commit addresses an SMMU hardware limitation where non-pixel streams
cannot address the low 600MB of IOVA space by moving stream constraints
into subnodes. It updates the dt-bindings example to use the new subnode
format and adds required 1:1 DMA translation properties.
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> index 0400ca1bff05d..dd80526838f11 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
[Severity: High]
This isn't a bug introduced by this patch, but this series fixes the
identical hardware flaw for sm8550 and sm8750 while leaving milos behind.
Should there be a matching update for the related Iris family device binding
(qcom,milos-iris.yaml) to address the same hardware SMMU limitation?
The qcom,milos-iris.yaml binding continues to use the flawed dual-stream
top-level approach, leaving it exposed to the SMMU page faults:
Documentation/devicetree/bindings/media/qcom,milos-iris.yaml:video-codec node
iommus = <&apps_smmu 0x1960 0>,
<&apps_smmu 0x1967 0>;
When the Iris VPU on Milos performs DMA for non-pixel streams below the
600MB IOVA boundary, the hardware SMMU will fault because the ranges are
not correctly restricted. Video decoding/encoding workloads triggered by
userspace applications will hit this IOVA allocation boundary over time.
The dt-bindings subsystem guide explicitly mandates matching updates for
related files in the same family. While the later driver patch falls back
to the old behavior for platforms lacking subnodes to maintain backward
compatibility, that old behavior is exactly what causes the crash this
series aims to fix.
[ ... ]
> @@ -150,70 +150,86 @@ examples:
> #include <dt-bindings/power/qcom-rpmpd.h>
> #include <dt-bindings/power/qcom,rpmhpd.h>
>
> - video-codec@aa00000 {
> - compatible = "qcom,sm8550-iris";
> - reg = <0x0aa00000 0xf0000>;
> - interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
[Severity: Low]
Does this soc wrapper node also need an empty ranges property to properly
translate memory addresses for its children?
The patch wraps the video-codec child in a soc node to demonstrate 1:1 DMA
translation. However, a bus node mapping 1:1 memory addresses for its
children must have an empty ranges property. Without it, the child's reg
addresses cannot be translated to the root CPU address space, making the DT
example technically invalid when running standard DT validation tools.
> +
> + video-codec@aa00000 {
> + compatible = "qcom,sm8550-iris";
> + reg = <0x0 0x0aa00000 0x0 0xf0000>;
> + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-vpu_iommu_iova_handling-v3-0-2fe43ae45495@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes
2026-09-09 14:57 ` [PATCH v3 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes Vikash Garodia
2026-09-09 15:09 ` sashiko-bot
@ 2026-09-09 20:57 ` Dmitry Baryshkov
2026-09-10 7:00 ` Krzysztof Kozlowski
2 siblings, 0 replies; 31+ messages in thread
From: Dmitry Baryshkov @ 2026-09-09 20:57 UTC (permalink / raw)
To: Vikash Garodia
Cc: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Bryan O'Donoghue, Stephan Gerhold,
Joerg Roedel (AMD), Will Deacon, Robin Murphy, linux-media,
linux-arm-msm, devicetree, linux-kernel, Konrad Dybcio,
Krzysztof Kozlowski, iommu, Daniel J Blueman, Vishnu Reddy
On Wed, Sep 09, 2026 at 08:27:09PM +0530, Vikash Garodia wrote:
> The VPU issues DMA through several SMMU streams, and the hardware does
> not give every stream the same addressable range. The non-pixel stream
> cannot address the low 600MB of IOVA space, while the pixel stream can
> address the full range:
> +-----------------------------------------------------------+
> | non-pixel stream addressable range (600 MB - 3.5 GB) |
> | 0x25800000 - 0xe0000000 |
> +-----------------------------------------------------------+
> | pixel stream addressable range (0 - 3.5 GB) |
> | 0x00000000 - 0xe0000000 |
> +-----------------------------------------------------------+
> A single "iommus" property on the video-codec node puts every stream in
> one IOMMU domain sharing one IOVA allocator, so nothing restricts a
> non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
> that boundary the hardware faults, which shows up as unhandled SMMU page
> faults and spontaneous reboots:
> https://gitlab.freedesktop.org/drm/msm/-/work_items/100
>
> Given that the address range restriction is for specific VPU stream, it
> should be ideally be moved to that stream. To achieve the same, a subset
> of streams is now represented as subnodes, so that each can be
> associated with its respective addressable range. The design was
> discussed and agreed by mainatiners here
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
>
> In the past, this limitation was addressed with an iommu-map approach,
> with the iris driver dynamically creating the devices. That approach was
> later concluded to be a hack to avoid having subnodes, and was NAKed by
> the iommu maintainers. It was discussed in detail here:
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
>
> Update example to the subnode form. Doing so, it picks up the supporting
> properties needed on the video-codec node, address-cells, size-cells and
> dma-ranges to declare 1:1 DMA translation into the parent. Given a soc
> with address-cells and size-cells as 2, update the video-codec "reg"
> accordingly.
>
> This patch depends-on:
> https://github.com/devicetree-org/dt-schema/pull/207
>
> Tested-by: Daniel J Blueman <daniel@quora.org>
> Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
> Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
> ---
> .../bindings/media/qcom,sm8550-iris.yaml | 118 ++++++++++++---------
> 1 file changed, 67 insertions(+), 51 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> index 0400ca1bff05dcef6b742c3fbf77e38adca9f280..dd80526838f1193f4c9105793ae4bf4fe0755ec2 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> @@ -98,10 +98,10 @@ required:
> - interconnect-names
> - resets
> - reset-names
> - - iommus
> - dma-coherent
>
> allOf:
> + - $ref: qcom,venus-common.yaml#
> - if:
> properties:
> compatible:
> @@ -150,70 +150,86 @@ examples:
> #include <dt-bindings/power/qcom-rpmpd.h>
> #include <dt-bindings/power/qcom,rpmhpd.h>
>
> - video-codec@aa00000 {
> - compatible = "qcom,sm8550-iris";
> - reg = <0x0aa00000 0xf0000>;
> - interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
No. The SoC node must go away. None of the schema examples have it.
>
> - power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
> - <&videocc VIDEO_CC_MVS0_GDSC>,
> - <&rpmhpd RPMHPD_MXC>,
> - <&rpmhpd RPMHPD_MMCX>;
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes
2026-09-09 14:57 ` [PATCH v3 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes Vikash Garodia
2026-09-09 15:09 ` sashiko-bot
2026-09-09 20:57 ` Dmitry Baryshkov
@ 2026-09-10 7:00 ` Krzysztof Kozlowski
2026-09-11 17:46 ` Vikash Garodia
2 siblings, 1 reply; 31+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-10 7:00 UTC (permalink / raw)
To: Vikash Garodia
Cc: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy,
linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, iommu, Daniel J Blueman, Vishnu Reddy
On Wed, Sep 09, 2026 at 08:27:09PM +0530, Vikash Garodia wrote:
> The VPU issues DMA through several SMMU streams, and the hardware does
> not give every stream the same addressable range. The non-pixel stream
> cannot address the low 600MB of IOVA space, while the pixel stream can
> address the full range:
> +-----------------------------------------------------------+
> | non-pixel stream addressable range (600 MB - 3.5 GB) |
> | 0x25800000 - 0xe0000000 |
> +-----------------------------------------------------------+
> | pixel stream addressable range (0 - 3.5 GB) |
> | 0x00000000 - 0xe0000000 |
> +-----------------------------------------------------------+
> A single "iommus" property on the video-codec node puts every stream in
> one IOMMU domain sharing one IOVA allocator, so nothing restricts a
> non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
> that boundary the hardware faults, which shows up as unhandled SMMU page
> faults and spontaneous reboots:
> https://gitlab.freedesktop.org/drm/msm/-/work_items/100
>
> Given that the address range restriction is for specific VPU stream, it
> should be ideally be moved to that stream. To achieve the same, a subset
> of streams is now represented as subnodes, so that each can be
> associated with its respective addressable range. The design was
> discussed and agreed by mainatiners here
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
>
> In the past, this limitation was addressed with an iommu-map approach,
> with the iris driver dynamically creating the devices. That approach was
> later concluded to be a hack to avoid having subnodes, and was NAKed by
> the iommu maintainers. It was discussed in detail here:
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
>
> Update example to the subnode form. Doing so, it picks up the supporting
> properties needed on the video-codec node, address-cells, size-cells and
> dma-ranges to declare 1:1 DMA translation into the parent. Given a soc
> with address-cells and size-cells as 2, update the video-codec "reg"
> accordingly.
>
> This patch depends-on:
> https://github.com/devicetree-org/dt-schema/pull/207
>
> Tested-by: Daniel J Blueman <daniel@quora.org>
I already asked at v2.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes
2026-09-10 7:00 ` Krzysztof Kozlowski
@ 2026-09-11 17:46 ` Vikash Garodia
0 siblings, 0 replies; 31+ messages in thread
From: Vikash Garodia @ 2026-09-11 17:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy,
linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, iommu, Daniel J Blueman, Vishnu Reddy
On 9/10/2026 12:30 PM, Krzysztof Kozlowski wrote:
> On Wed, Sep 09, 2026 at 08:27:09PM +0530, Vikash Garodia wrote:
>> The VPU issues DMA through several SMMU streams, and the hardware does
>> not give every stream the same addressable range. The non-pixel stream
>> cannot address the low 600MB of IOVA space, while the pixel stream can
>> address the full range:
>> +-----------------------------------------------------------+
>> | non-pixel stream addressable range (600 MB - 3.5 GB) |
>> | 0x25800000 - 0xe0000000 |
>> +-----------------------------------------------------------+
>> | pixel stream addressable range (0 - 3.5 GB) |
>> | 0x00000000 - 0xe0000000 |
>> +-----------------------------------------------------------+
>> A single "iommus" property on the video-codec node puts every stream in
>> one IOMMU domain sharing one IOVA allocator, so nothing restricts a
>> non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
>> that boundary the hardware faults, which shows up as unhandled SMMU page
>> faults and spontaneous reboots:
>> https://gitlab.freedesktop.org/drm/msm/-/work_items/100
>>
>> Given that the address range restriction is for specific VPU stream, it
>> should be ideally be moved to that stream. To achieve the same, a subset
>> of streams is now represented as subnodes, so that each can be
>> associated with its respective addressable range. The design was
>> discussed and agreed by mainatiners here
>> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
>>
>> In the past, this limitation was addressed with an iommu-map approach,
>> with the iris driver dynamically creating the devices. That approach was
>> later concluded to be a hack to avoid having subnodes, and was NAKed by
>> the iommu maintainers. It was discussed in detail here:
>> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
>>
>> Update example to the subnode form. Doing so, it picks up the supporting
>> properties needed on the video-codec node, address-cells, size-cells and
>> dma-ranges to declare 1:1 DMA translation into the parent. Given a soc
>> with address-cells and size-cells as 2, update the video-codec "reg"
>> accordingly.
>>
>> This patch depends-on:
>> https://github.com/devicetree-org/dt-schema/pull/207
>>
>> Tested-by: Daniel J Blueman <daniel@quora.org>
>
> I already asked at v2.
Sure, will drop the tag and dependency PR will be added to changelog.
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 03/13] dt-bindings: media: qcom,sm8750-iris: Add context bank subnodes
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
2026-09-09 14:57 ` [PATCH v3 01/13] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema Vikash Garodia
2026-09-09 14:57 ` [PATCH v3 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 21:32 ` Dmitry Baryshkov
2026-09-09 14:57 ` [PATCH v3 04/13] iommu: of_iommu: Add support for "iommu-ranges" on a device node Vikash Garodia
` (9 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Vishnu Reddy
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. The non-pixel stream
cannot address the low 600MB of IOVA space, while the pixel stream can
address the full range:
+-----------------------------------------------------------+
| non-pixel stream addressable range (600 MB - 3.5 GB) |
| 0x25800000 - 0xe0000000 |
+-----------------------------------------------------------+
| pixel stream addressable range (0 - 3.5 GB) |
| 0x00000000 - 0xe0000000 |
+-----------------------------------------------------------+
A single "iommus" property on the video-codec node puts every stream in
one IOMMU domain sharing one IOVA allocator, so nothing restricts a
non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
that boundary the hardware faults, which shows up as unhandled SMMU page
faults and spontaneous reboots:
https://gitlab.freedesktop.org/drm/msm/-/work_items/100
Given that the address range restriction is for specific VPU stream, it
should be ideally be moved to that stream. To achieve the same, a subset
of streams is now represented as subnodes, so that each can be
associated with its respective addressable range. The design was discussed
and agreed by mainatiners here
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
In the past, this limitation was addressed with an iommu-map approach,
with the iris driver dynamically creating the devices. That approach
was later concluded to be a hack to avoid having subnodes, and was NAKed
by the iommu maintainers. It was discussed in detail here:
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
Update example to the subnode form. Doing so, it picks up the supporting
properties needed on the video-codec node, address-cells, size-cells and
dma-ranges to declare 1:1 DMA translation into the parent. Given a soc
with address-cells and size-cells as 2, update the video-codec "reg"
accordingly.
This patch depends-on:
https://github.com/devicetree-org/dt-schema/pull/207
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
.../bindings/media/qcom,sm8750-iris.yaml | 187 +++++++++++----------
1 file changed, 101 insertions(+), 86 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml
index c42d3470bdac796cc878090e65becf6b62bd80ca..eafd8654092468e4422de65c12fae8606babbd32 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml
@@ -72,7 +72,6 @@ required:
- dma-coherent
- interconnects
- interconnect-names
- - iommus
- power-domain-names
- resets
- reset-names
@@ -92,95 +91,111 @@ examples:
#include <dt-bindings/power/qcom-rpmpd.h>
#include <dt-bindings/power/qcom,rpmhpd.h>
- video-codec@aa00000 {
- compatible = "qcom,sm8750-iris";
- reg = <0x0aa00000 0xf0000>;
-
- clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
- <&videocc_mvs0c_clk>,
- <&videocc_mvs0_clk>,
- <&gcc GCC_VIDEO_AXI1_CLK>,
- <&videocc_mvs0c_freerun_clk>,
- <&videocc_mvs0_freerun_clk>;
- clock-names = "iface",
- "core",
- "vcodec0_core",
- "iface1",
- "core_freerun",
- "vcodec0_core_freerun";
-
- dma-coherent;
- iommus = <&apps_smmu 0x1940 0>,
- <&apps_smmu 0x1947 0>;
-
- interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
- &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
- <&mmss_noc MASTER_VIDEO_MVP QCOM_ICC_TAG_ALWAYS
- &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
- interconnect-names = "cpu-cfg",
- "video-mem";
-
- interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
-
- operating-points-v2 = <&iris_opp_table>;
-
- memory-region = <&video_mem>;
-
- power-domains = <&videocc_mvs0c_gdsc>,
- <&videocc_mvs0_gdsc>,
- <&rpmhpd RPMHPD_MXC>,
- <&rpmhpd RPMHPD_MMCX>;
- power-domain-names = "venus",
- "vcodec0",
- "mxc",
- "mmcx";
-
- resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>,
- <&gcc GCC_VIDEO_AXI1_CLK_ARES>,
- <&videocc_mvs0c_freerun_clk_ares>,
- <&videocc_mvs0_freerun_clk_ares>;
- reset-names = "bus0",
- "bus1",
- "core",
- "vcodec0_core";
-
- iris_opp_table: opp-table {
- compatible = "operating-points-v2";
-
- opp-240000000 {
- opp-hz = /bits/ 64 <240000000>;
- required-opps = <&rpmhpd_opp_low_svs_d1>,
- <&rpmhpd_opp_low_svs_d1>;
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ video-codec@aa00000 {
+ compatible = "qcom,sm8750-iris";
+ reg = <0x0 0x0aa00000 0x0 0xf0000>;
+
+ clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
+ <&videocc_mvs0c_clk>,
+ <&videocc_mvs0_clk>,
+ <&gcc GCC_VIDEO_AXI1_CLK>,
+ <&videocc_mvs0c_freerun_clk>,
+ <&videocc_mvs0_freerun_clk>;
+ clock-names = "iface",
+ "core",
+ "vcodec0_core",
+ "iface1",
+ "core_freerun",
+ "vcodec0_core_freerun";
+
+ dma-coherent;
+
+ interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
+ <&mmss_noc MASTER_VIDEO_MVP QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+ interconnect-names = "cpu-cfg",
+ "video-mem";
+
+ interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+
+ operating-points-v2 = <&iris_opp_table>;
+
+ memory-region = <&video_mem>;
+
+ power-domains = <&videocc_mvs0c_gdsc>,
+ <&videocc_mvs0_gdsc>,
+ <&rpmhpd RPMHPD_MXC>,
+ <&rpmhpd RPMHPD_MMCX>;
+ power-domain-names = "venus",
+ "vcodec0",
+ "mxc",
+ "mmcx";
+
+ resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>,
+ <&gcc GCC_VIDEO_AXI1_CLK_ARES>,
+ <&videocc_mvs0c_freerun_clk_ares>,
+ <&videocc_mvs0_freerun_clk_ares>;
+ reset-names = "bus0",
+ "bus1",
+ "core",
+ "vcodec0_core";
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges = <0x0 0x0 0x0 0xe0000000>;
+
+ non-pixel {
+ iommus = <&apps_smmu 0x1940 0x0>;
+ iommu-ranges = <0x25800000 0xba800000>;
};
- opp-338000000 {
- opp-hz = /bits/ 64 <338000000>;
- required-opps = <&rpmhpd_opp_low_svs>,
- <&rpmhpd_opp_low_svs>;
+ pixel {
+ iommus = <&apps_smmu 0x1947 0x0>;
};
- opp-420000000 {
- opp-hz = /bits/ 64 <420000000>;
- required-opps = <&rpmhpd_opp_svs>,
- <&rpmhpd_opp_svs>;
- };
-
- opp-444000000 {
- opp-hz = /bits/ 64 <444000000>;
- required-opps = <&rpmhpd_opp_svs_l1>,
- <&rpmhpd_opp_svs_l1>;
- };
-
- opp-533333334 {
- opp-hz = /bits/ 64 <533333334>;
- required-opps = <&rpmhpd_opp_nom>,
- <&rpmhpd_opp_nom>;
- };
-
- opp-630000000 {
- opp-hz = /bits/ 64 <630000000>;
- required-opps = <&rpmhpd_opp_turbo>,
- <&rpmhpd_opp_turbo>;
+ iris_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-240000000 {
+ opp-hz = /bits/ 64 <240000000>;
+ required-opps = <&rpmhpd_opp_low_svs_d1>,
+ <&rpmhpd_opp_low_svs_d1>;
+ };
+
+ opp-338000000 {
+ opp-hz = /bits/ 64 <338000000>;
+ required-opps = <&rpmhpd_opp_low_svs>,
+ <&rpmhpd_opp_low_svs>;
+ };
+
+ opp-420000000 {
+ opp-hz = /bits/ 64 <420000000>;
+ required-opps = <&rpmhpd_opp_svs>,
+ <&rpmhpd_opp_svs>;
+ };
+
+ opp-444000000 {
+ opp-hz = /bits/ 64 <444000000>;
+ required-opps = <&rpmhpd_opp_svs_l1>,
+ <&rpmhpd_opp_svs_l1>;
+ };
+
+ opp-533333334 {
+ opp-hz = /bits/ 64 <533333334>;
+ required-opps = <&rpmhpd_opp_nom>,
+ <&rpmhpd_opp_nom>;
+ };
+
+ opp-630000000 {
+ opp-hz = /bits/ 64 <630000000>;
+ required-opps = <&rpmhpd_opp_turbo>,
+ <&rpmhpd_opp_turbo>;
+ };
};
};
};
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 03/13] dt-bindings: media: qcom,sm8750-iris: Add context bank subnodes
2026-09-09 14:57 ` [PATCH v3 03/13] dt-bindings: media: qcom,sm8750-iris: " Vikash Garodia
@ 2026-09-09 21:32 ` Dmitry Baryshkov
0 siblings, 0 replies; 31+ messages in thread
From: Dmitry Baryshkov @ 2026-09-09 21:32 UTC (permalink / raw)
To: Vikash Garodia
Cc: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Bryan O'Donoghue, Stephan Gerhold,
Joerg Roedel (AMD), Will Deacon, Robin Murphy, linux-media,
linux-arm-msm, devicetree, linux-kernel, Konrad Dybcio,
Krzysztof Kozlowski, iommu, Vishnu Reddy
On Wed, Sep 09, 2026 at 08:27:10PM +0530, Vikash Garodia wrote:
> The VPU issues DMA through several SMMU streams, and the hardware does
> not give every stream the same addressable range. The non-pixel stream
> cannot address the low 600MB of IOVA space, while the pixel stream can
> address the full range:
> +-----------------------------------------------------------+
> | non-pixel stream addressable range (600 MB - 3.5 GB) |
> | 0x25800000 - 0xe0000000 |
> +-----------------------------------------------------------+
> | pixel stream addressable range (0 - 3.5 GB) |
> | 0x00000000 - 0xe0000000 |
> +-----------------------------------------------------------+
> A single "iommus" property on the video-codec node puts every stream in
> one IOMMU domain sharing one IOVA allocator, so nothing restricts a
> non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
> that boundary the hardware faults, which shows up as unhandled SMMU page
> faults and spontaneous reboots:
> https://gitlab.freedesktop.org/drm/msm/-/work_items/100
>
> Given that the address range restriction is for specific VPU stream, it
> should be ideally be moved to that stream. To achieve the same, a subset
> of streams is now represented as subnodes, so that each can be
> associated with its respective addressable range. The design was discussed
> and agreed by mainatiners here
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
>
> In the past, this limitation was addressed with an iommu-map approach,
> with the iris driver dynamically creating the devices. That approach
> was later concluded to be a hack to avoid having subnodes, and was NAKed
> by the iommu maintainers. It was discussed in detail here:
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
>
> Update example to the subnode form. Doing so, it picks up the supporting
> properties needed on the video-codec node, address-cells, size-cells and
> dma-ranges to declare 1:1 DMA translation into the parent. Given a soc
> with address-cells and size-cells as 2, update the video-codec "reg"
> accordingly.
>
> This patch depends-on:
> https://github.com/devicetree-org/dt-schema/pull/207
>
> Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
> Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
> ---
> .../bindings/media/qcom,sm8750-iris.yaml | 187 +++++++++++----------
> 1 file changed, 101 insertions(+), 86 deletions(-)
>
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
Again, drop the soc node and adapt the _example_ to use 32-bit
addresses.
> +
> + video-codec@aa00000 {
> + compatible = "qcom,sm8750-iris";
> + reg = <0x0 0x0aa00000 0x0 0xf0000>;
> +
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 04/13] iommu: of_iommu: Add support for "iommu-ranges" on a device node
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (2 preceding siblings ...)
2026-09-09 14:57 ` [PATCH v3 03/13] dt-bindings: media: qcom,sm8750-iris: " Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 15:12 ` sashiko-bot
2026-09-09 14:57 ` [PATCH v3 05/13] media: iris: Add non-pixel and pixel context bank devices Vikash Garodia
` (8 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Vishnu Reddy
Reserving an IOVA address using "iommu-addresses" requires it to be
placed in the reserved-memory node. But when "iommu-addresses" is the
only property being described and there is no backing "reg" (i.e. no
actual reserved system memory), it does not really belong under
/reserved-memory. A device IOVA range is specific to its own address
space, and does not describe the system physical memory to make it
qualify under reserved-memory. Given this, place "iommu-addresses"
inside reserved-memory only when it is paired with a "reg", otherwise,
define it within the device own node. It was discussed here:
https://lore.kernel.org/all/662f7093-fb0a-4564-9ca0-98e03e68ba9a@kernel.org
Existing "iommu-addresses" property expects a phandle, which does
not make sense when the property is defined within the device node.
Introduce a new property, "iommu-ranges", to specify the device
specific IOVA ranges when there is no backing "reg".
This patch depends-on:
https://github.com/devicetree-org/dt-schema/pull/207
Suggested-by: Rob Herring <robh@kernel.org>
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
drivers/iommu/of_iommu.c | 107 +++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 100 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index a18bb60f6f3dfdba853b6e16dd2a8616f6a731c7..1b014666d6b8ee3beadbd77e2ec8a24459468652 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -190,6 +190,90 @@ iommu_resv_region_get_type(struct device *dev,
return IOMMU_RESV_RESERVED;
}
+/**
+ * of_iommu_derive_resv_regions - derive reserved regions which
+ * are outside of iommu-ranges
+ * @dev: device for which to get reserved regions
+ * @list: reserved region list
+ *
+ * A device can describe its own usable IOVA ranges directly on its node
+ * via "iommu-ranges". Everything not under those ranges is derived
+ * as a reserved region so the IOMMU allocator won't use it. Entries may
+ * appear in any order in the property.
+ */
+static void of_iommu_derive_resv_regions(struct device *dev, struct list_head *list)
+{
+ struct of_iommu_range {
+ struct list_head node;
+ phys_addr_t start;
+ phys_addr_t end;
+ } *pos, *new, *next_range;
+ int size, prot = IOMMU_READ | IOMMU_WRITE;
+ struct iommu_resv_region *region;
+ const __be32 *maps, *end;
+ phys_addr_t next = 0;
+ LIST_HEAD(ranges);
+
+ maps = of_get_property(dev->of_node, "iommu-ranges", &size);
+ if (!maps)
+ return;
+
+ end = maps + size / sizeof(__be32);
+
+ while (maps < end) {
+ phys_addr_t iova;
+ size_t length;
+
+ maps = of_translate_dma_region(dev->of_node, maps, &iova, &length);
+ if (!maps) {
+ dev_err(dev, "failed to parse iommu-ranges\n");
+ break;
+ }
+
+ if (!length)
+ continue;
+
+ list_for_each_entry(pos, &ranges, node)
+ if (pos->start > iova)
+ break;
+
+ new = kmalloc_obj(*new);
+ if (!new) {
+ dev_err(dev, "kmalloc_obj() failed\n");
+ continue;
+ }
+
+ new->start = iova;
+ new->end = iova + length - 1;
+ list_add_tail(&new->node, &pos->node);
+ }
+
+ if (list_empty(&ranges))
+ return;
+
+ if (of_dma_is_coherent(dev->of_node))
+ prot |= IOMMU_CACHE;
+
+ list_for_each_entry_safe(pos, next_range, &ranges, node) {
+ if (pos->start > next) {
+ region = iommu_alloc_resv_region(next, pos->start - next, prot,
+ IOMMU_RESV_RESERVED, GFP_KERNEL);
+ if (region)
+ list_add_tail(®ion->list, list);
+ }
+ if (pos->end + 1 > next)
+ next = pos->end + 1;
+
+ list_del(&pos->node);
+ kfree(pos);
+ }
+
+ region = iommu_alloc_resv_region(next, ~(phys_addr_t)0 - next + 1,
+ prot, IOMMU_RESV_RESERVED, GFP_KERNEL);
+ if (region)
+ list_add_tail(®ion->list, list);
+}
+
/**
* of_iommu_get_resv_regions - reserved region driver helper for device tree
* @dev: device for which to get reserved regions
@@ -214,10 +298,14 @@ void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
memset(&phys, 0, sizeof(phys));
+ maps = of_get_property(it.node, "iommu-addresses", &size);
+ if (!maps)
+ continue;
+
/*
- * The "reg" property is optional and can be omitted by reserved-memory regions
- * that represent reservations in the IOVA space, which are regions that should
- * not be mapped.
+ * "iommu-addresses" must be used in combination with a "reg" that provides
+ * the physical address and size of this memory region, for an identity 1:1
+ * IOVA mapping to that physical memory.
*/
if (of_property_present(it.node, "reg")) {
err = of_address_to_resource(it.node, 0, &phys);
@@ -226,11 +314,11 @@ void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
it.node, err);
continue;
}
- }
-
- maps = of_get_property(it.node, "iommu-addresses", &size);
- if (!maps)
+ } else {
+ dev_err(dev, "%pOF: iommu-addresses requires a reg property\n",
+ it.node);
continue;
+ }
end = maps + size / sizeof(__be32);
@@ -258,6 +346,9 @@ void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
}
type = iommu_resv_region_get_type(dev, &phys, iova, length);
+ if (type != IOMMU_RESV_DIRECT)
+ continue;
+
region = iommu_alloc_resv_region(iova, length, prot, type,
GFP_KERNEL);
if (region)
@@ -265,6 +356,8 @@ void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
}
}
}
+
+ of_iommu_derive_resv_regions(dev, list);
#endif
}
EXPORT_SYMBOL(of_iommu_get_resv_regions);
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 04/13] iommu: of_iommu: Add support for "iommu-ranges" on a device node
2026-09-09 14:57 ` [PATCH v3 04/13] iommu: of_iommu: Add support for "iommu-ranges" on a device node Vikash Garodia
@ 2026-09-09 15:12 ` sashiko-bot
0 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-09-09 15:12 UTC (permalink / raw)
To: Vikash Garodia; +Cc: devicetree, media-ci, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] Integer overflow when computing `pos->end + 1` causes incorrect overlapping reserved regions or zero-length reservations.
- [High] Breaks backward compatibility for `iommu-addresses` nodes without a `reg` property or with pure IOVA reservations.
- [Medium] Error paths fail-open and silently grant the device unrestricted IOVA access instead of restricting it.
Pre-existing issues:
- [High] Desynced DT property parsing when a phandle does not match `dev->of_node` causes missing reservations.
- [High] Missing check for NULL return from `of_translate_dma_region` causes NULL pointer dereference on the next iteration.
--
commit f881f2df9648f21e02230f5cfd4a02463cb42a63
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
iommu: of_iommu: Add support for "iommu-ranges" on a device node
This commit introduces support for the iommu-ranges property to allow
devices to specify their IOVA ranges without a backing reg property. It
derives implicit reserved regions for anything outside these ranges and
restricts iommu-addresses to only be used when paired with a reg property.
Link: https://lore.kernel.org/all/662f7093-fb0a-4564-9ca0-98e03e68ba9a@kernel.org
Link: https://github.com/devicetree-org/dt-schema/pull/207
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index a18bb60f6f3df..1b014666d6b8e 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -190,6 +190,90 @@ iommu_resv_region_get_type(struct device *dev,
> return IOMMU_RESV_RESERVED;
> }
>
> +/**
> + * of_iommu_derive_resv_regions - derive reserved regions which
> + * are outside of iommu-ranges
[ ... ]
> + maps = of_get_property(dev->of_node, "iommu-ranges", &size);
> + if (!maps)
> + return;
> +
> + end = maps + size / sizeof(__be32);
> +
> + while (maps < end) {
> + phys_addr_t iova;
> + size_t length;
> +
> + maps = of_translate_dma_region(dev->of_node, maps, &iova, &length);
> + if (!maps) {
> + dev_err(dev, "failed to parse iommu-ranges\n");
> + break;
> + }
[ ... ]
> + }
> +
> + if (list_empty(&ranges))
> + return;
[Severity: Medium]
If parsing fails and the loop breaks early, or if the ranges evaluate to
0 length, the ranges list will be empty and this path returns early without
deriving any reserved regions. Does this silently fail-open and grant the
device unrestricted IOVA access instead of restricting it?
> +
> + if (of_dma_is_coherent(dev->of_node))
> + prot |= IOMMU_CACHE;
> +
> + list_for_each_entry_safe(pos, next_range, &ranges, node) {
> + if (pos->start > next) {
> + region = iommu_alloc_resv_region(next, pos->start - next, prot,
> + IOMMU_RESV_RESERVED, GFP_KERNEL);
> + if (region)
> + list_add_tail(®ion->list, list);
> + }
> + if (pos->end + 1 > next)
> + next = pos->end + 1;
[Severity: High]
If an explicitly allowed iommu-ranges entry extends to the end of the
physical address space (for example, 0xFFFFFFFF on a 32-bit system), could
pos->end + 1 wrap around to 0?
If it wraps to 0, the condition (0 > next) evaluates to false, leaving
the next variable stale.
> +
> + list_del(&pos->node);
> + kfree(pos);
> + }
> +
> + region = iommu_alloc_resv_region(next, ~(phys_addr_t)0 - next + 1,
> + prot, IOMMU_RESV_RESERVED, GFP_KERNEL);
[Severity: High]
If next is left stale due to the wrap-around above, will this final
allocation use the stale next value to reserve the remainder of the address
space, erroneously covering the explicitly allowed IOVA space?
> + if (region)
> + list_add_tail(®ion->list, list);
> +}
> +
> /**
> * of_iommu_get_resv_regions - reserved region driver helper for device tree
[ ... ]
> @@ -214,10 +298,14 @@ void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
>
> memset(&phys, 0, sizeof(phys));
>
> + maps = of_get_property(it.node, "iommu-addresses", &size);
> + if (!maps)
> + continue;
> +
> /*
> - * The "reg" property is optional and can be omitted by reserved-memory regions
> - * that represent reservations in the IOVA space, which are regions that should
> - * not be mapped.
> + * "iommu-addresses" must be used in combination with a "reg" that provides
> + * the physical address and size of this memory region, for an identity 1:1
> + * IOVA mapping to that physical memory.
> */
> if (of_property_present(it.node, "reg")) {
> err = of_address_to_resource(it.node, 0, &phys);
> @@ -226,11 +314,11 @@ void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
> it.node, err);
> continue;
> }
> - }
> -
> - maps = of_get_property(it.node, "iommu-addresses", &size);
> - if (!maps)
> + } else {
> + dev_err(dev, "%pOF: iommu-addresses requires a reg property\n",
> + it.node);
> continue;
> + }
[Severity: High]
Does this explicitly reject legacy Device Tree nodes that lack a reg property?
Existing Device Trees using iommu-addresses to describe pure IOVA reservations
without a reg property might fail to parse here, leaving the IOVA space
unreserved. Could this break backward compatibility and violate the DT stable
ABI guarantee?
> end = maps + size / sizeof(__be32);
>
> while (maps < end) {
> struct device_node *np;
> u32 phandle;
>
> phandle = be32_to_cpup(maps++);
[Severity: High]
This is a pre-existing issue, but if of_translate_dma_region() returns NULL
during the previous iteration, does this dereference a NULL pointer?
> np = of_find_node_by_phandle(phandle);
>
> if (np == dev->of_node) {
> int prot = IOMMU_READ | IOMMU_WRITE;
> struct iommu_resv_region *region;
> enum iommu_resv_type type;
> phys_addr_t iova;
> size_t length;
>
> if (of_dma_is_coherent(dev->of_node))
> prot |= IOMMU_CACHE;
>
> maps = of_translate_dma_region(np, maps, &iova, &length);
[Severity: High]
This is a pre-existing issue, but does this overwrite the loop iterator maps
with a potentially NULL return value without validation? If it returns NULL
due to a parsing error, the next iteration will evaluate maps < end as true
since NULL is less than the end pointer boundary.
[Severity: High]
Also a pre-existing issue, but if np does not match dev->of_node, the code
skips processing the region. Is there a missing else block here to advance the
maps pointer over the iova and length cells?
Without it, the next iteration will read the first cell of the ignored iova
as the next phandle, throwing the parser out of sync and causing subsequent
reservations to be silently ignored.
> @@ -258,6 +346,9 @@ void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
> }
> type = iommu_resv_region_get_type(dev, &phys, iova, length);
>
> + if (type != IOMMU_RESV_DIRECT)
> + continue;
[Severity: High]
Does this drop any pure IOMMU_RESV_RESERVED regions that were previously
supported and parseable before this patch?
> +
> region = iommu_alloc_resv_region(iova, length, prot, type,
> GFP_KERNEL);
> if (region)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-vpu_iommu_iova_handling-v3-0-2fe43ae45495@oss.qualcomm.com?part=4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 05/13] media: iris: Add non-pixel and pixel context bank devices
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (3 preceding siblings ...)
2026-09-09 14:57 ` [PATCH v3 04/13] iommu: of_iommu: Add support for "iommu-ranges" on a device node Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 15:16 ` sashiko-bot
2026-09-09 14:57 ` [PATCH v3 06/13] media: iris: Route buffers to the matching context bank device Vikash Garodia
` (7 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Daniel J Blueman, Vishnu Reddy
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. The non-pixel stream
cannot address the low 600MB of IOVA space, while the pixel stream can
address the full range:
+-----------------------------------------------------------+
| non-pixel stream addressable range (600 MB - 3.5 GB) |
| 0x25800000 - 0xe0000000 |
+-----------------------------------------------------------+
| pixel stream addressable range (0 - 3.5 GB) |
| 0x00000000 - 0xe0000000 |
+-----------------------------------------------------------+
A single "iommus" property on the video-codec node puts every stream in
one IOMMU domain sharing one IOVA allocator, so nothing restricts a
non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
that boundary the hardware faults, which shows up as unhandled SMMU page
faults and spontaneous reboots:
https://gitlab.freedesktop.org/drm/msm/-/work_items/100
Now the device tree describes "non-pixel" and "pixel" as separate
context bank subnodes, each carrying its own "iommus" stream IDs.
Add helper functions to create and clean up devices from these DT
subnodes, and call them from iris_probe() and iris_remove(). This
applies to the common probe path as it is required for all platforms.
Set the context banks before v4l2_device_register() so the DMA plumbing
is in place before any video device is visible to userspace, and tear
them down on the probe error path and in iris_remove().
Tested-by: Daniel J Blueman <daniel@quora.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_core.h | 4 ++
drivers/media/platform/qcom/iris/iris_probe.c | 71 ++++++++++++++++++++++++++-
2 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
index 24da60448cf24820af7947b85eb7208555ab7786..3c96f46cf567b2802148b2a7cedb8488b6b9468b 100644
--- a/drivers/media/platform/qcom/iris/iris_core.h
+++ b/drivers/media/platform/qcom/iris/iris_core.h
@@ -36,6 +36,8 @@ struct qcom_ubwc_cfg_data;
* struct iris_core - holds core parameters valid for all instances
*
* @dev: reference to device structure
+ * @np_dev: reference to non-pixel device structure
+ * @p_dev: reference to pixel device structure
* @reg_base: IO memory base address
* @irq: iris irq
* @v4l2_dev: a holder for v4l2 device structure
@@ -81,6 +83,8 @@ struct qcom_ubwc_cfg_data;
struct iris_core {
struct device *dev;
+ struct device *np_dev;
+ struct device *p_dev;
void __iomem *reg_base;
int irq;
struct v4l2_device v4l2_dev;
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index e4acf4a74f944bcae83089ef5489f204d4b0078e..debd1f0e57038d7abf463e82fd80f887b67750e8 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -150,6 +150,67 @@ static int iris_init_resources(struct iris_core *core)
return iris_init_resets(core);
}
+static struct device *iris_create_cb_dev(struct iris_core *core, const char *name)
+{
+ struct platform_device_info plat_dev_info = {};
+ struct device_node *child_of_node;
+ struct platform_device *pdev;
+
+ child_of_node = of_get_child_by_name(core->dev->of_node, name);
+ if (!child_of_node)
+ return NULL;
+
+ plat_dev_info.dma_mask = core->iris_platform_data->dma_mask;
+ plat_dev_info.fwnode = &child_of_node->fwnode;
+ plat_dev_info.name = child_of_node->name;
+ plat_dev_info.id = PLATFORM_DEVID_AUTO;
+ plat_dev_info.parent = core->dev;
+
+ pdev = platform_device_register_full(&plat_dev_info);
+ of_node_put(child_of_node);
+ if (IS_ERR(pdev))
+ return ERR_CAST(pdev);
+
+ dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
+ dma_set_seg_boundary(&pdev->dev, DMA_BIT_MASK(32));
+
+ return &pdev->dev;
+}
+
+static int iris_init_cb_devs(struct iris_core *core)
+{
+ struct device *dev;
+
+ dev = iris_create_cb_dev(core, "non-pixel");
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
+
+ core->np_dev = dev;
+
+ dev = iris_create_cb_dev(core, "pixel");
+ if (IS_ERR(dev))
+ goto unreg_np_dev;
+
+ core->p_dev = dev;
+
+ return 0;
+
+unreg_np_dev:
+ if (core->np_dev)
+ platform_device_unregister(to_platform_device(core->np_dev));
+ core->np_dev = NULL;
+
+ return PTR_ERR(dev);
+}
+
+static void iris_deinit_cb_devs(struct iris_core *core)
+{
+ if (core->p_dev)
+ platform_device_unregister(to_platform_device(core->p_dev));
+ if (core->np_dev)
+ platform_device_unregister(to_platform_device(core->np_dev));
+}
+
static int iris_register_video_device(struct iris_core *core, enum domain_type type)
{
struct video_device *vdev;
@@ -207,6 +268,8 @@ static void iris_remove(struct platform_device *pdev)
v4l2_device_unregister(&core->v4l2_dev);
+ iris_deinit_cb_devs(core);
+
mutex_destroy(&core->lock);
}
@@ -269,10 +332,14 @@ static int iris_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = v4l2_device_register(dev, &core->v4l2_dev);
+ ret = iris_init_cb_devs(core);
if (ret)
return ret;
+ ret = v4l2_device_register(dev, &core->v4l2_dev);
+ if (ret)
+ goto err_cb_deinit;
+
ret = iris_register_video_device(core, DECODER);
if (ret)
goto err_v4l2_unreg;
@@ -306,6 +373,8 @@ static int iris_probe(struct platform_device *pdev)
video_unregister_device(core->vdev_dec);
err_v4l2_unreg:
v4l2_device_unregister(&core->v4l2_dev);
+err_cb_deinit:
+ iris_deinit_cb_devs(core);
return ret;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 05/13] media: iris: Add non-pixel and pixel context bank devices
2026-09-09 14:57 ` [PATCH v3 05/13] media: iris: Add non-pixel and pixel context bank devices Vikash Garodia
@ 2026-09-09 15:16 ` sashiko-bot
0 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-09-09 15:16 UTC (permalink / raw)
To: Vikash Garodia; +Cc: conor+dt, media-ci, robh, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free of devres-allocated `core` memory on driver unbind via lingering file descriptors.
- [High] Missing `cancel_delayed_work_sync` in driver teardown allows `sys_error_handler` to execute asynchronously on freed memory.
--
commit 5f2788e633116dd856570ea29baabf6ce4157fbe
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
media: iris: Add non-pixel and pixel context bank devices
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. This patch describes
"non-pixel" and "pixel" as separate context bank subnodes. Helper
functions are added to create and clean up these devices during probe
and remove.
> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index e4acf4a74f944..debd1f0e57038 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
[ ... ]
> @@ -207,6 +268,8 @@ static void iris_remove(struct platform_device *pdev)
>
> v4l2_device_unregister(&core->v4l2_dev);
>
> + iris_deinit_cb_devs(core);
> +
> mutex_destroy(&core->lock);
[Severity: High]
This isn't a bug introduced by this patch, but doesn't this leave the
driver vulnerable to a use-after-free if a userspace process has the
video device open during driver unbind?
The core context is allocated with devm_kzalloc() in iris_probe(),
so it will be freed automatically upon return from iris_remove().
However, this function does not wait for active V4L2 file descriptors
to close. If a lingering file descriptor is closed later, iris_close()
will invoke iris_remove_session(), which accesses the freed core
memory and attempts to lock the destroyed mutex:
iris_vidc.c:iris_remove_session() {
...
mutex_lock(&core->lock);
...
}
[Severity: High]
This is a pre-existing issue, but is there a missing
cancel_delayed_work_sync() for core->sys_error_handler during teardown?
In iris_probe(), the delayed work is initialized:
INIT_DELAYED_WORK(&core->sys_error_handler, iris_sys_error_handler);
If a hardware SYS_ERROR interrupt fires near the time of unbind, the
delayed work could be scheduled. Since iris_remove() completes without
cancelling it, the core memory is freed by devres.
The work queue could subsequently execute iris_sys_error_handler()
asynchronously on the freed pointer, attempting to access core->dev
and call iris_core_deinit(core).
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-vpu_iommu_iova_handling-v3-0-2fe43ae45495@oss.qualcomm.com?part=5
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 06/13] media: iris: Route buffers to the matching context bank device
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (4 preceding siblings ...)
2026-09-09 14:57 ` [PATCH v3 05/13] media: iris: Add non-pixel and pixel context bank devices Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 15:13 ` sashiko-bot
2026-09-09 14:57 ` [PATCH v3 07/13] media: iris: Skip DMA mask setup when the core device has no IOMMU Vikash Garodia
` (6 subsequent siblings)
12 siblings, 1 reply; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Daniel J Blueman, Vishnu Reddy
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. The non-pixel stream
cannot address the low 600MB of IOVA space, while the pixel stream can
address the full range.
Add iris_get_cb_dev(), which maps a buffer type to the owning context
bank device. Bitstream and internal buffers (BIN, ARP, COMV, LINE,
NON_COMV, PERSIST) belong to the non-pixel device, and uncompressed
buffers (DPB, PARTIAL, SCRATCH_1, SCRATCH_2, VPSS) to the pixel device.
BUF_INPUT and BUF_OUTPUT depend on direction and are resolved from
inst->domain: for a decoder the input is non-pixel and the output pixel,
and the other way round for an encoder.
Fall back to core->dev whenever the relevant context bank device is
absent, so platforms still describing "iommus" on the parent iris node
behave exactly as before to maintain backward compatibility.
Tested-by: Daniel J Blueman <daniel@quora.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_buffer.c | 8 ++---
drivers/media/platform/qcom/iris/iris_hfi_queue.c | 16 +++++----
drivers/media/platform/qcom/iris/iris_resources.c | 41 +++++++++++++++++++++++
drivers/media/platform/qcom/iris/iris_resources.h | 1 +
drivers/media/platform/qcom/iris/iris_vidc.c | 4 +--
5 files changed, 57 insertions(+), 13 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
index eb8de60c1177f5e1ab83b90a3c8c80e0b4d1f02e..6e03d13ac1a792c539bf8fe6d26354aa29d7b3a2 100644
--- a/drivers/media/platform/qcom/iris/iris_buffer.c
+++ b/drivers/media/platform/qcom/iris/iris_buffer.c
@@ -531,7 +531,7 @@ static int iris_create_internal_buffer(struct iris_inst *inst,
enum iris_buffer_type buffer_type, u32 index)
{
struct iris_buffers *buffers = &inst->buffers[buffer_type];
- struct iris_core *core = inst->core;
+ struct device *dev = iris_get_cb_dev(inst, buffer_type);
struct iris_buffer *buffer;
if (!buffers->size)
@@ -547,7 +547,7 @@ static int iris_create_internal_buffer(struct iris_inst *inst,
buffer->buffer_size = buffers->size;
buffer->dma_attrs = DMA_ATTR_WRITE_COMBINE | DMA_ATTR_NO_KERNEL_MAPPING;
- buffer->kvaddr = dma_alloc_attrs(core->dev, buffer->buffer_size,
+ buffer->kvaddr = dma_alloc_attrs(dev, buffer->buffer_size,
&buffer->device_addr, GFP_KERNEL, buffer->dma_attrs);
if (!buffer->kvaddr) {
kfree(buffer);
@@ -650,10 +650,10 @@ int iris_queue_internal_buffers(struct iris_inst *inst, u32 plane)
void iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buffer)
{
- struct iris_core *core = inst->core;
+ struct device *dev = iris_get_cb_dev(inst, buffer->type);
list_del(&buffer->list);
- dma_free_attrs(core->dev, buffer->buffer_size, buffer->kvaddr,
+ dma_free_attrs(dev, buffer->buffer_size, buffer->kvaddr,
buffer->device_addr, buffer->dma_attrs);
kfree(buffer);
}
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_queue.c b/drivers/media/platform/qcom/iris/iris_hfi_queue.c
index bf6db23b53e2106c08f2139b643f8626af8bc40a..ce6a682b0f9ada79f9fae26289db298855d777c2 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_queue.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_queue.c
@@ -245,25 +245,26 @@ static void iris_hfi_queue_deinit(struct iris_iface_q_info *iface_q)
int iris_hfi_queues_init(struct iris_core *core)
{
+ struct device *dev = core->np_dev ? core->np_dev : core->dev;
struct iris_hfi_queue_table_header *q_tbl_hdr;
u32 queue_size;
/* Iris hardware requires 4K queue alignment */
queue_size = ALIGN((sizeof(*q_tbl_hdr) + (IFACEQ_QUEUE_SIZE * IFACEQ_NUMQ)), SZ_4K);
- core->iface_q_table_vaddr = dma_alloc_attrs(core->dev, queue_size,
+ core->iface_q_table_vaddr = dma_alloc_attrs(dev, queue_size,
&core->iface_q_table_daddr,
GFP_KERNEL, DMA_ATTR_WRITE_COMBINE);
if (!core->iface_q_table_vaddr) {
- dev_err(core->dev, "queues alloc and map failed\n");
+ dev_err(dev, "queues alloc and map failed\n");
return -ENOMEM;
}
- core->sfr_vaddr = dma_alloc_attrs(core->dev, SFR_SIZE,
+ core->sfr_vaddr = dma_alloc_attrs(dev, SFR_SIZE,
&core->sfr_daddr,
GFP_KERNEL, DMA_ATTR_WRITE_COMBINE);
if (!core->sfr_vaddr) {
- dev_err(core->dev, "sfr alloc and map failed\n");
- dma_free_attrs(core->dev, queue_size, core->iface_q_table_vaddr,
+ dev_err(dev, "sfr alloc and map failed\n");
+ dma_free_attrs(dev, queue_size, core->iface_q_table_vaddr,
core->iface_q_table_daddr, DMA_ATTR_WRITE_COMBINE);
return -ENOMEM;
}
@@ -291,6 +292,7 @@ int iris_hfi_queues_init(struct iris_core *core)
void iris_hfi_queues_deinit(struct iris_core *core)
{
+ struct device *dev = core->np_dev ? core->np_dev : core->dev;
u32 queue_size;
if (!core->iface_q_table_vaddr)
@@ -300,7 +302,7 @@ void iris_hfi_queues_deinit(struct iris_core *core)
iris_hfi_queue_deinit(&core->message_queue);
iris_hfi_queue_deinit(&core->command_queue);
- dma_free_attrs(core->dev, SFR_SIZE, core->sfr_vaddr,
+ dma_free_attrs(dev, SFR_SIZE, core->sfr_vaddr,
core->sfr_daddr, DMA_ATTR_WRITE_COMBINE);
core->sfr_vaddr = NULL;
@@ -309,7 +311,7 @@ void iris_hfi_queues_deinit(struct iris_core *core)
queue_size = ALIGN(sizeof(struct iris_hfi_queue_table_header) +
(IFACEQ_QUEUE_SIZE * IFACEQ_NUMQ), SZ_4K);
- dma_free_attrs(core->dev, queue_size, core->iface_q_table_vaddr,
+ dma_free_attrs(dev, queue_size, core->iface_q_table_vaddr,
core->iface_q_table_daddr, DMA_ATTR_WRITE_COMBINE);
core->iface_q_table_vaddr = NULL;
diff --git a/drivers/media/platform/qcom/iris/iris_resources.c b/drivers/media/platform/qcom/iris/iris_resources.c
index 2c4d34c7bd77d1f78d3572f659da656eb3b12113..a6c3df892ca9888d82414ca6b48c80efbf7b06d0 100644
--- a/drivers/media/platform/qcom/iris/iris_resources.c
+++ b/drivers/media/platform/qcom/iris/iris_resources.c
@@ -12,6 +12,7 @@
#include <linux/reset.h>
#include "iris_core.h"
+#include "iris_instance.h"
#include "iris_resources.h"
#define BW_THRESHOLD 50000
@@ -138,3 +139,43 @@ int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type
return 0;
}
+
+struct device *iris_get_cb_dev(struct iris_inst *inst, enum iris_buffer_type buffer_type)
+{
+ struct iris_core *core = inst->core;
+ struct device *dev = NULL;
+
+ switch (buffer_type) {
+ case BUF_INPUT:
+ if (inst->domain == DECODER)
+ dev = core->np_dev;
+ else
+ dev = core->p_dev;
+ break;
+ case BUF_OUTPUT:
+ if (inst->domain == DECODER)
+ dev = core->p_dev;
+ else
+ dev = core->np_dev;
+ break;
+ case BUF_DPB:
+ case BUF_PARTIAL:
+ case BUF_SCRATCH_1:
+ case BUF_SCRATCH_2:
+ case BUF_VPSS:
+ dev = core->p_dev;
+ break;
+ case BUF_BIN:
+ case BUF_ARP:
+ case BUF_COMV:
+ case BUF_LINE:
+ case BUF_NON_COMV:
+ case BUF_PERSIST:
+ dev = core->np_dev;
+ break;
+ default:
+ dev_err(core->dev, "invalid buffer type: %d\n", buffer_type);
+ }
+
+ return dev ? dev : core->dev;
+}
diff --git a/drivers/media/platform/qcom/iris/iris_resources.h b/drivers/media/platform/qcom/iris/iris_resources.h
index 6bfbd2dc6db095ec05e53c894e048285f82446c6..a9a5bddb19c24917de4c5dba52567934b77e5c59 100644
--- a/drivers/media/platform/qcom/iris/iris_resources.h
+++ b/drivers/media/platform/qcom/iris/iris_resources.h
@@ -15,5 +15,6 @@ int iris_unset_icc_bw(struct iris_core *core);
int iris_set_icc_bw(struct iris_core *core, unsigned long icc_bw);
int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type clk_type);
int iris_prepare_enable_clock(struct iris_core *core, enum platform_clk_type clk_type);
+struct device *iris_get_cb_dev(struct iris_inst *inst, enum iris_buffer_type buffer_type);
#endif
diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
index fcbc60016beec693f2ce27927a09a2d51494bc38..064cf0a80748dab1be0515c4c2b6688aeb2b7022 100644
--- a/drivers/media/platform/qcom/iris/iris_vidc.c
+++ b/drivers/media/platform/qcom/iris/iris_vidc.c
@@ -108,7 +108,7 @@ iris_m2m_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_
src_vq->drv_priv = inst;
src_vq->buf_struct_size = sizeof(struct iris_buffer);
src_vq->min_reqbufs_allocation = MIN_BUFFERS;
- src_vq->dev = inst->core->dev;
+ src_vq->dev = iris_get_cb_dev(inst, BUF_INPUT);
src_vq->lock = &inst->ctx_q_lock;
ret = vb2_queue_init(src_vq);
if (ret)
@@ -122,7 +122,7 @@ iris_m2m_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_
dst_vq->drv_priv = inst;
dst_vq->buf_struct_size = sizeof(struct iris_buffer);
dst_vq->min_reqbufs_allocation = MIN_BUFFERS;
- dst_vq->dev = inst->core->dev;
+ dst_vq->dev = iris_get_cb_dev(inst, BUF_OUTPUT);
dst_vq->lock = &inst->ctx_q_lock;
return vb2_queue_init(dst_vq);
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 06/13] media: iris: Route buffers to the matching context bank device
2026-09-09 14:57 ` [PATCH v3 06/13] media: iris: Route buffers to the matching context bank device Vikash Garodia
@ 2026-09-09 15:13 ` sashiko-bot
0 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-09-09 15:13 UTC (permalink / raw)
To: Vikash Garodia; +Cc: robh, conor+dt, devicetree, media-ci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Context bank devices (np_dev and p_dev) are unregistered and freed during driver removal while V4L2 queues and internal buffers still hold references to them, leading to a Use-After-Free and IOMMU faults in dma_free_attrs().
Pre-existing issues:
- [Critical] V4L2 instances access devres-managed `iris_core` structure after it has been freed during module removal.
--
commit 0b3161aa43cd126ba013789330d51a653b78bf3a
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
Subject: media: iris: Route buffers to the matching context bank device
The VPU issues direct memory accesses through several stream units, requiring
non-pixel streams to use a specific context bank device due to address range
differences. This commit updates the buffer and queue allocation routines
to fetch and use the matching context bank device based on the buffer type.
> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
> index eb8de60c1177f..6e03d13ac1a79 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
[ ... ]
> @@ -650,10 +650,10 @@ int iris_queue_internal_buffers(struct iris_inst *inst, u32 plane)
>
> void iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buffer)
> {
> - struct iris_core *core = inst->core;
> + struct device *dev = iris_get_cb_dev(inst, buffer->type);
>
> list_del(&buffer->list);
> - dma_free_attrs(core->dev, buffer->buffer_size, buffer->kvaddr,
> + dma_free_attrs(dev, buffer->buffer_size, buffer->kvaddr,
> buffer->device_addr, buffer->dma_attrs);
[Severity: Critical]
Does this code introduce a use-after-free regression for the context bank
device?
If userspace keeps the device node open during driver unbind, the context
bank devices might be unregistered and freed. When the file descriptor is
subsequently closed by userspace, tearing down the internal buffers will
invoke dma_free_attrs() with the freed device.
Since we are using these devices for memory tracking without taking a
reference, can this lead to memory corruption?
> diff --git a/drivers/media/platform/qcom/iris/iris_resources.c b/drivers/media/platform/qcom/iris/iris_resources.c
> index 2c4d34c7bd77d..a6c3df892ca98 100644
> --- a/drivers/media/platform/qcom/iris/iris_resources.c
> +++ b/drivers/media/platform/qcom/iris/iris_resources.c
[ ... ]
> @@ -138,3 +139,43 @@ int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type
>
> return 0;
> }
> +
> +struct device *iris_get_cb_dev(struct iris_inst *inst, enum iris_buffer_type buffer_type)
> +{
> + struct iris_core *core = inst->core;
[Severity: Critical]
This is a pre-existing issue, but can this dereference of inst->core lead
to a use-after-free?
The iris_core structure is devres-managed. If the driver is unbound while
userspace still has the file descriptor open, devres will free the core
structure immediately.
When the file is closed later, teardown paths that call into functions like
iris_get_cb_dev() will access the freed inst->core structure.
> diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
> index fcbc60016beec..064cf0a80748d 100644
> --- a/drivers/media/platform/qcom/iris/iris_vidc.c
> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c
> @@ -108,7 +108,7 @@ iris_m2m_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_
> src_vq->drv_priv = inst;
> src_vq->buf_struct_size = sizeof(struct iris_buffer);
> src_vq->min_reqbufs_allocation = MIN_BUFFERS;
> - src_vq->dev = inst->core->dev;
> + src_vq->dev = iris_get_cb_dev(inst, BUF_INPUT);
> src_vq->lock = &inst->ctx_q_lock;
[Severity: Critical]
Can assigning this dynamically unregistered context bank device to the
queue cause a similar regression?
Like the internal buffers, if the driver unbinds while userspace
holds a file descriptor open, the queue teardown during the deferred
file close might dereference the freed context bank device.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-vpu_iommu_iova_handling-v3-0-2fe43ae45495@oss.qualcomm.com?part=6
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 07/13] media: iris: Skip DMA mask setup when the core device has no IOMMU
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (5 preceding siblings ...)
2026-09-09 14:57 ` [PATCH v3 06/13] media: iris: Route buffers to the matching context bank device Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 14:57 ` [PATCH v3 08/13] arm64: dts: qcom: hamoa: Add Iris context bank subnodes Vikash Garodia
` (5 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Daniel J Blueman, Vishnu Reddy
Once the streams are described as context bank subnodes, the "iommus"
property no longer sits on the parent iris node. That device then has no
IOMMU domain of its own and never performs DMA directly, so calling
dma_set_mask_and_coherent() on it is meaningless, and on a device with no
IOMMU it can fail and abort probe.
Set the DMA mask only when device_iommu_mapped() reports an IOMMU on the
core device. Platforms that have not been converted still carry "iommus"
on the parent node and keep the existing setup. For converted platforms
the mask is applied to each context bank device instead, in
iris_create_cb_dev().
Tested-by: Daniel J Blueman <daniel@quora.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_probe.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index debd1f0e57038d7abf463e82fd80f887b67750e8..4bdb078d83b5c331869dab2409bc4d10ab302737 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -352,9 +352,11 @@ static int iris_probe(struct platform_device *pdev)
dma_mask = core->iris_platform_data->dma_mask;
- ret = dma_set_mask_and_coherent(dev, dma_mask);
- if (ret)
- goto err_vdev_unreg_enc;
+ if (device_iommu_mapped(dev)) {
+ ret = dma_set_mask_and_coherent(dev, dma_mask);
+ if (ret)
+ goto err_vdev_unreg_enc;
+ }
dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
dma_set_seg_boundary(&pdev->dev, DMA_BIT_MASK(32));
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 08/13] arm64: dts: qcom: hamoa: Add Iris context bank subnodes
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (6 preceding siblings ...)
2026-09-09 14:57 ` [PATCH v3 07/13] media: iris: Skip DMA mask setup when the core device has no IOMMU Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 14:57 ` [PATCH v3 09/13] arm64: dts: qcom: sm8550: " Vikash Garodia
` (4 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Daniel J Blueman, Vishnu Reddy
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. The non-pixel stream
cannot address the low 600MB of IOVA space, while the pixel stream can
address the full range:
+-----------------------------------------------------------+
| non-pixel stream addressable range (600 MB - 3.5 GB) |
| 0x25800000 - 0xe0000000 |
+-----------------------------------------------------------+
| pixel stream addressable range (0 - 3.5 GB) |
| 0x00000000 - 0xe0000000 |
+-----------------------------------------------------------+
A single "iommus" property on the video-codec node puts every stream in
one IOMMU domain sharing one IOVA allocator, so nothing restricts a
non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
that boundary the hardware faults, which shows up as unhandled SMMU page
faults and spontaneous reboots:
https://gitlab.freedesktop.org/drm/msm/-/work_items/100
Given that the address range restriction is for specific VPU stream, it
should be ideally be moved to that stream. To achieve the same, a subset
of streams is now represented as subnodes, so that each can be
associated with its respective addressable range. The design was discussed
and agreed by mainatiners here
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
In the past, this limitation was addressed with an iommu-map approach,
with the iris driver dynamically creating the devices. That approach
was later concluded to be a hack to avoid having subnodes, and was NAKed
by the iommu maintainers. It was discussed in detail here:
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
The video-codec node includes address-cells, size-cells and dma-ranges
to declare 1:1 mapping for DMA translation to the parent.
Tested-by: Daniel J Blueman <daniel@quora.org>
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/hamoa.dtsi | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
index 5e92645c5c3cd7f2491e8d76f51f583fdb34c98a..e1e0b7d91870f383bb5213033a34e1dc29595368 100644
--- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
+++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
@@ -5463,10 +5463,12 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
reset-names = "bus";
- iommus = <&apps_smmu 0x1940 0>,
- <&apps_smmu 0x1947 0>;
dma-coherent;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges = <0x0 0x0 0x0 0xe0000000>;
+
/*
* IRIS firmware is signed by vendors, only
* enable on boards where the proper signed firmware
@@ -5474,6 +5476,15 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
*/
status = "disabled";
+ non-pixel {
+ iommus = <&apps_smmu 0x1940 0x0>;
+ iommu-ranges = <0x25800000 0xba800000>;
+ };
+
+ pixel {
+ iommus = <&apps_smmu 0x1947 0x0>;
+ };
+
iris_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 09/13] arm64: dts: qcom: sm8550: Add Iris context bank subnodes
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (7 preceding siblings ...)
2026-09-09 14:57 ` [PATCH v3 08/13] arm64: dts: qcom: hamoa: Add Iris context bank subnodes Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 14:57 ` [PATCH v3 10/13] arm64: dts: qcom: lemans: " Vikash Garodia
` (3 subsequent siblings)
12 siblings, 0 replies; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Daniel J Blueman, Vishnu Reddy
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. The non-pixel stream
cannot address the low 600MB of IOVA space, while the pixel stream can
address the full range:
+-----------------------------------------------------------+
| non-pixel stream addressable range (600 MB - 3.5 GB) |
| 0x25800000 - 0xe0000000 |
+-----------------------------------------------------------+
| pixel stream addressable range (0 - 3.5 GB) |
| 0x00000000 - 0xe0000000 |
+-----------------------------------------------------------+
A single "iommus" property on the video-codec node puts every stream in
one IOMMU domain sharing one IOVA allocator, so nothing restricts a
non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
that boundary the hardware faults, which shows up as unhandled SMMU page
faults and spontaneous reboots:
https://gitlab.freedesktop.org/drm/msm/-/work_items/100
Given that the address range restriction is for specific VPU stream, it
should be ideally be moved to that stream. To achieve the same, a subset
of streams is now represented as subnodes, so that each can be
associated with its respective addressable range. The design was discussed
and agreed by mainatiners here
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
In the past, this limitation was addressed with an iommu-map approach,
with the iris driver dynamically creating the devices. That approach
was later concluded to be a hack to avoid having subnodes, and was NAKed
by the iommu maintainers. It was discussed in detail here:
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
The video-codec node includes address-cells, size-cells and dma-ranges
to declare 1:1 mapping for DMA translation to the parent.
Tested-by: Daniel J Blueman <daniel@quora.org>
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sm8550.dtsi | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 0e701e709235c6e0a5efd7f8447dfa6ef6f6e21f..38bc5b483007ad797daaf37527b4759bc8924a2f 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -3691,8 +3691,6 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
reset-names = "bus";
- iommus = <&apps_smmu 0x1940 0>,
- <&apps_smmu 0x1947 0>;
dma-coherent;
/*
@@ -3702,6 +3700,19 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
*/
status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges = <0x0 0x0 0x0 0xe0000000>;
+
+ non-pixel {
+ iommus = <&apps_smmu 0x1940 0x0>;
+ iommu-ranges = <0x25800000 0xba800000>;
+ };
+
+ pixel {
+ iommus = <&apps_smmu 0x1947 0x0>;
+ };
+
iris_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v3 10/13] arm64: dts: qcom: lemans: Add Iris context bank subnodes
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (8 preceding siblings ...)
2026-09-09 14:57 ` [PATCH v3 09/13] arm64: dts: qcom: sm8550: " Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 15:22 ` sashiko-bot
2026-09-10 7:01 ` Krzysztof Kozlowski
2026-09-09 14:57 ` [PATCH v3 11/13] arm64: dts: qcom: monaco: " Vikash Garodia
` (2 subsequent siblings)
12 siblings, 2 replies; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Daniel J Blueman, Vishnu Reddy
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. The non-pixel stream
cannot address the low 600MB of IOVA space, while the pixel stream can
address the full range:
+-----------------------------------------------------------+
| non-pixel stream addressable range (600 MB - 3.5 GB) |
| 0x25800000 - 0xe0000000 |
+-----------------------------------------------------------+
| pixel stream addressable range (0 - 3.5 GB) |
| 0x00000000 - 0xe0000000 |
+-----------------------------------------------------------+
A single "iommus" property on the video-codec node puts every stream in
one IOMMU domain sharing one IOVA allocator, so nothing restricts a
non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
that boundary the hardware faults, which shows up as unhandled SMMU page
faults and spontaneous reboots:
https://gitlab.freedesktop.org/drm/msm/-/work_items/100
Given that the address range restriction is for specific VPU stream, it
should be ideally be moved to that stream. To achieve the same, a subset
of streams is now represented as subnodes, so that each can be
associated with its respective addressable range. The design was discussed
and agreed by mainatiners here
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
In the past, this limitation was addressed with an iommu-map approach,
with the iris driver dynamically creating the devices. That approach
was later concluded to be a hack to avoid having subnodes, and was NAKed
by the iommu maintainers. It was discussed in detail here:
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
The video-codec node includes address-cells, size-cells and dma-ranges
to declare 1:1 mapping for DMA translation to the parent.
Tested-by: Daniel J Blueman <daniel@quora.org>
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/lemans.dtsi | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/lemans.dtsi b/arch/arm64/boot/dts/qcom/lemans.dtsi
index 695eae1b7256911e656ab1ecdd92aca135d92f39..afed5e43b4fcda8b8400e815d24c5094834677f3 100644
--- a/arch/arm64/boot/dts/qcom/lemans.dtsi
+++ b/arch/arm64/boot/dts/qcom/lemans.dtsi
@@ -4962,12 +4962,23 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
reset-names = "bus";
- iommus = <&apps_smmu 0x0880 0x0400>,
- <&apps_smmu 0x0887 0x0400>;
dma-coherent;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges = <0x0 0x0 0x0 0xe0000000>;
+
status = "disabled";
+ non-pixel {
+ iommus = <&apps_smmu 0x0880 0x0400>;
+ iommu-ranges = <0x25800000 0xba800000>;
+ };
+
+ pixel {
+ iommus = <&apps_smmu 0x0887 0x0400>;
+ };
+
iris_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 10/13] arm64: dts: qcom: lemans: Add Iris context bank subnodes
2026-09-09 14:57 ` [PATCH v3 10/13] arm64: dts: qcom: lemans: " Vikash Garodia
@ 2026-09-09 15:22 ` sashiko-bot
2026-09-10 7:01 ` Krzysztof Kozlowski
1 sibling, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-09-09 15:22 UTC (permalink / raw)
To: Vikash Garodia; +Cc: media-ci, robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing IOMMU configuration for child context bank devices causes SMMU translation faults
--
commit a2c4ab829f603d5afbc86327c3483e445b4cefed
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
arm64: dts: qcom: lemans: Add Iris context bank subnodes
This commit moves the iommus property from the parent video-codec node
to specific subnodes (non-pixel and pixel) to restrict the addressable
range for the non-pixel stream. The intent is to prevent unhandled SMMU
page faults by ensuring allocations avoid the low 600MB of IOVA space.
> diff --git a/arch/arm64/boot/dts/qcom/lemans.dtsi b/arch/arm64/boot/dts/qcom/lemans.dtsi
> index 695eae1b72569..afed5e43b4fcd 100644
> --- a/arch/arm64/boot/dts/qcom/lemans.dtsi
> +++ b/arch/arm64/boot/dts/qcom/lemans.dtsi
[ ... ]
> @@ -4962,12 +4962,23 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
> resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
> reset-names = "bus";
>
> - iommus = <&apps_smmu 0x0880 0x0400>,
> - <&apps_smmu 0x0887 0x0400>;
> dma-coherent;
>
> + #address-cells = <1>;
> + #size-cells = <1>;
> + dma-ranges = <0x0 0x0 0x0 0xe0000000>;
>
> status = "disabled";
>
> + non-pixel {
> + iommus = <&apps_smmu 0x0880 0x0400>;
> + iommu-ranges = <0x25800000 0xba800000>;
> + };
> +
> + pixel {
> + iommus = <&apps_smmu 0x0887 0x0400>;
> + };
[Severity: High]
Because these subnodes lack a compatible string, do they ever bind to a
driver?
If they don't bind, the driver core's really_probe() is skipped, meaning
platform_dma_configure() and of_dma_configure() are not executed
automatically.
In the iris driver code, iris_create_cb_dev() instantiates these subnodes
dynamically via platform_device_register_full():
drivers/media/platform/qcom/iris/iris_probe.c:iris_create_cb_dev() {
...
pdev = platform_device_register_full(&plat_dev_info);
of_node_put(child_of_node);
if (IS_ERR(pdev))
return ERR_CAST(pdev);
...
}
Unlike the venus driver which explicitly calls of_dma_configure() for
driverless firmware nodes, the iris driver appears to omit explicit IOMMU
configuration on these newly created devices.
When the driver later allocates DMA buffers using core->np_dev or core->p_dev,
will those allocations bypass the IOMMU due to the missing iommu_fwspec
attachment?
Could this result in the DMA memory being allocated using direct physical
addresses instead of IOVAs, triggering the very SMMU translation faults this
patch aims to fix?
> +
> iris_opp_table: opp-table {
> compatible = "operating-points-v2";
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-vpu_iommu_iova_handling-v3-0-2fe43ae45495@oss.qualcomm.com?part=10
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 10/13] arm64: dts: qcom: lemans: Add Iris context bank subnodes
2026-09-09 14:57 ` [PATCH v3 10/13] arm64: dts: qcom: lemans: " Vikash Garodia
2026-09-09 15:22 ` sashiko-bot
@ 2026-09-10 7:01 ` Krzysztof Kozlowski
1 sibling, 0 replies; 31+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-10 7:01 UTC (permalink / raw)
To: Vikash Garodia
Cc: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy,
linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, iommu, Daniel J Blueman, Vishnu Reddy
On Wed, Sep 09, 2026 at 08:27:17PM +0530, Vikash Garodia wrote:
> The VPU issues DMA through several SMMU streams, and the hardware does
> not give every stream the same addressable range. The non-pixel stream
> cannot address the low 600MB of IOVA space, while the pixel stream can
> address the full range:
> +-----------------------------------------------------------+
> | non-pixel stream addressable range (600 MB - 3.5 GB) |
> | 0x25800000 - 0xe0000000 |
> +-----------------------------------------------------------+
> | pixel stream addressable range (0 - 3.5 GB) |
> | 0x00000000 - 0xe0000000 |
> +-----------------------------------------------------------+
> A single "iommus" property on the video-codec node puts every stream in
> one IOMMU domain sharing one IOVA allocator, so nothing restricts a
> non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
> that boundary the hardware faults, which shows up as unhandled SMMU page
> faults and spontaneous reboots:
> https://gitlab.freedesktop.org/drm/msm/-/work_items/100
>
> Given that the address range restriction is for specific VPU stream, it
> should be ideally be moved to that stream. To achieve the same, a subset
> of streams is now represented as subnodes, so that each can be
> associated with its respective addressable range. The design was discussed
> and agreed by mainatiners here
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
>
> In the past, this limitation was addressed with an iommu-map approach,
> with the iris driver dynamically creating the devices. That approach
> was later concluded to be a hack to avoid having subnodes, and was NAKed
> by the iommu maintainers. It was discussed in detail here:
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
>
> The video-codec node includes address-cells, size-cells and dma-ranges
> to declare 1:1 mapping for DMA translation to the parent.
>
> Tested-by: Daniel J Blueman <daniel@quora.org>
Really, this commit on Lemans platform was tested?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 11/13] arm64: dts: qcom: monaco: Add Iris context bank subnodes
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (9 preceding siblings ...)
2026-09-09 14:57 ` [PATCH v3 10/13] arm64: dts: qcom: lemans: " Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 15:21 ` sashiko-bot
2026-09-10 7:03 ` Krzysztof Kozlowski
2026-09-09 14:57 ` [PATCH v3 12/13] arm64: dts: qcom: sm8650: " Vikash Garodia
2026-09-09 14:57 ` [PATCH v3 13/13] arm64: dts: qcom: sm8750: " Vikash Garodia
12 siblings, 2 replies; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Daniel J Blueman, Vishnu Reddy
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. The non-pixel stream
cannot address the low 600MB of IOVA space, while the pixel stream can
address the full range:
+-----------------------------------------------------------+
| non-pixel stream addressable range (600 MB - 3.5 GB) |
| 0x25800000 - 0xe0000000 |
+-----------------------------------------------------------+
| pixel stream addressable range (0 - 3.5 GB) |
| 0x00000000 - 0xe0000000 |
+-----------------------------------------------------------+
A single "iommus" property on the video-codec node puts every stream in
one IOMMU domain sharing one IOVA allocator, so nothing restricts a
non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
that boundary the hardware faults, which shows up as unhandled SMMU page
faults and spontaneous reboots:
https://gitlab.freedesktop.org/drm/msm/-/work_items/100
Given that the address range restriction is for specific VPU stream, it
should be ideally be moved to that stream. To achieve the same, a subset
of streams is now represented as subnodes, so that each can be
associated with its respective addressable range. The design was discussed
and agreed by mainatiners here
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
In the past, this limitation was addressed with an iommu-map approach,
with the iris driver dynamically creating the devices. That approach
was later concluded to be a hack to avoid having subnodes, and was NAKed
by the iommu maintainers. It was discussed in detail here:
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
The video-codec node includes address-cells, size-cells and dma-ranges
to declare 1:1 mapping for DMA translation to the parent.
Tested-by: Daniel J Blueman <daniel@quora.org>
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/monaco.dtsi | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
index cfe39f923224f6c9af0a1b15a84f2fefb90d53d5..e4ad5960c97850693064f5dccec9b27cb75f3987 100644
--- a/arch/arm64/boot/dts/qcom/monaco.dtsi
+++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
@@ -5404,12 +5404,23 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
reset-names = "bus";
- iommus = <&apps_smmu 0x0880 0x0400>,
- <&apps_smmu 0x0887 0x0400>;
dma-coherent;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges = <0x0 0x0 0x0 0xe0000000>;
+
status = "disabled";
+ non-pixel {
+ iommus = <&apps_smmu 0x0880 0x0400>;
+ iommu-ranges = <0x25800000 0xba800000>;
+ };
+
+ pixel {
+ iommus = <&apps_smmu 0x0887 0x0400>;
+ };
+
iris_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 11/13] arm64: dts: qcom: monaco: Add Iris context bank subnodes
2026-09-09 14:57 ` [PATCH v3 11/13] arm64: dts: qcom: monaco: " Vikash Garodia
@ 2026-09-09 15:21 ` sashiko-bot
2026-09-10 7:03 ` Krzysztof Kozlowski
1 sibling, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-09-09 15:21 UTC (permalink / raw)
To: Vikash Garodia; +Cc: robh, conor+dt, devicetree, media-ci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing of_dma_configure() on dynamically created context bank sub-devices, resulting in unmapped DMA and SMMU faults.
--
commit 6813696691410944233b1d75d049bcb783208f85
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
arm64: dts: qcom: monaco: Add Iris context bank subnodes
This commit adds subnodes for the non-pixel and pixel VPU streams to the
video-codec node. This allows assigning specific addressable ranges to each
stream to prevent DMA allocations from landing in the restricted low 600MB
of IOVA space.
> diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
> index cfe39f923224f..e4ad5960c9785 100644
> --- a/arch/arm64/boot/dts/qcom/monaco.dtsi
> +++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
> @@ -5404,12 +5404,23 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
[ ... ]
> + non-pixel {
> + iommus = <&apps_smmu 0x0880 0x0400>;
> + iommu-ranges = <0x25800000 0xba800000>;
> + };
> +
> + pixel {
> + iommus = <&apps_smmu 0x0887 0x0400>;
> + };
[Severity: High]
Does the missing IOMMU configuration for these dynamically created subnodes
cause DMA allocations to fall back to unmapped physical memory?
While this patch adds the context bank subnodes to the device tree, an earlier
patch in this series ("media: iris: Add non-pixel and pixel context bank
devices") leaves the instantiated platform devices without DMA or IOMMU
configuration.
Because no driver binds to these dummy sub-devices, the Linux driver core never
calls really_probe() on them, meaning the bus-level platform_dma_configure() is
never invoked. Without an explicit call to of_dma_configure(), the IOMMU is
never attached to the sub-devices created in iris_create_cb_dev():
drivers/media/platform/qcom/iris/iris_probe.c:iris_create_cb_dev() {
...
pdev = platform_device_register_full(&plat_dev_info);
of_node_put(child_of_node);
if (IS_ERR(pdev))
return ERR_CAST(pdev);
dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
dma_set_seg_boundary(&pdev->dev, DMA_BIT_MASK(32));
return &pdev->dev;
}
Without an explicit of_dma_configure(&pdev->dev, child_of_node, true) before
returning, doesn't this unconditionally trigger unhandled SMMU page faults and
spontaneous reboots during video operations when the VPU attempts to access
the unmapped memory?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-vpu_iommu_iova_handling-v3-0-2fe43ae45495@oss.qualcomm.com?part=11
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v3 11/13] arm64: dts: qcom: monaco: Add Iris context bank subnodes
2026-09-09 14:57 ` [PATCH v3 11/13] arm64: dts: qcom: monaco: " Vikash Garodia
2026-09-09 15:21 ` sashiko-bot
@ 2026-09-10 7:03 ` Krzysztof Kozlowski
2026-09-11 17:53 ` Vikash Garodia
1 sibling, 1 reply; 31+ messages in thread
From: Krzysztof Kozlowski @ 2026-09-10 7:03 UTC (permalink / raw)
To: Vikash Garodia
Cc: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy,
linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, iommu, Daniel J Blueman, Vishnu Reddy
On Wed, Sep 09, 2026 at 08:27:18PM +0530, Vikash Garodia wrote:
> The VPU issues DMA through several SMMU streams, and the hardware does
> not give every stream the same addressable range. The non-pixel stream
> cannot address the low 600MB of IOVA space, while the pixel stream can
> address the full range:
> +-----------------------------------------------------------+
> | non-pixel stream addressable range (600 MB - 3.5 GB) |
> | 0x25800000 - 0xe0000000 |
> +-----------------------------------------------------------+
> | pixel stream addressable range (0 - 3.5 GB) |
> | 0x00000000 - 0xe0000000 |
> +-----------------------------------------------------------+
> A single "iommus" property on the video-codec node puts every stream in
> one IOMMU domain sharing one IOVA allocator, so nothing restricts a
> non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
> that boundary the hardware faults, which shows up as unhandled SMMU page
> faults and spontaneous reboots:
> https://gitlab.freedesktop.org/drm/msm/-/work_items/100
>
> Given that the address range restriction is for specific VPU stream, it
> should be ideally be moved to that stream. To achieve the same, a subset
> of streams is now represented as subnodes, so that each can be
> associated with its respective addressable range. The design was discussed
> and agreed by mainatiners here
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
>
> In the past, this limitation was addressed with an iommu-map approach,
> with the iris driver dynamically creating the devices. That approach
> was later concluded to be a hack to avoid having subnodes, and was NAKed
> by the iommu maintainers. It was discussed in detail here:
> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
>
> The video-codec node includes address-cells, size-cells and dma-ranges
> to declare 1:1 mapping for DMA translation to the parent.
>
> Tested-by: Daniel J Blueman <daniel@quora.org>
I do not believe the testing actually occured, except some case, so
please apply it ONLY to the commits where applicable.
I already asked this at v2.
If Daniel did not confirm where this was tested, then the tag DOES NOT
apply to any of the DTS patches.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 11/13] arm64: dts: qcom: monaco: Add Iris context bank subnodes
2026-09-10 7:03 ` Krzysztof Kozlowski
@ 2026-09-11 17:53 ` Vikash Garodia
0 siblings, 0 replies; 31+ messages in thread
From: Vikash Garodia @ 2026-09-11 17:53 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy,
linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, iommu, Daniel J Blueman, Vishnu Reddy
On 9/10/2026 12:33 PM, Krzysztof Kozlowski wrote:
> On Wed, Sep 09, 2026 at 08:27:18PM +0530, Vikash Garodia wrote:
>> The VPU issues DMA through several SMMU streams, and the hardware does
>> not give every stream the same addressable range. The non-pixel stream
>> cannot address the low 600MB of IOVA space, while the pixel stream can
>> address the full range:
>> +-----------------------------------------------------------+
>> | non-pixel stream addressable range (600 MB - 3.5 GB) |
>> | 0x25800000 - 0xe0000000 |
>> +-----------------------------------------------------------+
>> | pixel stream addressable range (0 - 3.5 GB) |
>> | 0x00000000 - 0xe0000000 |
>> +-----------------------------------------------------------+
>> A single "iommus" property on the video-codec node puts every stream in
>> one IOMMU domain sharing one IOVA allocator, so nothing restricts a
>> non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
>> that boundary the hardware faults, which shows up as unhandled SMMU page
>> faults and spontaneous reboots:
>> https://gitlab.freedesktop.org/drm/msm/-/work_items/100
>>
>> Given that the address range restriction is for specific VPU stream, it
>> should be ideally be moved to that stream. To achieve the same, a subset
>> of streams is now represented as subnodes, so that each can be
>> associated with its respective addressable range. The design was discussed
>> and agreed by mainatiners here
>> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
>>
>> In the past, this limitation was addressed with an iommu-map approach,
>> with the iris driver dynamically creating the devices. That approach
>> was later concluded to be a hack to avoid having subnodes, and was NAKed
>> by the iommu maintainers. It was discussed in detail here:
>> https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
>>
>> The video-codec node includes address-cells, size-cells and dma-ranges
>> to declare 1:1 mapping for DMA translation to the parent.
>>
>> Tested-by: Daniel J Blueman <daniel@quora.org>
>
> I do not believe the testing actually occured, except some case, so
> please apply it ONLY to the commits where applicable.
>
> I already asked this at v2.
>
> If Daniel did not confirm where this was tested, then the tag DOES NOT
> apply to any of the DTS patches.
you are right, it was not clearly confirmed where it was tested. I would
drop the tag.
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 12/13] arm64: dts: qcom: sm8650: Add Iris context bank subnodes
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (10 preceding siblings ...)
2026-09-09 14:57 ` [PATCH v3 11/13] arm64: dts: qcom: monaco: " Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 15:26 ` sashiko-bot
2026-09-09 14:57 ` [PATCH v3 13/13] arm64: dts: qcom: sm8750: " Vikash Garodia
12 siblings, 1 reply; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Daniel J Blueman, Vishnu Reddy
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. The non-pixel stream
cannot address the low 600MB of IOVA space, while the pixel stream can
address the full range:
+-----------------------------------------------------------+
| non-pixel stream addressable range (600 MB - 3.5 GB) |
| 0x25800000 - 0xe0000000 |
+-----------------------------------------------------------+
| pixel stream addressable range (0 - 3.5 GB) |
| 0x00000000 - 0xe0000000 |
+-----------------------------------------------------------+
A single "iommus" property on the video-codec node puts every stream in
one IOMMU domain sharing one IOVA allocator, so nothing restricts a
non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
that boundary the hardware faults, which shows up as unhandled SMMU page
faults and spontaneous reboots:
https://gitlab.freedesktop.org/drm/msm/-/work_items/100
Given that the address range restriction is for specific VPU stream, it
should be ideally be moved to that stream. To achieve the same, a subset
of streams is now represented as subnodes, so that each can be
associated with its respective addressable range. The design was discussed
and agreed by mainatiners here
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
In the past, this limitation was addressed with an iommu-map approach,
with the iris driver dynamically creating the devices. That approach
was later concluded to be a hack to avoid having subnodes, and was NAKed
by the iommu maintainers. It was discussed in detail here:
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
The video-codec node includes address-cells, size-cells and dma-ranges
to declare 1:1 mapping for DMA translation to the parent.
Tested-by: Daniel J Blueman <daniel@quora.org>
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sm8650.dtsi | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index b679fd8afb7d533f2ca42316cea6fb34997feb75..97dad9c7f7fc47451d14620700aa5ec9ff0d09aa 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -5275,11 +5275,12 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
"xo",
"core";
- iommus = <&apps_smmu 0x1940 0>,
- <&apps_smmu 0x1947 0>;
-
dma-coherent;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges = <0x0 0x0 0x0 0xe0000000>;
+
/*
* IRIS firmware is signed by vendors, only
* enable on boards where the proper signed firmware
@@ -5287,6 +5288,15 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
*/
status = "disabled";
+ non-pixel {
+ iommus = <&apps_smmu 0x1940 0x0>;
+ iommu-ranges = <0x25800000 0xba800000>;
+ };
+
+ pixel {
+ iommus = <&apps_smmu 0x1947 0x0>;
+ };
+
iris_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 12/13] arm64: dts: qcom: sm8650: Add Iris context bank subnodes
2026-09-09 14:57 ` [PATCH v3 12/13] arm64: dts: qcom: sm8650: " Vikash Garodia
@ 2026-09-09 15:26 ` sashiko-bot
0 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-09-09 15:26 UTC (permalink / raw)
To: Vikash Garodia; +Cc: robh, devicetree, conor+dt, media-ci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Dynamically created sub-devices for SMMU context banks lack PM runtime enablement, keeping the SMMU permanently active.
- [High] of_iommu_derive_resv_regions() erroneously translates IOVAs into CPU physical addresses.
--
commit 6e1de71c7ed4433a945133dd5bedc79a78bdd2e4
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
arm64: dts: qcom: sm8650: Add Iris context bank subnodes
This commit introduces subnodes for the non-pixel and pixel VPU SMMU streams
so that each stream can be associated with its respective addressable range.
It adds an iommu-ranges property to the non-pixel stream to restrict its
allocations, addressing hardware faults caused by buffers landing below the
600MB IOVA boundary.
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index b679fd8afb7d5..97dad9c7f7fc4 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
[ ... ]
> @@ -5287,6 +5288,15 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
> */
> status = "disabled";
>
> + non-pixel {
> + iommus = <&apps_smmu 0x1940 0x0>;
[Severity: High]
Does adding these subnodes cause a PM runtime leak in the SMMU?
When these dummy platform devices are dynamically created for context banks by
the iris driver in drivers/media/platform/qcom/iris/iris_probe.c:
iris_create_cb_dev() {
...
pdev = platform_device_register_full(&plat_dev_info);
...
}
The IOMMU core intercepts them, attaches them to the SMMU, and establishes a
PM runtime device link (DL_FLAG_PM_RUNTIME). Because the driver never
explicitly calls pm_runtime_enable() on these dummy devices, their PM
runtime disable depth remains greater than zero.
The PM core interprets this as the consumer being active, forcing the
supplier (the SMMU) and its interconnect power domains to resume and stay
active permanently.
> + iommu-ranges = <0x25800000 0xba800000>;
[Severity: High]
Does the use of iommu-ranges here cause the IOMMU core to reserve the wrong
address space?
When this property is parsed in drivers/iommu/of_iommu.c to compute reserved
regions:
of_iommu_derive_resv_regions() {
...
maps = of_translate_dma_region(dev->of_node, maps, &iova, &length);
...
}
The function of_translate_dma_region() uses dma-ranges to translate up to a
CPU physical address, rather than preserving the intended IOVA space.
This CPU physical address is then incorrectly passed as the IOVA base to
iommu_alloc_resv_region(), which can severely break the IOVA allocator and
block valid DMA allocations on architectures with non-identity DMA offsets.
> + };
> +
> + pixel {
> + iommus = <&apps_smmu 0x1947 0x0>;
> + };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-vpu_iommu_iova_handling-v3-0-2fe43ae45495@oss.qualcomm.com?part=12
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 13/13] arm64: dts: qcom: sm8750: Add Iris context bank subnodes
2026-09-09 14:57 [PATCH v3 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (11 preceding siblings ...)
2026-09-09 14:57 ` [PATCH v3 12/13] arm64: dts: qcom: sm8650: " Vikash Garodia
@ 2026-09-09 14:57 ` Vikash Garodia
2026-09-09 15:24 ` sashiko-bot
12 siblings, 1 reply; 31+ messages in thread
From: Vikash Garodia @ 2026-09-09 14:57 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio, Stanimir Varbanov,
Neil Armstrong, Dmitry Baryshkov, Bryan O'Donoghue,
Stephan Gerhold, Joerg Roedel (AMD), Will Deacon, Robin Murphy
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
Vishnu Reddy
The VPU issues DMA through several SMMU streams, and the hardware does
not give every stream the same addressable range. The non-pixel stream
cannot address the low 600MB of IOVA space, while the pixel stream can
address the full range:
+-----------------------------------------------------------+
| non-pixel stream addressable range (600 MB - 3.5 GB) |
| 0x25800000 - 0xe0000000 |
+-----------------------------------------------------------+
| pixel stream addressable range (0 - 3.5 GB) |
| 0x00000000 - 0xe0000000 |
+-----------------------------------------------------------+
A single "iommus" property on the video-codec node puts every stream in
one IOMMU domain sharing one IOVA allocator, so nothing restricts a
non-pixel buffer to avoid 0 to 600MB. Once an allocation lands below
that boundary the hardware faults, which shows up as unhandled SMMU page
faults and spontaneous reboots:
https://gitlab.freedesktop.org/drm/msm/-/work_items/100
Given that the address range restriction is for specific VPU stream, it
should be ideally be moved to that stream. To achieve the same, a subset
of streams is now represented as subnodes, so that each can be
associated with its respective addressable range. The design was discussed
and agreed by mainatiners here
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
In the past, this limitation was addressed with an iommu-map approach,
with the iris driver dynamically creating the devices. That approach
was later concluded to be a hack to avoid having subnodes, and was NAKed
by the iommu maintainers. It was discussed in detail here:
https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org/
The video-codec node includes address-cells, size-cells and dma-ranges
to declare 1:1 mapping for DMA translation to the parent.
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/sm8750.dtsi | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
index eb3a6990ec30ba3ca1c3fe468ffdb4e9df00174d..206ac22cab2c0dfabc1ca5f619f598e4823776de 100644
--- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
@@ -3026,8 +3026,6 @@ iris: video-codec@aa00000 {
"vcodec0_core_freerun";
dma-coherent;
- iommus = <&apps_smmu 0x1940 0>,
- <&apps_smmu 0x1947 0>;
interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
&config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
@@ -3060,6 +3058,10 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
"core",
"vcodec0_core";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges = <0x0 0x0 0x0 0xe0000000>;
+
/*
* IRIS firmware is signed by vendors, only
* enable in boards where the proper signed firmware
@@ -3067,6 +3069,15 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
*/
status = "disabled";
+ non-pixel {
+ iommus = <&apps_smmu 0x1940 0x0>;
+ iommu-ranges = <0x25800000 0xba800000>;
+ };
+
+ pixel {
+ iommus = <&apps_smmu 0x1947 0x0>;
+ };
+
iris_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v3 13/13] arm64: dts: qcom: sm8750: Add Iris context bank subnodes
2026-09-09 14:57 ` [PATCH v3 13/13] arm64: dts: qcom: sm8750: " Vikash Garodia
@ 2026-09-09 15:24 ` sashiko-bot
0 siblings, 0 replies; 31+ messages in thread
From: sashiko-bot @ 2026-09-09 15:24 UTC (permalink / raw)
To: Vikash Garodia; +Cc: conor+dt, media-ci, devicetree, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving the `iommus` property to subnodes breaks IOMMU configuration for the VPU because the `iris` driver does not configure DMA for these child devices.
--
commit 2a638b0a96fbbd027cf960b36f77ad140f56417d
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
arm64: dts: qcom: sm8750: Add Iris context bank subnodes
This patch adds non-pixel and pixel subnodes to the Iris video-codec
node to restrict address ranges for specific VPU streams. It removes
the single iommus property from the main node to resolve unhandled
SMMU page faults caused by non-pixel buffers falling below the 600MB
boundary.
> diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> index eb3a6990ec30b..206ac22cab2c0 100644
> --- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> @@ -3026,8 +3026,6 @@ iris: video-codec@aa00000 {
> "vcodec0_core_freerun";
>
> dma-coherent;
> - iommus = <&apps_smmu 0x1940 0>,
> - <&apps_smmu 0x1947 0>;
>
> interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
[ ... ]
> @@ -3067,6 +3069,15 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
> */
> status = "disabled";
>
> + non-pixel {
> + iommus = <&apps_smmu 0x1940 0x0>;
> + iommu-ranges = <0x25800000 0xba800000>;
> + };
> +
> + pixel {
> + iommus = <&apps_smmu 0x1947 0x0>;
> + };
> +
> iris_opp_table: opp-table {
[Severity: High]
Does moving the iommus property to these subnodes break IOMMU configuration
for the VPU?
Since these subnodes lack compatible strings, they will not bind to a
driver. Looking at iris_create_cb_dev() in the driver, it creates
platform devices for the child nodes but doesn't call of_dma_configure()
or attach an IOMMU domain:
drivers/media/platform/qcom/iris/iris_probe.c:iris_create_cb_dev() {
...
pdev = platform_device_register_full(&plat_dev_info);
of_node_put(child_of_node);
if (IS_ERR(pdev))
return ERR_CAST(pdev);
dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
dma_set_seg_boundary(&pdev->dev, DMA_BIT_MASK(32));
return &pdev->dev;
}
Because no driver binds to these sub-devices, the driver core never calls
really_probe() and skips platform_dma_configure(). Since the iris driver
does not manually configure DMA either, does this leave the sub-devices
without IOMMU configuration?
This could cause DMA allocations to bypass the IOMMU and yield raw physical
addresses instead of IOVAs. When the VPU attempts to access these addresses,
the SMMU might generate context faults.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-vpu_iommu_iova_handling-v3-0-2fe43ae45495@oss.qualcomm.com?part=13
^ permalink raw reply [flat|nested] 31+ messages in thread