* [PATCH v4 01/13] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 18:49 ` [PATCH v4 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes Vikash Garodia
` (11 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia
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.
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] 23+ messages in thread* [PATCH v4 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
2026-09-13 18:49 ` [PATCH v4 01/13] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 19:03 ` sashiko-bot
2026-09-13 18:49 ` [PATCH v4 03/13] dt-bindings: media: qcom,sm8750-iris: " Vikash Garodia
` (10 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
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.
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>
---
.../devicetree/bindings/media/qcom,sm8550-iris.yaml | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
index 0400ca1bff05dcef6b742c3fbf77e38adca9f280..6ee9d23554cffb30c92017ef99de15c2493455ac 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:
@@ -177,12 +177,23 @@ examples:
resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
reset-names = "bus";
- iommus = <&apps_smmu 0x1940 0x0000>,
- <&apps_smmu 0x1947 0x0000>;
dma-coherent;
operating-points-v2 = <&iris_opp_table>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges = <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] 23+ messages in thread* Re: [PATCH v4 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes
2026-09-13 18:49 ` [PATCH v4 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes Vikash Garodia
@ 2026-09-13 19:03 ` sashiko-bot
2026-09-13 19:10 ` Vikash Garodia
0 siblings, 1 reply; 23+ messages in thread
From: sashiko-bot @ 2026-09-13 19:03 UTC (permalink / raw)
To: Vikash Garodia; +Cc: robh, conor+dt, media-ci, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing subnode updates for the Milos Iris VPU enforce a DoS-vulnerable SMMU configuration.
--
commit 63b882fd7198eab0ac9d5c0a3932a253f2082c86
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes
This commit updates the sm8550-iris device tree binding to move SMMU stream
definitions into separate context bank subnodes. This addresses an issue where
placing all streams in a single top-level iommus property failed to prevent
non-pixel buffers from landing in the restricted low 600MB of IOVA space,
which caused unhandled SMMU page faults and system reboots.
> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> index 0400ca1bff05d..6ee9d23554cff 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
[Severity: High]
This is a pre-existing issue, but does the qcom,milos-iris.yaml binding
also need to be updated to support this fix?
Currently, qcom,milos-iris.yaml still mandates a top-level iommus property
under the required block:
Documentation/devicetree/bindings/media/qcom,milos-iris.yaml:
required:
- compatible
...
- iommus
Device trees for Milos cannot adopt the subnode fix without failing schema
validation. This forces the DoS-vulnerable monolithic SMMU configuration
where unprivileged userspace or remote video playback can trigger VPU
processing and DMA allocations below 600MB, resulting in SMMU page faults
and spontaneous reboots.
The SM8750 VPU update was successfully resolved in follow-up commit
"dt-bindings: media: qcom,sm8750-iris: Add context bank subnodes", but
Milos appears to remain unaddressed.
Should qcom,milos-iris.yaml be updated to remove the top-level iommus
requirement similarly to this file?
> - dma-coherent
>
> allOf:
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-vpu_iommu_iova_handling-v4-0-9b9074a73c41@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes
2026-09-13 19:03 ` sashiko-bot
@ 2026-09-13 19:10 ` Vikash Garodia
0 siblings, 0 replies; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 19:10 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, conor+dt, media-ci, devicetree
On 9/14/2026 12:33 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Missing subnode updates for the Milos Iris VPU enforce a DoS-vulnerable SMMU configuration.
> --
>
> commit 63b882fd7198eab0ac9d5c0a3932a253f2082c86
> Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
>
> dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes
>
> This commit updates the sm8550-iris device tree binding to move SMMU stream
> definitions into separate context bank subnodes. This addresses an issue where
> placing all streams in a single top-level iommus property failed to prevent
> non-pixel buffers from landing in the restricted low 600MB of IOVA space,
> which caused unhandled SMMU page faults and system reboots.
>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
>> index 0400ca1bff05d..6ee9d23554cff 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
>
> [Severity: High]
> This is a pre-existing issue, but does the qcom,milos-iris.yaml binding
> also need to be updated to support this fix?
>
> Currently, qcom,milos-iris.yaml still mandates a top-level iommus property
> under the required block:
>
> Documentation/devicetree/bindings/media/qcom,milos-iris.yaml:
> required:
> - compatible
> ...
> - iommus
>
> Device trees for Milos cannot adopt the subnode fix without failing schema
> validation. This forces the DoS-vulnerable monolithic SMMU configuration
> where unprivileged userspace or remote video playback can trigger VPU
> processing and DMA allocations below 600MB, resulting in SMMU page faults
> and spontaneous reboots.
This is handled with the fix
https://lore.kernel.org/all/20260818-reserve_iova_in_driver-v2-1-5005a1154408@oss.qualcomm.com/
Regards,
Vikash
>
> The SM8750 VPU update was successfully resolved in follow-up commit
> "dt-bindings: media: qcom,sm8750-iris: Add context bank subnodes", but
> Milos appears to remain unaddressed.
>
> Should qcom,milos-iris.yaml be updated to remove the top-level iommus
> requirement similarly to this file?
>
>> - dma-coherent
>>
>> allOf:
>
> [ ... ]
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 03/13] dt-bindings: media: qcom,sm8750-iris: Add context bank subnodes
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
2026-09-13 18:49 ` [PATCH v4 01/13] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema Vikash Garodia
2026-09-13 18:49 ` [PATCH v4 02/13] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 18:49 ` [PATCH v4 04/13] iommu: of_iommu: Add support for "iommu-ranges" on a device node Vikash Garodia
` (9 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
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.
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>
---
.../devicetree/bindings/media/qcom,sm8750-iris.yaml | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8750-iris.yaml
index c42d3470bdac796cc878090e65becf6b62bd80ca..b6d5ded1aae6a1a6836c2c24b1dae548cdc6f9d2 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
@@ -110,8 +109,6 @@ examples:
"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>,
@@ -144,6 +141,19 @@ examples:
"core",
"vcodec0_core";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges = <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] 23+ messages in thread* [PATCH v4 04/13] iommu: of_iommu: Add support for "iommu-ranges" on a device node
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (2 preceding siblings ...)
2026-09-13 18:49 ` [PATCH v4 03/13] dt-bindings: media: qcom,sm8750-iris: " Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 19:07 ` sashiko-bot
2026-09-13 18:49 ` [PATCH v4 05/13] media: iris: Add non-pixel and pixel context bank devices Vikash Garodia
` (8 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
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".
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 | 115 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 108 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index a18bb60f6f3dfdba853b6e16dd2a8616f6a731c7..baf74db838d6bc45df2b36c49eb47eeca21b4b7b 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -190,6 +190,98 @@ 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, "%pOF: failed to parse iommu-ranges\n",
+ dev->of_node);
+ break;
+ }
+
+ if (!length)
+ continue;
+
+ if (iova + length < iova) {
+ dev_err(dev, "%pOF: iommu-ranges overflows address space\n",
+ dev->of_node);
+ continue;
+ }
+
+ list_for_each_entry(pos, &ranges, node)
+ if (pos->start > iova)
+ break;
+
+ new = kmalloc_obj(*new);
+ if (!new)
+ continue;
+
+ new->start = iova;
+ new->end = iova + length;
+ 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 > next || !pos->end)
+ next = pos->end;
+
+ list_del(&pos->node);
+ kfree(pos);
+ }
+
+ if (!next)
+ return;
+
+ 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 +306,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 +322,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 +354,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 +364,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] 23+ messages in thread* Re: [PATCH v4 04/13] iommu: of_iommu: Add support for "iommu-ranges" on a device node
2026-09-13 18:49 ` [PATCH v4 04/13] iommu: of_iommu: Add support for "iommu-ranges" on a device node Vikash Garodia
@ 2026-09-13 19:07 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-09-13 19:07 UTC (permalink / raw)
To: Vikash Garodia; +Cc: devicetree, robh, conor+dt, media-ci
Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
New issues:
- [High] Breaks backward compatibility for existing Device Trees using iommu-addresses without a reg property.
- [High] Silent fail-open behavior grants full IOVA access if iommu-ranges parsing or allocation fails.
- [High] Truncation of the remaining reserved region length on 32-bit systems with LPAE (64-bit phys_addr_t).
- [High] Mismatched direct mappings are discarded instead of being reserved, contradicting the emitted kernel warning.
- [High] of_translate_dma_region is called without validating that the property buffer contains enough remaining cells.
- [High] of_translate_dma_region improperly translates IOVAs into CPU physical addresses, resulting in incorrect IOMMU reservations.
- [High] The overflow check iova + length < iova incorrectly rejects a valid IOVA range that extends exactly to the maximum address.
Pre-existing issues:
- [High] Misparsing of multi-device iommu-addresses property due to incorrect iterator advancement.
- [High] Unchecked return value from of_translate_dma_region leads to a NULL pointer dereference.
--
commit e45b53e829ee68d166cf13e6d742c00ee86c9f4c
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
iommu: of_iommu: Add support for "iommu-ranges" on a device node
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
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index a18bb60f6f3df..baf74db838d6b 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -190,6 +190,98 @@ 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);
[Severity: High]
Does this code validate that the property buffer has enough remaining cells
before reading them? While the loop checks maps < end in
of_iommu_derive_resv_regions(), of_translate_dma_region() blindly reads
na + ns cells without bounds checking. Could a malformed iommu-ranges
property cause an out-of-bounds read of kernel memory?
Also, does this function incorrectly translate IOVAs into CPU physical
addresses? The iommu-ranges property specifies usable device IOVA ranges,
but of_translate_dma_region() translates the address through the parent
bus dma-ranges into a CPU physical address. Since IOMMU reserved regions
operate in the IOVA space, doesn't this corrupt the reservation targets
and leave the actual IOVA space unprotected?
> + if (!maps) {
> + dev_err(dev, "%pOF: failed to parse iommu-ranges\n",
> + dev->of_node);
> + break;
> + }
> +
> + if (!length)
> + continue;
> +
> + if (iova + length < iova) {
[Severity: High]
Does this overflow check reject a valid IOVA range that extends exactly to
the maximum physical address? In of_iommu_derive_resv_regions(), if a
valid hardware IOVA mapping spans exactly to the end of the address space
limit, iova + length will wrap around to exactly 0. Since 0 < iova
evaluates to true, wouldn't this falsely reject the valid maximum-address
range as an overflow and leave that IOVA space completely unprotected?
> + dev_err(dev, "%pOF: iommu-ranges overflows address space\n",
> + dev->of_node);
> + continue;
> + }
> +
> + list_for_each_entry(pos, &ranges, node)
> + if (pos->start > iova)
> + break;
> +
> + new = kmalloc_obj(*new);
> + if (!new)
> + continue;
> +
> + new->start = iova;
> + new->end = iova + length;
> + list_add_tail(&new->node, &pos->node);
> + }
> +
> + if (list_empty(&ranges))
> + return;
[Severity: High]
Does this fail-open instead of fail-secure if iommu-ranges parsing or
memory allocation fails? If of_translate_dma_region() returns NULL for a
malformed property or kmalloc_obj() fails in
of_iommu_derive_resv_regions(), the loop breaks or continues, leaving the
ranges list empty. This early return would then exit without adding any
reserved regions, effectively telling the IOMMU that the entire address
space is usable 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 > next || !pos->end)
> + next = pos->end;
> +
> + list_del(&pos->node);
> + kfree(pos);
> + }
> +
> + if (!next)
> + return;
> +
> + region = iommu_alloc_resv_region(next, ~(phys_addr_t)0 - next + 1,
> + prot, IOMMU_RESV_RESERVED, GFP_KERNEL);
[Severity: High]
Can this truncate the length calculation on 32-bit systems with LPAE?
In of_iommu_derive_resv_regions(), the remaining address space length is
calculated as ~(phys_addr_t)0 - next + 1. On 32-bit LPAE systems,
phys_addr_t is 64-bit, but iommu_alloc_resv_region() expects a size_t
for length, which is 32-bit. Wouldn't this cast to size_t silently
truncate the 64-bit length, restricting the reservation up to the 4GB
boundary and leaving the remaining 64-bit IOVA space unreserved?
> + if (region)
> + list_add_tail(®ion->list, list);
> +}
[ ... ]
> @@ -214,10 +306,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 +322,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 break backward compatibility for existing Device Trees?
The previous code in of_iommu_get_resv_regions() explicitly documented
and handled the reg property as optional for IOVA reservations. By
introducing a strict requirement and failing if reg is missing, wouldn't
older Device Trees that omitted reg now fail to parse, resulting in an
ABI breakage?
>
> end = maps + size / sizeof(__be32);
>
> while (maps < end) {
> struct device_node *np;
> u32 phandle;
>
> phandle = be32_to_cpup(maps++);
> np = of_find_node_by_phandle(phandle);
>
> if (np == dev->of_node) {
[Severity: High]
This is a pre-existing issue, but does this loop correctly advance the
maps pointer when np != dev->of_node? In of_iommu_get_resv_regions(), if
the current phandle does not match, it skips processing but fails to
advance maps by the size of the unhandled IOVA range (na + ns). Wouldn't
the next iteration then incorrectly read an IOVA cell as a phandle,
causing a parser desync?
> 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 isn't a bug introduced by this patch, but does the return value of
of_translate_dma_region() need to be checked for NULL? In
of_iommu_get_resv_regions(), if it fails and returns NULL, it is assigned
to maps. The loop condition maps < end will evaluate to true, and the
next iteration will call be32_to_cpup(maps++), resulting in a NULL
pointer dereference.
> if (length == 0) {
> dev_warn(dev, "Cannot reserve IOVA region of 0 size\n");
> continue;
> }
> type = iommu_resv_region_get_type(dev, &phys, iova, length);
>
> + if (type != IOMMU_RESV_DIRECT)
> + continue;
[Severity: High]
Does this skip creating the reservation when the mapping is not direct?
In of_iommu_get_resv_regions(), the iommu_resv_region_get_type() helper
detects non-direct mappings, warns the user it is treating them as
reservations, and returns IOMMU_RESV_RESERVED. By forcefully skipping
when type is not IOMMU_RESV_DIRECT, doesn't this entirely drop the region
instead of creating the reservation, contradicting the helper's intent
and breaking existing IOVA reservations?
> +
> region = iommu_alloc_resv_region(iova, length, prot, type,
> GFP_KERNEL);
> if (region)
> list_add_tail(®ion->list, list);
> }
> }
> }
> +
> + of_iommu_derive_resv_regions(dev, list);
> #endif
> }
> EXPORT_SYMBOL(of_iommu_get_resv_regions);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-vpu_iommu_iova_handling-v4-0-9b9074a73c41@oss.qualcomm.com?part=4
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 05/13] media: iris: Add non-pixel and pixel context bank devices
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (3 preceding siblings ...)
2026-09-13 18:49 ` [PATCH v4 04/13] iommu: of_iommu: Add support for "iommu-ranges" on a device node Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 19:06 ` sashiko-bot
2026-09-13 18:49 ` [PATCH v4 06/13] media: iris: Route buffers to the matching context bank device Vikash Garodia
` (7 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
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
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().
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] 23+ messages in thread* Re: [PATCH v4 05/13] media: iris: Add non-pixel and pixel context bank devices
2026-09-13 18:49 ` [PATCH v4 05/13] media: iris: Add non-pixel and pixel context bank devices Vikash Garodia
@ 2026-09-13 19:06 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-09-13 19:06 UTC (permalink / raw)
To: Vikash Garodia; +Cc: media-ci, robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free of `iris_core` structure due to devres and asynchronous V4L2 file closures.
- [High] Race condition in probe exposes video device before initialization, causing NULL dereference.
- [High] `iris_remove()` destroys `core->lock` without acquiring it, leading to a race condition with concurrent file closure.
--
commit 06944aa24efc3154b815139f80472c743bbbd5e1
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
media: iris: Add non-pixel and pixel context bank devices
This commit adds helper functions to create and clean up "non-pixel" and
"pixel" context bank devices from device tree subnodes. It sets up the context
banks prior to video device registration to ensure proper DMA addressable
range routing, preventing SMMU page faults and spontaneous reboots.
> 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);
> +
[Severity: High]
This is a pre-existing issue, but does iris_remove() allow a use-after-free
of core if a userspace process keeps the device node open?
The core structure is allocated with devm_kzalloc() in iris_probe(),
meaning it is freed automatically when the driver unbinds. However,
iris_remove() runs to completion and does not block for active file
descriptors to close.
If userspace holds the file descriptor open during an unbind event and later
closes it, iris_close() is invoked, which dereferences the freed memory:
drivers/media/platform/qcom/iris/iris_vidc.c:iris_close() {
...
iris_check_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
...
}
> mutex_destroy(&core->lock);
[Severity: High]
This isn't a bug introduced by this patch, but is it safe to destroy
core->lock here unconditionally?
If iris_remove() executes concurrently with a file closure, iris_close()
can acquire this lock via iris_remove_session():
drivers/media/platform/qcom/iris/iris_vidc.c:iris_remove_session() {
mutex_lock(&core->lock);
list_for_each_entry_safe(iter, temp, &core->instances, list) {
...
mutex_unlock(&core->lock);
}
Destroying the mutex while it might be held by another thread violates locking
rules and can lead to corruption when the racing thread releases the 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);
[Severity: High]
This is a pre-existing issue, but is there a race condition here that exposes
the video device to userspace before initialization is complete?
iris_register_video_device() calls video_register_device(), making the
/dev/videoX node visible, and then sets video_set_drvdata(). If userspace
races to open the device node in between, video_drvdata() could return NULL.
Additionally, PM runtime enablement in iris_probe() happens after the device
is registered:
drivers/media/platform/qcom/iris/iris_probe.c:iris_probe() {
...
pm_runtime_use_autosuspend(core->dev);
ret = devm_pm_runtime_enable(core->dev);
...
}
If iris_open() is called by userspace before this completes, it attempts to
dereference core->dev (which could be NULL if video_set_drvdata() hasn't
executed yet), leading to a kernel panic:
drivers/media/platform/qcom/iris/iris_vidc.c:iris_open() {
struct iris_core *core = video_drvdata(filp);
...
ret = pm_runtime_resume_and_get(core->dev);
...
}
> if (ret)
> goto err_v4l2_unreg;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-vpu_iommu_iova_handling-v4-0-9b9074a73c41@oss.qualcomm.com?part=5
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 06/13] media: iris: Route buffers to the matching context bank device
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (4 preceding siblings ...)
2026-09-13 18:49 ` [PATCH v4 05/13] media: iris: Add non-pixel and pixel context bank devices Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 19:13 ` sashiko-bot
2026-09-13 18:49 ` [PATCH v4 07/13] media: iris: Skip DMA mask setup when the core device has no IOMMU Vikash Garodia
` (6 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
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.
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.
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] 23+ messages in thread* Re: [PATCH v4 06/13] media: iris: Route buffers to the matching context bank device
2026-09-13 18:49 ` [PATCH v4 06/13] media: iris: Route buffers to the matching context bank device Vikash Garodia
@ 2026-09-13 19:13 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-09-13 19:13 UTC (permalink / raw)
To: Vikash Garodia; +Cc: devicetree, media-ci, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free of `core` structure and context bank devices during deferred V4L2 file descriptor closure.
- [High] Use-After-Free of `core` structure due to uncanceled `sys_error_handler` delayed work.
--
commit 6eb05ecb82941180acaafce80a0667d695f203de
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
media: iris: Route buffers to the matching context bank device
This commit introduces iris_get_cb_dev() to dynamically route buffers to the
correct context bank device based on their type and domain. It handles the
different addressable IOVA ranges between pixel and non-pixel SMMU streams.
If a relevant context bank device is missing, it falls back to the core
device for backward compatibility.
> 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);
> kfree(buffer);
> }
[Severity: High]
This isn't a bug introduced by this patch, but this patch introduces
additional use-after-free vectors by dynamically managing unrefcounted
context bank devices.
If userspace holds a file descriptor open and the driver is unbound,
iris_remove() unregisters the context bank devices (p_dev, np_dev). After
iris_remove() returns, the devres subsystem frees the core structure. Later,
when the userspace application closes the file descriptor, iris_close() is
invoked, which calls into iris_destroy_internal_buffer().
Could this lead to a use-after-free when iris_get_cb_dev() dereferences
inst->core, and subsequently when dma_free_attrs() uses the already freed
dev?
While reviewing this code, I also noticed a pre-existing lifetime issue in
iris_remove():
drivers/media/platform/qcom/iris/iris_probe.c:iris_remove() {
...
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 if a hardware error occurs,
the driver schedules the sys_error_handler delayed work. If the driver is
unbound before this work executes, iris_remove() completes without calling
cancel_delayed_work_sync(&core->sys_error_handler).
Since the devres subsystem then automatically frees the core structure, can
this cause a use-after-free when the delayed work timer expires and
iris_sys_error_handler() dereferences the freed core pointer?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-vpu_iommu_iova_handling-v4-0-9b9074a73c41@oss.qualcomm.com?part=6
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 07/13] media: iris: Skip DMA mask setup when the core device has no IOMMU
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (5 preceding siblings ...)
2026-09-13 18:49 ` [PATCH v4 06/13] media: iris: Route buffers to the matching context bank device Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 19:03 ` sashiko-bot
2026-09-13 18:49 ` [PATCH v4 08/13] arm64: dts: qcom: hamoa: Add Iris context bank subnodes Vikash Garodia
` (5 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel,
Konrad Dybcio, Krzysztof Kozlowski, iommu, Vikash Garodia,
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().
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] 23+ messages in thread* Re: [PATCH v4 07/13] media: iris: Skip DMA mask setup when the core device has no IOMMU
2026-09-13 18:49 ` [PATCH v4 07/13] media: iris: Skip DMA mask setup when the core device has no IOMMU Vikash Garodia
@ 2026-09-13 19:03 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-09-13 19:03 UTC (permalink / raw)
To: Vikash Garodia; +Cc: media-ci, conor+dt, devicetree, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Race condition leading to a NULL pointer dereference because the video device is registered before its private data and PM runtime are fully initialized.
--
commit 238457488dcd9147aa8709c84d5fae08c48163b9
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
media: iris: Skip DMA mask setup when the core device has no IOMMU
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.
> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index debd1f0e57038..4bdb078d83b5c 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;
> + }
[Severity: High]
This isn't a bug introduced by this patch, but could this sequence leave a
window for a race condition where userspace accesses the video device before
it is fully initialized?
In iris_probe.c:iris_register_video_device(), the video node is registered
before the driver data is set:
ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
if (ret)
goto err_vdev_release;
video_set_drvdata(vdev, core);
This exposes the /dev/videoX node to userspace immediately. If a process
like udev opens the device right away, iris_open() in iris_vidc.c can execute
before video_set_drvdata() or the remainder of iris_probe() completes.
If this happens, iris_open() calls video_drvdata() which returns a null
pointer. Passing this to pm_runtime_resume_and_get(core->dev) would then
cause a null pointer dereference.
Furthermore, exposing the device before devm_pm_runtime_enable() completes
further down in iris_probe() could leave the device in an uninitialized
state, resulting in unexpected access errors.
>
> dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
> dma_set_seg_boundary(&pdev->dev, DMA_BIT_MASK(32));
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-vpu_iommu_iova_handling-v4-0-9b9074a73c41@oss.qualcomm.com?part=7
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 08/13] arm64: dts: qcom: hamoa: Add Iris context bank subnodes
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (6 preceding siblings ...)
2026-09-13 18:49 ` [PATCH v4 07/13] media: iris: Skip DMA mask setup when the core device has no IOMMU Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 19:08 ` sashiko-bot
2026-09-13 18:49 ` [PATCH v4 09/13] arm64: dts: qcom: sm8550: " Vikash Garodia
` (4 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
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/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 fcd88e5f5d7ee1b15e4329d45131ab969e1c9970..63e9ad58097f79b62102c79ff0cffddf12d00f48 100644
--- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
+++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
@@ -5462,10 +5462,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
@@ -5473,6 +5475,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] 23+ messages in thread* Re: [PATCH v4 08/13] arm64: dts: qcom: hamoa: Add Iris context bank subnodes
2026-09-13 18:49 ` [PATCH v4 08/13] arm64: dts: qcom: hamoa: Add Iris context bank subnodes Vikash Garodia
@ 2026-09-13 19:08 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-09-13 19:08 UTC (permalink / raw)
To: Vikash Garodia; +Cc: conor+dt, devicetree, media-ci, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing IOMMU configuration for dynamically created dummy platform devices.
--
commit 27813a8c872ca25525366d3507070b27ab569ab8
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
arm64: dts: qcom: hamoa: Add Iris context bank subnodes
This commit adds non-pixel and pixel subnodes to the video-codec node to
assign specific IOVA address ranges to different SMMU streams. It replaces the
single iommus property on the parent node with subnode-specific configurations.
> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> index fcd88e5f5d7ee..63e9ad58097f7 100644
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
[ ... ]
> @@ -5473,6 +5475,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>;
> + };
> +
[Severity: High]
Since these subnodes are dynamically instantiated as platform devices by the
Iris driver using platform_device_register_full(), they do not have matching
drivers and will never enter really_probe().
The platform bus normally sets up IOMMU and DMA during really_probe() by
calling platform_dma_configure() -> of_dma_configure().
Without the driver manually calling of_dma_configure() on these new
sub-devices, will their IOMMU mappings remain unconfigured?
If they are left unconfigured, could the video-codec hardware streams bypass
the SMMU IOVA allocator or fail DMA mappings, resulting in unhandled SMMU
context faults when a video encoding or decoding session is initiated?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-vpu_iommu_iova_handling-v4-0-9b9074a73c41@oss.qualcomm.com?part=8
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 09/13] arm64: dts: qcom: sm8550: Add Iris context bank subnodes
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (7 preceding siblings ...)
2026-09-13 18:49 ` [PATCH v4 08/13] arm64: dts: qcom: hamoa: Add Iris context bank subnodes Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 19:05 ` sashiko-bot
2026-09-13 18:49 ` [PATCH v4 10/13] arm64: dts: qcom: lemans: " Vikash Garodia
` (3 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
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/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 23604436add303348867f890890f2f7af4f831af..60164b853ae0764cdd79c4680490e66a548b0613 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -3690,8 +3690,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;
/*
@@ -3701,6 +3699,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] 23+ messages in thread* Re: [PATCH v4 09/13] arm64: dts: qcom: sm8550: Add Iris context bank subnodes
2026-09-13 18:49 ` [PATCH v4 09/13] arm64: dts: qcom: sm8550: " Vikash Garodia
@ 2026-09-13 19:05 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-09-13 19:05 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:
Pre-existing issues:
- [High] Failure to skip address and size cells for non-matching nodes when parsing `iommu-addresses`.
--
commit a702abacb77d093fbe2f72280c242fce5e476787
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
arm64: dts: qcom: sm8550: Add Iris context bank subnodes
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. This patch moves the address range restriction
to specific VPU streams represented as subnodes.
> diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> index 23604436add30..60164b853ae07 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
[ ... ]
> @@ -3701,6 +3699,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>;
> + };
> +
[Severity: High]
This isn't a bug introduced by this patch, but it is a pre-existing High
severity issue that requires fixing.
Will this device tree layout expose a parser desynchronization in the IOMMU
code when handling nodes?
If an iommu-addresses property contains entries for multiple devices, the
parser fails to skip address and size cells for non-matching nodes:
drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() {
if (np == dev->of_node) {
int prot = IOMMU_READ | IOMMU_WRITE;
...
maps = of_translate_dma_region(np, maps, &iova, &length);
...
}
}
Because there is no else branch to advance the maps pointer past the
address and size cells for non-matching nodes, if the parser encounters a
phandle for a different device first, it evaluates np == dev->of_node as
false.
In the next loop iteration, the parser then misinterprets the first address
cell of the skipped entry as the next phandle.
Could this desynchronization cause the parser to miss IOMMU reserved regions,
allowing the IOMMU allocator to allocate IOVA addresses that overlap with
reserved physical memory regions and resulting in SMMU faults or memory
corruption?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-vpu_iommu_iova_handling-v4-0-9b9074a73c41@oss.qualcomm.com?part=9
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 10/13] arm64: dts: qcom: lemans: Add Iris context bank subnodes
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (8 preceding siblings ...)
2026-09-13 18:49 ` [PATCH v4 09/13] arm64: dts: qcom: sm8550: " Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 18:49 ` [PATCH v4 11/13] arm64: dts: qcom: monaco: " Vikash Garodia
` (2 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
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/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] 23+ messages in thread* [PATCH v4 11/13] arm64: dts: qcom: monaco: Add Iris context bank subnodes
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (9 preceding siblings ...)
2026-09-13 18:49 ` [PATCH v4 10/13] arm64: dts: qcom: lemans: " Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 19:15 ` sashiko-bot
2026-09-13 18:49 ` [PATCH v4 12/13] arm64: dts: qcom: sm8650: " Vikash Garodia
2026-09-13 18:49 ` [PATCH v4 13/13] arm64: dts: qcom: sm8750: " Vikash Garodia
12 siblings, 1 reply; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
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/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 395d32d368443e6c73b934b183c3de007ca2e6b5..89ea87a274a159da872facb688cd742d8d28f3c5 100644
--- a/arch/arm64/boot/dts/qcom/monaco.dtsi
+++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
@@ -5403,12 +5403,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] 23+ messages in thread* Re: [PATCH v4 11/13] arm64: dts: qcom: monaco: Add Iris context bank subnodes
2026-09-13 18:49 ` [PATCH v4 11/13] arm64: dts: qcom: monaco: " Vikash Garodia
@ 2026-09-13 19:15 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-09-13 19:15 UTC (permalink / raw)
To: Vikash Garodia; +Cc: conor+dt, devicetree, robh, media-ci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver fails to configure DMA and the IOMMU for dynamically created subdevices, causing fatal SMMU faults when the parent's `iommus` property is removed.
--
commit 7baf55062247d31a2ed357168e7566838a0d7a92
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
arm64: dts: qcom: monaco: Add Iris context bank subnodes
This commit represents a subset of Iris VPU streams as subnodes in the
device tree to associate them with their respective addressable ranges.
It removes the iommus property from the parent video-codec node and
places them into the new non-pixel and pixel subnodes instead.
Link: https://gitlab.freedesktop.org/drm/msm/-/work_items/100
Link: https://lore.kernel.org/all/c7b956a9-d3e8-4e18-b780-5d08f5cd2ca1@kernel.org
> diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
> index 395d32d368443..89ea87a274a15 100644
> --- a/arch/arm64/boot/dts/qcom/monaco.dtsi
> +++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
> @@ -5403,12 +5403,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>;
[Severity: High]
This is a pre-existing issue, but does removing the iommus property from the
parent node here actively expose an SMMU translation fault?
The qcom_iris driver dynamically creates these subdevices manually via
platform_device_register_full() (for example, in iris_create_cb_dev() in
drivers/media/platform/qcom/iris/iris_probe.c). Since no driver binds to
these newly created subdevices, platform_dma_configure() bails out early
and skips calling iommu_device_use_default_domain().
Previously, this was masked because the driver could fall back to the parent
video-codec device configuration. By removing the iommus property from the
parent here, the non-pixel subdevice is left without IOMMU-backed DMA ops.
It receives physical addresses instead of IOVAs, causing the SMMU hardware
to block the unmapped transactions and triggering spontaneous reboots.
Should the driver be updated to properly configure DMA for its dynamically
created subdevices before making this device tree change?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-vpu_iommu_iova_handling-v4-0-9b9074a73c41@oss.qualcomm.com?part=11
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 12/13] arm64: dts: qcom: sm8650: Add Iris context bank subnodes
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (10 preceding siblings ...)
2026-09-13 18:49 ` [PATCH v4 11/13] arm64: dts: qcom: monaco: " Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
2026-09-13 18:49 ` [PATCH v4 13/13] arm64: dts: qcom: sm8750: " Vikash Garodia
12 siblings, 0 replies; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
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/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 99509305f9702addbbf3f4269e558e08d0bc1b25..88233c1f6107c5fbc0f0f68202fe6383abb10f3f 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -5274,11 +5274,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
@@ -5286,6 +5287,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] 23+ messages in thread* [PATCH v4 13/13] arm64: dts: qcom: sm8750: Add Iris context bank subnodes
2026-09-13 18:49 [PATCH v4 00/13] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
` (11 preceding siblings ...)
2026-09-13 18:49 ` [PATCH v4 12/13] arm64: dts: qcom: sm8650: " Vikash Garodia
@ 2026-09-13 18:49 ` Vikash Garodia
12 siblings, 0 replies; 23+ messages in thread
From: Vikash Garodia @ 2026-09-13 18:49 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,
Abel Vesa
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 28cf22da854d01413b8e86be3f6330354aad05f2..2e1e07eb5b6aa49850a7a5572ef9e755e298b799 100644
--- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
@@ -3025,8 +3025,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>,
@@ -3059,6 +3057,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
@@ -3066,6 +3068,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] 23+ messages in thread